electriclilies commented on a change in pull request #8914:
URL: https://github.com/apache/tvm/pull/8914#discussion_r703742150



##########
File path: include/tvm/ir/module.h
##########
@@ -122,6 +122,7 @@ class IRModuleNode : public Object {
     v->Visit("global_var_map_", &global_var_map_);
     v->Visit("global_type_var_map_", &global_type_var_map_);
     v->Visit("source_map", &source_map);
+    v->Visit("attrs", &attrs);

Review comment:
       @manupa-arm I've been pushing the `Map<Target, IRModule>` further and 
further down the stack, with the goal of eventually removing it. Eventually, 
I'd like all the APIs to just consume a IRModule where each function is 
annotated with its target. 
   
   Right now, IRModule's attributes contains all the information needed to 
construct a LoweredOutput, and I have some conversion functions to extract that 
info before I stick it into the LoweredOutput. 
   
   You're correct that at this point I'm not copying the attributes into the 
`per_target_modules` IRModules.  I think that this is OK for now because 
`per_target_modules` only exists inside a `LoweredOutput`, and the consumers of 
`LoweredOutput` don't expect there to be attributes on the 
`per_target_modules`. Additionally, I'm going removing `LoweredOutput` (and 
`lowered_funcs`) during more follow up work.
   
   Unfortunately,`LoweredOutput` is deeply intertwined with the `build` API, so 
removing `LoweredOutput`is not a change I can make here. 
   
   My next step will be refactoring the `build` API with the goal of removing 
`LoweredOutput`. There's also a bunch of other cleanup that needs to be done on 
the `build` API that will be extensive. 
   




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to