Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, the post-parsing well-formed check performed by TVMScript allowed a call to GlobalVar in a @R.function, but only if it occurred within the context of a @I.ir_module. If @R.function appeared on its own, calls to a GlobalVar would be treated as calls to an undefined function.

@Lunderberg Lunderberg requested a review from slyubomirsky March 24, 2024 18:38
Prior to this commit, the post-parsing well-formed check performed by
TVMScript allowed a call to `GlobalVar` in a `@R.function`, but only
if it occurred within the context of a `@I.ir_module`.  If
`@R.function` appeared on its own, calls to a `GlobalVar` would be
treated as calls to an undefined function.
@Lunderberg Lunderberg force-pushed the analysis_global_var_in_bare_relax_function branch from 013848e to 25a0e31 Compare March 25, 2024 13:37
Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting case, one that did not occur to me. I think it's fine to have a separate rule for analyzing a standalone function compared to an entire module. The restructuring also cleans up some of the code in the parser, so I'm for it.

@Lunderberg Lunderberg merged commit 72f0326 into apache:main Mar 26, 2024
@Lunderberg Lunderberg deleted the analysis_global_var_in_bare_relax_function branch March 26, 2024 13:03
thaisacs pushed a commit to thaisacs/tvm that referenced this pull request Apr 3, 2024
* [Analysis] Allow calls to GlobalVar in @R.function

Prior to this commit, the post-parsing well-formed check performed by
TVMScript allowed a call to `GlobalVar` in a `@R.function`, but only
if it occurred within the context of a `@I.ir_module`.  If
`@R.function` appeared on its own, calls to a `GlobalVar` would be
treated as calls to an undefined function.

* Use approrpirate well-formed checks TIR/Relax functions

* Lint fix

* Import order fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants