-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
release/19.x: [IndVars] Check if WideInc available before trying to use it #106892
Conversation
@llvm/pr-subscribers-llvm-transforms Author: None (llvmbot) ChangesBackport c9a5e1b Requested by: @nikic Full diff: https://github.com/llvm/llvm-project/pull/106892.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index 5bda7c50c62c66..0b4a75e0bc52de 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -1928,18 +1928,24 @@ Instruction *WidenIV::widenIVUse(WidenIV::NarrowIVDefUse DU,
if (!WideAddRec.first)
return nullptr;
- // Reuse the IV increment that SCEVExpander created. Recompute flags, unless
- // the flags for both increments agree and it is safe to use the ones from
- // the original inc. In that case, the new use of the wide increment won't
- // be more poisonous.
- bool NeedToRecomputeFlags =
- !SCEVExpander::canReuseFlagsFromOriginalIVInc(OrigPhi, WidePhi,
- DU.NarrowUse, WideInc) ||
- DU.NarrowUse->hasNoUnsignedWrap() != WideInc->hasNoUnsignedWrap() ||
- DU.NarrowUse->hasNoSignedWrap() != WideInc->hasNoSignedWrap();
+ auto CanUseWideInc = [&]() {
+ if (!WideInc)
+ return false;
+ // Reuse the IV increment that SCEVExpander created. Recompute flags,
+ // unless the flags for both increments agree and it is safe to use the
+ // ones from the original inc. In that case, the new use of the wide
+ // increment won't be more poisonous.
+ bool NeedToRecomputeFlags =
+ !SCEVExpander::canReuseFlagsFromOriginalIVInc(
+ OrigPhi, WidePhi, DU.NarrowUse, WideInc) ||
+ DU.NarrowUse->hasNoUnsignedWrap() != WideInc->hasNoUnsignedWrap() ||
+ DU.NarrowUse->hasNoSignedWrap() != WideInc->hasNoSignedWrap();
+ return WideAddRec.first == WideIncExpr &&
+ Rewriter.hoistIVInc(WideInc, DU.NarrowUse, NeedToRecomputeFlags);
+ };
+
Instruction *WideUse = nullptr;
- if (WideAddRec.first == WideIncExpr &&
- Rewriter.hoistIVInc(WideInc, DU.NarrowUse, NeedToRecomputeFlags))
+ if (CanUseWideInc())
WideUse = WideInc;
else {
WideUse = cloneIVUser(DU, WideAddRec.first);
diff --git a/llvm/test/Transforms/IndVarSimplify/pr106239.ll b/llvm/test/Transforms/IndVarSimplify/pr106239.ll
new file mode 100644
index 00000000000000..8d5aa99539a5a7
--- /dev/null
+++ b/llvm/test/Transforms/IndVarSimplify/pr106239.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=indvars < %s | FileCheck %s
+
+target datalayout = "n8:16:32:64"
+
+; Make sure it does not crash.
+
+define i32 @m() {
+; CHECK-LABEL: define i32 @m() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br label %[[FOR_BODY_I6:.*]]
+; CHECK: [[FOR_BODY_I6]]:
+; CHECK-NEXT: br i1 true, label %[[I_EXIT:.*]], label %[[IF_END_I:.*]]
+; CHECK: [[IF_END_I]]:
+; CHECK-NEXT: store i64 0, ptr null, align 8
+; CHECK-NEXT: br label %[[FOR_BODY_I6]]
+; CHECK: [[I_EXIT]]:
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ %div.i4 = sdiv i32 1, 0
+ br label %for.body.i6
+
+for.body.i6: ; preds = %if.end.i, %entry
+ %add57.i = phi i32 [ %add.i7, %if.end.i ], [ 0, %entry ]
+ br i1 true, label %i.exit, label %if.end.i
+
+if.end.i: ; preds = %for.body.i6
+ %add.i7 = add i32 %add57.i, %div.i4
+ %conv.i = zext i32 %add57.i to i64
+ store i64 %conv.i, ptr null, align 8
+ br label %for.body.i6
+
+i.exit: ; preds = %for.body.i6
+ ret i32 0
+}
|
@shafik @AaronBallman is this good to be merged? |
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 think you may have intended to tag someone other than Shafik and I for this one, but the changes LGTM nonetheless.
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.
LGTM looks like a safe cherry-pick
WideInc/WideIncExpr can be null. Previously this worked out because the comparison with WideIncExpr would fail. Now we have accesses to WideInc prior to that. Avoid the issue with an explicit check. Fixes llvm#106239. (cherry picked from commit c9a5e1b)
@nikic (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Backport c9a5e1b
Requested by: @nikic