-
Couldn't load subscription status.
- Fork 15k
[DA] Check monotonicity for subscripts #154527
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
a0d05ec
75639b9
5063efc
4dad091
8986cd4
dcb3b19
e93be2e
72b2902
b5ca793
65ccc61
d113571
137c193
f9b2a4e
e83fdb8
a098743
b16cc64
98a5de4
8c4f97b
c563a8f
55df30a
85129c6
e15715e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3308,6 +3308,322 @@ void DependenceInfo::updateDirection(Dependence::DVEntry &Level, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| llvm_unreachable("constraint has unexpected kind"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enum class MonotonicityType { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MaySignedWrap, ///< The expression contains arithmetic operations that may | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< cause signed wrap. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Constant, ///< The expression is constant. If a SCEV is classified as | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< Constant, it also implies that it doesn't contain any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< arithmetic operations that may cause signed wrap. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Monotonic, ///< The expression is monotonically increasing or decreasing. This | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< is exclusive of Constant. That is, we say an SCEV is Monotonic | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< iff it contains at least one AddRec where its step reccurence | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ///< value is non-zero. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// A visitor that checks the signed monotonicity of SCEVs. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| struct SCEVSignedMonotonicityChecker | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : public SCEVVisitor<SCEVSignedMonotonicityChecker, MonotonicityType> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// \p Ptr is the pointer that the SCEV is associated with, if any. It may be | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// used for the inferrence. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SCEVSignedMonotonicityChecker(ScalarEvolution *SE, const Loop *OutermostLoop, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const Value *Ptr = nullptr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : SE(SE), OutermostLoop(OutermostLoop), Ptr(Ptr) {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitAddExpr(const SCEVAddExpr *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitAddRecExpr(const SCEVAddRecExpr *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitMulExpr(const SCEVMulExpr *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitUnknown(const SCEVUnknown *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitZeroExtendExpr(const SCEVZeroExtendExpr *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitSignExtendExpr(const SCEVSignExtendExpr *Expr); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitConstant(const SCEVConstant *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::Constant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitVScale(const SCEVVScale *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::Constant; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: Handle more cases. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitPtrToIntExpr(const SCEVPtrToIntExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitTruncateExpr(const SCEVTruncateExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitUDivExpr(const SCEVUDivExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitSMaxExpr(const SCEVSMaxExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitUMaxExpr(const SCEVUMaxExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitSMinExpr(const SCEVSMinExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitUMinExpr(const SCEVUMinExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitSequentialUMinExpr(const SCEVSequentialUMinExpr *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MonotonicityType visitCouldNotCompute(const SCEVCouldNotCompute *) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return MonotonicityType::MaySignedWrap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ScalarEvolution *SE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const Loop *OutermostLoop; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ; The value of step reccurence is not invariant with respect to the outer most | |
| ; loop (the i-loop). | |
| ; | |
| ; offset_i = 0; | |
| ; for (int i = 0; i < 100; i++) { | |
| ; for (int j = 0; j < 100; j++) | |
| ; a[offset_i + j] = 0; | |
| ; offset_i += (i % 2 == 0) ? 0 : 3; | |
| ; } | |
| ; | |
| define void @step_is_variant(ptr %a) { | |
| ; CHECK-LABEL: 'step_is_variant' | |
| ; CHECK: Failed to prove monotonicity for: %offset.i | |
| ; CHECK: Failed to prove monotonicity for: {%offset.i,+,1}<nuw><nsw><%loop.j> | |
| ; CHECK: Monotonicity: Unknown expr: {%offset.i,+,1}<nuw><nsw><%loop.j> | |
| ; | |
| entry: | |
| br label %loop.i.header | |
| loop.i.header: | |
| %i = phi i64 [ 0, %entry ], [ %i.inc, %loop.i.latch ] | |
| %offset.i = phi i64 [ 0, %entry ], [ %offset.i.next, %loop.i.latch ] | |
| %step.i.0 = phi i64 [ 0, %entry ], [ %step.i.1, %loop.i.latch ] | |
| %step.i.1 = phi i64 [ 3, %entry ], [ %step.i.0, %loop.i.latch ] | |
| br label %loop.j | |
| loop.j: | |
| %j = phi i64 [ 0, %loop.i.header ], [ %j.inc, %loop.j ] | |
| %offset = add nsw i64 %offset.i, %j | |
| %idx = getelementptr inbounds i8, ptr %a, i64 %offset | |
| store i8 0, ptr %idx | |
| %j.inc = add nsw i64 %j, 1 | |
| %exitcond.j = icmp eq i64 %j.inc, 100 | |
| br i1 %exitcond.j, label %loop.i.latch, label %loop.j | |
| loop.i.latch: | |
| %i.inc = add nsw i64 %i, 1 | |
| %offset.i.next = add nsw i64 %offset.i, %step.i.0 | |
| %exitcond.i = icmp eq i64 %i.inc, 100 | |
| br i1 %exitcond.i, label %exit, label %loop.i.header | |
| exit: | |
| ret void | |
| } |
Outdated
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.
I dont understand one thing here. If the entire SCEV is NSW, why do we need to check if its NSW for individual operands? Do you have specific case in mind?
Also, I am trying to understand what MonotonicityType::Monotonic really means. Just going by the mathematical definition of monotonicity,
- why
Monotonic + Monotonicshould be MaySignedWrap? Shouldnt it be Monotonic? - why
Monotonic + Constantshould be Constant? Shouldnt it be Monotonic?
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.
First of all, probably the name is misleading. MaySignedWrap should be renamed to something like Unknown.
If the entire SCEV is NSW, why do we need to check if its NSW for individual operands? Do you have specific case in mind?
I was imagining an example like {0,+,%m}<%loop> + {0,+,%n}<%loop>, but I'm not sure if such a representation can actually exist. If ScalarEvolution guarantees that this form is always folded into something like {0,+,(%m+%n)}<%loop>, then maybe this is unnecessary. But if not, I'm not confident it's safe when each operand can potentially overflow (although DA doesn't support this kind of format)
Just going by the mathematical definition of monotonicity
DA breaks exactly due to the gap between mathematical theory and LLVM IR semantics.
- why
Monotonic + Monotonicshould be MaySignedWrap? Shouldnt it be Monotonic?
To clearly distinguish between Constant and Monotonic. I was considering a case like {0,+,1}<%loop> + {0,+,-1}<%loop>. This always seems to evaluate to 0 (i.e., a Constant), but again, I'm not sure if such a representation can exist in SCEV.
- why
Monotonic + Constantshould be Constant? Shouldnt it be Monotonic?
I think this is just a simple implementation mistake. Thanks for pointing it out.
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.
First of all, probably the name is misleading. MaySignedWrap should be renamed to something like Unknown
ok, please change to Unknown or CouldNotCompute. MaySignedWrap is confusing
example like {0,+,%m}<%loop> + {0,+,%n}<%loop>
If the entire expression is nuw/nsw then individual SCEVs must follow the same pattern but vice-versa cant be true(this may wrap).
that this form is always folded into something like {0,+,(%m+%n)}<%loop>
this is not true because when its split form, each AddRed can have different values . But with (%m+%n) , every itr is multiple of (%m+%n)
{0,+,1}<%loop> + {0,+,-1}<%loop>
this expr can have values 0(=0+0), -1(=0-1), 1(=1+0), 0(=1-1). This is definitely not a constant. So, this should be Unknown
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.
What I'm not entirely sure about is whether, given the following IR, the SCEV corresponding to %mn_i is guaranteed to be {0,+,(%m + %n)}<%loop> rather than {0,+,%m}<%loop> + {0,+,%n}<%loop>
loop:
%i = phi i64 [ 0, %entry ], [ %i.inc, %loop ]
%m_i = mul nsw i64 %m, %i
%n_i = mul nsw i64 %n, %i
%mn_i = add nsw i64 %m_i, %n_i
...If not, then I don't think we can say "Monotonic + Monotonic = Monotonic", since %m could be equal to -1 * %n, in which case %mn_i would always be zero. I don't know whether a constant value is considered to "monotonic" in general mathematical theory, but it is a corner case in the context of DA.
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.
if m is invariant in current loop then it should be {0,+,(%m + %n)}<%loop> and vice-versa if n comes from outer loop
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.
Okay, then I think the logic can be simplified.
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.
@kasuga-fj Expressions like {0,+,%m}<%loop> + {0,+,%n}<%loop> are possible if SCEV either hits the arithmetic depth limit, or the huge expression limit.
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.
@nikic I see, thanks for letting me know.
Outdated
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.
should this be IsNoWrap ?
Outdated
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.
shouldnt you check for NoWrap? The expression, if unsigned, may not fit into the signed range
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.
This is intentional. DA currently mixes signed and unsigned interpretations, which can lead to incorrect results. One of the main goals of this PR (and future ones) is to unify all integer interpretations in DA under a signed one.
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.
if it only has nuw, the analysis would go wrong.
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, and the expected behavior is to detect such expressions and bail out of the analysis. The subsequent checks attempt to prove properties similar to nsw when it's not explicitly attached, although I'm not sure they're truly necessary (I've not tested enough yet).
Outdated
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.
this is SCEVUnknown in the context of current loop. Why do we need to evaluate in the scope of outermost loop?
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.
Just to align with the current behavior.
llvm-project/llvm/lib/Analysis/DependenceAnalysis.cpp
Lines 854 to 867 in db02476
| // Returns true if Expression is loop invariant in LoopNest. | |
| bool DependenceInfo::isLoopInvariant(const SCEV *Expression, | |
| const Loop *LoopNest) const { | |
| // Unlike ScalarEvolution::isLoopInvariant() we consider an access outside of | |
| // any loop as invariant, because we only consier expression evaluation at a | |
| // specific position (where the array access takes place), and not across the | |
| // entire function. | |
| if (!LoopNest) | |
| return true; | |
| // If the expression is invariant in the outermost loop of the loop nest, it | |
| // is invariant anywhere in the loop nest. | |
| return SE->isLoopInvariant(Expression, LoopNest->getOutermostLoop()); | |
| } |
But I'm not sure whether checking invariance in the current loop is sufficient or not.
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.
think conversely. If this was MonotonicityType::Constant, would this function be ever called? It should still be MonotonicityType::MaySignedWrap
Outdated
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.
Found a more serious issue (ref: #157859). To keep this PR simpler, I now think it is better to ignore the delinearized subscripts at the moment, and check monotonicity only when delinearization fails...
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.
done
Outdated
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.
Same comment. Why evaluate only in the scope of Outermost loop? Shouldnt this be in context of current loop?
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.
What is the utility of
Constant?isa<SCEVConstant>(...)should be sufficient. Did you mean "invariant" to a specific loop?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, "invariant" seems more reasonable here.