areusch commented on code in PR #12066:
URL: https://github.com/apache/tvm/pull/12066#discussion_r919247564


##########
include/tvm/ir/module.h:
##########
@@ -64,6 +65,8 @@ class IRModuleNode : public Object {
   /* \brief Additional attributes storing meta-data about the module. */
   DictAttrs attrs;
 
+  GlobalVarSupply global_var_supply;

Review Comment:
   > Given that we can recreate GlobalVarSupply from the current IRModule
   
   I'm not sure this is so trivial. A couple of cases to think through:
   - right now we still lower per-backend separately, so the NameSupply needs 
to track names across a collection of IRModules in the compiler
   - Suppose we needed to reserve an extern name not mentioned in the IR 
anywhere. We would surely need to serialize something to track that.
   
   The first case here seems to be the one most at tension with the idea of 
making NameSupply idempotent. I agree that NameSupply isn't necessarily 
attached to any given IRModule now, so having a way to access it should mainly 
be a convenience method and not participate in IRModule serialization at least 
right now. I'm not so sure it means that it's valid to serialize an IRModule 
and destroy a TVM instance, then load it back in another TVM instance and 
expect the NameSupply to be the same.



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