Ubospica commented on code in PR #16090:
URL: https://github.com/apache/tvm/pull/16090#discussion_r1388695896


##########
python/tvm/relax/block_builder.py:
##########
@@ -661,12 +661,20 @@ def normalize(self, expr: Expr) -> Expr:
     def get(self) -> tvm.IRModule:
         """Return the IRModule being built.
 
+        Note this call may invalidate global vars previously returned by this 
builder
+        (see tvm.relax.transform.NormalizeGlobalVar), so it is recommended to 
call this function
+        once at the end of the build process.
+
         Returns
         -------
         ret : tvm.IRModule
             An IRModule with Relax and TIR functions being built.
         """
-        return _ffi_api.BlockBuilderGetContextIRModule(self)  # type: ignore
+        # avoid circular import
+        from .transform import NormalizeGlobalVar
+
+        mod = _ffi_api.BlockBuilderGetContextIRModule(self)  # type: ignore

Review Comment:
   For the cost of renormalization, currently if no function needs renaming, 
the pass will return directly and not get into the AST. In this case I think 
the cost is not that high.
   
   The wellformed check is already added.
   
   And for `bb.finalize()`, I think this will make it confused to use 
`bb.finalize()` or `bb.get()`, because their only difference is whether apply 
NormalizeGlobalVar or not. If we require users only use `bb.finalize()` at the 
end of the IRModule building process, that means a major behaviour change and 
all related tests might need to be changed.



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