[GitHub] [tvm] areusch commented on a diff in pull request #12066: Unify name mangling in TVM

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

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: commits-unsubscribe@tvm.apache.org

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