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.

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

[ARITH] detect iter affine map with predicate #7752

Merged
merged 6 commits into from
Mar 27, 2021
Merged

[ARITH] detect iter affine map with predicate #7752

merged 6 commits into from
Mar 27, 2021

Conversation

spectrometerHBH
Copy link
Contributor

Update on previous PR #6667

Enhance the detect_iter_map utility to be able to detect split patterns with predicates.

cc @tqchen

@tqchen
Copy link
Member

tqchen commented Mar 26, 2021

Thanks @spectrometerHBH . @MasterJH5574 @hzfan @Hzfengsy please also help to take a look when you have time

@tqchen tqchen self-assigned this Mar 26, 2021
Copy link
Member

@tqchen tqchen left a comment

Choose a reason for hiding this comment

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

some minor comments

src/arith/iter_affine_map.cc Outdated Show resolved Hide resolved
}

/*! \brief Count the size of the PrimExpr. */
class PrimExprSizeCounter : public ExprVisitor {
Copy link
Member

@tqchen tqchen Mar 26, 2021

Choose a reason for hiding this comment

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

move to src/analysis/expr_complexity.cc analysis.h and expose as a function size_t ExprComplexity(const PrimExpr& expr); document as number of expressions in the child

Copy link
Member

Choose a reason for hiding this comment

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

Yes. In my previous pr #7469 I have done something. We could borrow it. My pr has the left problem of max ops counter, however, it is not an urgent problem IMO, which just a more safer guard for complex expr analysis.

tests/python/unittest/test_arith_iter_affine_map.py Outdated Show resolved Hide resolved
@tqchen
Copy link
Member

tqchen commented Mar 26, 2021

Thanks @spectrometerHBH There seems to be three places where the ExprComplexity is involved.

Given your PrimExprSizeCounter counting is simpler, can we create an ExprComplexity function in analysis and call from there?

@tqchen
Copy link
Member

tqchen commented Mar 26, 2021

cc @FrozenGene @yzhliu

@tqchen tqchen added the status: need update need update based on feedbacks label Mar 26, 2021
Copy link
Contributor

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

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

Thanks @spectrometerHBH! Overall LGTM. Just some nits.

src/arith/iter_affine_map.cc Outdated Show resolved Hide resolved
src/arith/iter_affine_map.cc Outdated Show resolved Hide resolved
@spectrometerHBH
Copy link
Contributor Author

spectrometerHBH commented Mar 27, 2021

Thanks @spectrometerHBH There seems to be three places where the ExprComplexity is involved.

Given your PrimExprSizeCounter counting is simpler, can we create an ExprComplexity function in analysis and call from there?

@tqchen I'm not sure whther current ExprComplexity is compatible with mine. Looks like current ExprComplexity doesn't count Nodes like Load/BufferLoad/Cast....

@tqchen
Copy link
Member

tqchen commented Mar 27, 2021

Thanks @spectrometerHBH I think calculating all exprs is the right def. Skipping load do makes sense for index expressions, but my guess is that we can always try the other way if it turns out to be not good

@tqchen tqchen merged commit dc81767 into apache:main Mar 27, 2021
@tqchen
Copy link
Member

tqchen commented Mar 27, 2021

Thanks @spectrometerHBH @FrozenGene @MasterJH5574 this PR is now merged

@spectrometerHBH spectrometerHBH deleted the iter-affine-map-predicate branch March 27, 2021 13:18
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: need review status: need update need update based on feedbacks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants