Skip to content

Conversation

@kparzysz-quic
Copy link
Contributor

In the same way as T.macro, R.macro support hygiene (via "hygienic" keyword), but unlike TIR macros, Relax macros produce expressions, and have to return a value: the last statement in R.macro must be "return".

@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 1, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

3 similar comments
@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 1, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 1, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 1, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@kparzysz-quic
Copy link
Contributor Author

cc: @cyx-6 @yzh119 @slyubomirsky @masahi

@junrushao
Copy link
Member

CC @slyubomirsky if you’d love to review

@slyubomirsky
Copy link
Contributor

I'll review, sorry I didn't see the pings.

@slyubomirsky
Copy link
Contributor

slyubomirsky commented Aug 11, 2023

Can macros take in Relax vars as arguments and define new Relax vars? That would be an important test case for hygienic macros. Otherwise, everything in this PR looks good to me.

edit: I think this case should simply work, but we should make sure of it. Relax var identity is determined by pointer, so creating new vars inside a macro should "just work."

@junrushao
Copy link
Member

Any updates?

@kparzysz-quic
Copy link
Contributor Author

Sorry, I was on vacation in the last two weeks.

Relax macros are technically SeqExprs, so they cannot define variables that are visible outside. The value of the macro is the last expression, which syntactically must be wrapped in a return expression.

In the same way as T.macro, R.macro support hygiene (via "hygienic"
keyword), but unlike TIR macros, Relax macros produce expressions,
and have to return a value: the last statement in R.macro must be
"return".
@kparzysz-quic
Copy link
Contributor Author

Rebased, and added a test for using (outside of a macro) a variable defined in a macro.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Looking good to me! @slyubomirsky would you like to take a second look?

@junrushao junrushao merged commit 71c6839 into apache:unity Sep 2, 2023
@kparzysz-quic kparzysz-quic deleted the unity-macro branch September 4, 2023 14:57
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.

4 participants