[TVMC][TensorRT] Enable TensorRT target through TVMC #7658

akmaru · 2021-03-15T06:29:46Z

Adds TensorRT integration as a composite target into TVMC.

leandron

Thanks for the PR. I suggest we coordinate this with #7577, as you're gonna see on my comment. Once that comes in, with a little refactoring we can have this support done.

leandron · 2021-03-15T10:14:03Z

python/tvm/driver/tvmc/compiler.py

@@ -196,9 +196,9 @@ def compile_model(
    for codegen_from_cli in extra_targets:
        codegen = composite_target.get_codegen_by_target(codegen_from_cli["name"])
        partition_function = codegen["pass_pipeline"]
-        mod = partition_function(mod, params)
+        mod, codegen_config = partition_function(mod, params)


In the interest of standardizing all the partition_for_* functions, I think it would be good to do a little refactoring in the partition_for_tensorrt so that it only returns mod. This is important not only for TVMC, but I think it would be a good move in terms of future partitioning support in TVM.

There is some new functionality (yet to be merged) in #7577 to have an init function per target in tvmc, in which we can then populate the configs using the existing get_tensorrt_version.

I suggest we coordinate to get #7577 landed and then adjust this to comply with the suggestions above, so that we can get a uniform support for all targets. What do you think?

(cc @comaniac)

I agree that it would be easier to ask all pritition functions return a single module. However, the init function introduced by #7577 seems not applicable to this case to me. The config required by TensorRT is used when building the module; while the init function used by Vitis-AI imports the required package. I have no idea how could we make it general enough to support both Vitis-AI and TensorRT.

On the other hand, let a partition function return a tuple of module and build config seems more straightforward at the first glance, as I cannot think of a better idea for now. First, partition functions won't be a Relay pass in a pass sequence (at least for now). Second, it is reasonable that the build config may need to be constructed during the partition. Separating them to two APIs may introduce overheads and be more confusing to non TVMC users, which are still majority in this community.

I agree with @comaniac, I don't see a good way to separate this yet. The parameters used for parition_for_tensorrt affect both partitioning and building the module.

Fair enough. I thought a bit more about it and as with #7577, we will be already sending a config dictionary for the partition function. so I think it is ok to get the same, or a modified version of that dictionary as a result, which fits with the existing support on TensorRT.

I guess an alternative could be for the partitioning to be done within the pass context, and have the partition function set the config in the passcontext.

That's our ultimate goal of the composite target, which integrates partition and other codegen specific functions. Once we have it, the partition logic could be completely hidden from not only users but also frontend interface.

For now, I tried to receive and apply the config returned partiton_for_tensrort.

Yeah, I agree this is simpler and effective for now. Happy to approve once we get the linting issues sorted out and CI passing here.

comaniac

cc @trevor-m who is the main contributor of TensorRT integration.

comaniac · 2021-03-15T17:02:49Z

python/tvm/driver/tvmc/compiler.py

        if codegen["config_key"] is not None:
-            config[codegen["config_key"]] = codegen_from_cli["opts"]
+            config[codegen["config_key"]] = codegen_from_cli["opts"] or codegen_config


It means the generated codegen_config will be completely overwritten by the user config, but we should make it like

codegen_config.update(codegen_from_cli["opts"] if codegen_from_cli["opts"] is not None else {})

The config returned by partiton_for_tensorrt reflects the user config by thecodegen_from_cli argument.
And, the other composite_targets don't return config.
So, config with a extra_codegen can be overwritten codegen_config if any.

The config returned by partiton_for_tensorrt reflects the user config by the codegen_from_cli argument.

This is not guaranteed. What if partition_for_xxx only uses a part of the configs in codegen_from_cli?

Sorry, I don't know it means.

The implementation before this change use all of codegen_from_cli as the config.
If should consider only uses a part of partiton_for_xxx, it's due to not this change but original implementation.
Or, should it apply also in this change?

Sorry I misinterpreted the meaning of codegen_from_cli (their names are sort of confusing...).
Since codegen_from_cli is given by each composite target, this way should be fine.

comaniac · 2021-03-15T17:05:57Z

python/tvm/driver/tvmc/compiler.py

@@ -196,9 +196,9 @@ def compile_model(
    for codegen_from_cli in extra_targets:
        codegen = composite_target.get_codegen_by_target(codegen_from_cli["name"])
        partition_function = codegen["pass_pipeline"]
-        mod = partition_function(mod, params)
+        mod, codegen_config = partition_function(mod, params)


I agree that it would be easier to ask all pritition functions return a single module. However, the init function introduced by #7577 seems not applicable to this case to me. The config required by TensorRT is used when building the module; while the init function used by Vitis-AI imports the required package. I have no idea how could we make it general enough to support both Vitis-AI and TensorRT.

On the other hand, let a partition function return a tuple of module and build config seems more straightforward at the first glance, as I cannot think of a better idea for now. First, partition functions won't be a Relay pass in a pass sequence (at least for now). Second, it is reasonable that the build config may need to be constructed during the partition. Separating them to two APIs may introduce overheads and be more confusing to non TVMC users, which are still majority in this community.

leandron · 2021-04-08T08:39:09Z

@akmaru just a heads-up that #7577 is now merged and we can proceed with the changes proposed here.

akmaru · 2021-05-25T19:44:59Z

Sorry for my late fix, please review again!

comaniac

LGTM. Please fix the CI.

luohao123 · 2021-09-23T09:13:44Z

@akmaru Any updates on this?

comaniac · 2021-10-11T16:23:33Z

Gentle ping @akmaru please fix the CI

masahi · 2022-01-09T22:36:53Z

@akmaru can you update?

masahi assigned comaniac Mar 15, 2021

leandron reviewed Mar 15, 2021

View reviewed changes

comaniac reviewed Mar 15, 2021

View reviewed changes

Akira Maruoka and others added 3 commits May 24, 2021 17:05

Add TensorRT codegen registory

690beb1

Handle config generated by partiton_for_tensorrt

20a86ca

Test for TVMC compile with TensorRT integration

e5b1a92

akmaru force-pushed the tvmc-tensorrt-integration branch from 0b9945f to e5b1a92 Compare May 25, 2021 19:35

comaniac approved these changes May 25, 2021

View reviewed changes

Formatted

e666ac4

jroesch added the needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it label Jan 19, 2022

areusch added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it and removed needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it labels Oct 19, 2022

tqchen closed this Sep 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TVMC][TensorRT] Enable TensorRT target through TVMC #7658

[TVMC][TensorRT] Enable TensorRT target through TVMC #7658

akmaru commented Mar 15, 2021

leandron left a comment

leandron Mar 15, 2021 •

edited

Loading

comaniac Mar 15, 2021

trevor-m Mar 15, 2021

leandron Mar 15, 2021

trevor-m Mar 16, 2021

comaniac Mar 16, 2021

akmaru May 25, 2021

leandron Jun 1, 2021 •

edited

Loading

comaniac left a comment

comaniac Mar 15, 2021

akmaru May 25, 2021

comaniac May 25, 2021

akmaru May 25, 2021

comaniac May 25, 2021

comaniac Mar 15, 2021

leandron commented Apr 8, 2021

akmaru commented May 25, 2021

comaniac left a comment

luohao123 commented Sep 23, 2021

comaniac commented Oct 11, 2021

masahi commented Jan 9, 2022

[TVMC][TensorRT] Enable TensorRT target through TVMC #7658

[TVMC][TensorRT] Enable TensorRT target through TVMC #7658

Conversation

akmaru commented Mar 15, 2021

leandron left a comment

Choose a reason for hiding this comment

leandron Mar 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandron Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

comaniac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandron commented Apr 8, 2021

akmaru commented May 25, 2021

comaniac left a comment

Choose a reason for hiding this comment

luohao123 commented Sep 23, 2021

comaniac commented Oct 11, 2021

masahi commented Jan 9, 2022

leandron Mar 15, 2021 •

edited

Loading

leandron Jun 1, 2021 •

edited

Loading