areusch commented on a change in pull request #7785:
URL: https://github.com/apache/tvm/pull/7785#discussion_r618529957
##########
File path: src/relay/backend/build_module.cc
##########
@@ -473,23 +517,25 @@ class RelayBuildModule : public runtime::ModuleNode {
// Relay IRModule -> IRModule optimizations.
relay_module = Optimize(relay_module, targets_, params);
+
// Get the updated function.
auto func = Downcast<Function>(relay_module->Lookup("main"));
// Generate code for the updated function.
- graph_codegen_ = std::unique_ptr<GraphCodegen>(new GraphCodegen());
- graph_codegen_->Init(nullptr, targets_);
- graph_codegen_->Codegen(func);
-
- ret_.graph_json = graph_codegen_->GetJSON();
- ret_.params = graph_codegen_->GetParams();
+ const String executor_str =
+ target_host->GetAttr<String>("executor").value_or(kTvmExecutorGraph);
Review comment:
i agree this isn't too invasive now, the main question is whether
`executor` should become a top-level `tvm.relay.build` parameter or whether
e.g. `runtime_config` or a similarly named struct should be there instead. I
feel like at least these parameters may not belong with target (and there are
probably more, these just come to mind):
- executor
- runtime
- link-params (this one is arguable, but I don't think link-params fully
describes the param placement)
I think it would be great to have another PR which addresses the question
of: how do we model runtime config in TVM targets, and how should changes there
affect autotuning schedule keys? The last bit is the contentious part. For this
PR, perhaps we could take the approach I did with runtime: define it as a
Target attribute with no default, and elsewhere make the default when
unspecified kTvmExecutorGraph. This way, in the usual non-aot case, we don't
affect tuning logs. wdyt?
##########
File path: src/relay/backend/build_module.cc
##########
@@ -473,23 +517,25 @@ class RelayBuildModule : public runtime::ModuleNode {
// Relay IRModule -> IRModule optimizations.
relay_module = Optimize(relay_module, targets_, params);
+
// Get the updated function.
auto func = Downcast<Function>(relay_module->Lookup("main"));
// Generate code for the updated function.
- graph_codegen_ = std::unique_ptr<GraphCodegen>(new GraphCodegen());
- graph_codegen_->Init(nullptr, targets_);
- graph_codegen_->Codegen(func);
-
- ret_.graph_json = graph_codegen_->GetJSON();
- ret_.params = graph_codegen_->GetParams();
+ const String executor_str =
+ target_host->GetAttr<String>("executor").value_or(kTvmExecutorGraph);
Review comment:
cc @jroesch @tqchen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
