Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bugfix] Visit each input param of the function in ExprVisitor visit_function #8521

Merged
merged 1 commit into from Jul 27, 2021
Merged

[Bugfix] Visit each input param of the function in ExprVisitor visit_function #8521

merged 1 commit into from Jul 27, 2021

Conversation

JoeyChou-SiMa-ai
Copy link
Contributor

In the ExprMutator the visit_function https://github.com/apache/tvm/blob/main/python/tvm/relay/expr_functor.py#L203will visit each input param while in ExprVisitor it doesn't https://github.com/apache/tvm/blob/main/python/tvm/relay/expr_functor.py#L155.

This PR added visitation for each input param in ExprVisitor's visit_function.

cc @comaniac

mbrookhart
mbrookhart approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

LGTM with that caveat that the python Visitor can only handle small models due to recursion limits in python. It's great for prototyping, but passes should more generally use C++.

@JoeyChou-SiMa-ai
Copy link
Contributor Author

LGTM with that caveat that the python Visitor can only handle small models due to recursion limits in python. It's great for prototyping, but passes should more generally use C++.

Thanks @mbrookhart for the quick response, will take your advice.

comaniac
comaniac approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM

@comaniac comaniac merged commit a492db8 into apache:main Jul 27, 2021
@comaniac
Copy link
Contributor

Thanks @JoeyChou-SiMa-ai @mbrookhart

ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
@JoeyChou-SiMa-ai @ylc
[Bugfix] Visit each input param of the function in ExprVisitor visit_…
e559b70 
…function (apache#8521)
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@JoeyChou-SiMa-ai @ylc
[Bugfix] Visit each input param of the function in ExprVisitor visit_…
095aab6 
…function (apache#8521)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookhart mbrookhart mbrookhart approved these changes

@comaniac comaniac comaniac approved these changes

@anijain2305 anijain2305 Awaiting requested review from anijain2305

@icemelon icemelon Awaiting requested review from icemelon

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@jwfromm jwfromm Awaiting requested review from jwfromm

@MarisaKirisame MarisaKirisame Awaiting requested review from MarisaKirisame

@slyubomirsky slyubomirsky Awaiting requested review from slyubomirsky

@vinx13 vinx13 Awaiting requested review from vinx13

@wweic wweic Awaiting requested review from wweic

@yzhliu yzhliu Awaiting requested review from yzhliu

@zhiics zhiics Awaiting requested review from zhiics

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

Assignees
No one assigned
Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JoeyChou-SiMa-ai @comaniac @mbrookhart