-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[ARITH] detect iter affine map with predicate #7752
Conversation
Thanks @spectrometerHBH . @MasterJH5574 @hzfan @Hzfengsy please also help to take a look when you have time |
There was a problem hiding this 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
} | ||
|
||
/*! \brief Count the size of the PrimExpr. */ | ||
class PrimExprSizeCounter : public ExprVisitor { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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? |
There was a problem hiding this 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.
@tqchen I'm not sure whther current ExprComplexity is compatible with mine. Looks like current ExprComplexity doesn't count Nodes like Load/BufferLoad/Cast.... |
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 |
Thanks @spectrometerHBH @FrozenGene @MasterJH5574 this PR is now merged |
Update on previous PR #6667
Enhance the detect_iter_map utility to be able to detect split patterns with predicates.
cc @tqchen