[GitHub] [tvm] Mousius commented on a change in pull request #8886: Remove LoweredModule

[GitBox <gi...@apache.org>]

Mousius commented on a change in pull request #8886:
URL: https://github.com/apache/tvm/pull/8886#discussion_r700876166



##########
File path: src/relay/backend/graph_executor_codegen.cc
##########
@@ -270,8 +274,13 @@ class GraphExecutorCodegen : public backend::MemoizedExprTranslator<std::vector<
           std::make_pair(static_cast<int>(param_storage_ids_[param.first]), param.second)));
     }
     ret.function_metadata = std::move(function_metadata_);
-    ret.lowered_funcs = lowered_module.per_target_module;
-    ret.external_mods = lowered_module.external_mods;
+
+    Optional<Array<tvm::runtime::Module>> external_modules =
+        lowered_mod->GetAttr<Array<tvm::runtime::Module>>("external_mods");
+    // This is the point where we separate the functions in the module by target

Review comment:
       Do we not need the same `ICHECK(external_modules) << "Attribute \"external_modules\" should be set at this point.";` here as in AOT? I'm always in favour of not including the checks if we have no real way to trigger it given we know the TE Compilers behaviour, but I'd suggest we should be consistent.

##########
File path: src/relay/backend/te_compiler.h
##########
@@ -174,27 +158,19 @@ void UpdateFunctionMetadata(Function relay_func,
  */
 Target GetTargetFromInteger(DLDeviceType dev_type, TargetMap targets);
 
-/*! \brief Utility to convert a LoweredModule to an IRModule.
+/*! \brief Utility to separate the the functions in an IRModule by Target.

Review comment:
       ```suggestion
   /*! \brief Utility to separate the functions in an IRModule by Target.
   ```




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org