Skip to content

Conversation

@slyubomirsky
Copy link
Contributor

A small follow-up to #15140, which was merged before I could make this addition: This change throws an error if a function has been annotated as private but the user manually specifies the global symbol. Since it is a contradiction to do so, it is best to detect this error rather than have it silently work (with the effect of actually making the function public).

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jun 27, 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

1 similar comment
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jun 27, 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

@junrushao
Copy link
Member

junrushao commented Jun 27, 2023

This is a really cool addition! A bit off the topic: do you think we should sugar the global_symbol attribute to make it part of the decorator? For example,

@R.function(global_symbol=True) # implies the global symbol name as "my_relax_func"
def my_relax_func: ...
@I.prim_func(global_symbol=True) # implies the global symbol name as "my_relax_func"
def my_tir_func: ...
@I.prim_func # implies the private symbol
def my_private_func: ...

@slyubomirsky
Copy link
Contributor Author

slyubomirsky commented Jun 27, 2023

Having syntax sugar for global symbol might make sense. However, as established in the last PR, I think we would prefer for the default not to be private

@tqchen Any thoughts on this suggestion?

@junrushao
Copy link
Member

in fact default not being private sounds better to me :-)

@tqchen
Copy link
Member

tqchen commented Jun 27, 2023

I agree, we would go for default not private

@slyubomirsky slyubomirsky merged commit 0dba0e3 into apache:unity Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants