[SCEV] Restrict wrap flags when construction same AR for multiple phis. #80430
+88
−37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds a set of tests taken from/llvm/test/Transforms/IndVarSimplify/iv-poison.ll with multiple congruent IVs but different set of flags on the increments.
When constructing AddRecs from multiple equivalent phi nodes modulo increment wrap flags, the resulting AddRec has the union of all increment wrap flags as no-wrap flags. This seems incorrect: uses of this AddRec are more poisonous in contexts where an original phi node didn't have an increment with wrap flags. This patch tries to ensures that the wrap flags for the constructed AddRecs are valid for all phis that get mapped to an AddRec. To do so, first check if there's already an existing AddRec. In that case, the available wrap-flags are an upper bound on the wrap-flags; additional wrap flags may make existing uses more poisonous. If the AddRec can never be poison, it should still be safe to use the full IR flags from IR. Note the change in @iv_hoist_nuw_poison_extra_use fixes a mis-compile found by Alive2.
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesWhen constructing AddRecs from multiple equivalent phi nodes modulo This seems incorrect: uses of this AddRec are more poisonous in contexts This patch tries to ensures that the wrap flags for the constructed If the AddRec can never be poison, it should still be safe to use the Note the change in @iv_hoist_nuw_poison_extra_use fixes a mis-compile Patch is 27.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80430.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index af3ad822e0b0d..2cad21b8b2a4f 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -563,6 +563,11 @@ class ScalarEvolution {
/// Return an existing SCEV for V if there is one, otherwise return nullptr.
const SCEV *getExistingSCEV(Value *V);
+ /// Return the AddRec for \p Ops and \p L if there is one, otherwise return
+ /// nullptr.
+ const SCEVAddRecExpr *getExistingAddRecExpr(ArrayRef<const SCEV *> Ops,
+ const Loop *L);
+
const SCEV *getConstant(ConstantInt *V);
const SCEV *getConstant(const APInt &Val);
const SCEV *getConstant(Type *Ty, uint64_t V, bool isSigned = false);
@@ -2124,8 +2129,10 @@ class ScalarEvolution {
/// This is like \c isSCEVExprNeverPoison but it specifically works for
/// instructions that will get mapped to SCEV add recurrences. Return true
/// if \p I will never generate poison under the assumption that \p I is an
- /// add recurrence on the loop \p L.
- bool isAddRecNeverPoison(const Instruction *I, const Loop *L);
+ /// add recurrence on the loop \p L. Don't call \c isSCEVExprNeverPoison if \p
+ /// CheckSCEVScope is false.
+ bool isAddRecNeverPoison(const Instruction *I, const Loop *L,
+ bool CheckSCEVScope = true);
/// Similar to createAddRecFromPHI, but with the additional flexibility of
/// suggesting runtime overflow checks in case casts are encountered.
diff --git a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
index fd884f2a2f55b..fcfcb1c5af879 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
@@ -392,6 +392,10 @@ class SCEVAddRecExpr : public SCEVNAryExpr {
SubclassData |= Flags;
}
+ /// Set the flags for the recurrence to \p Flags. This overwrites any existing
+ /// flags.
+ void forceSetNoWrapFlags(NoWrapFlags Flags) { SubclassData = Flags; }
+
/// Return the value of this chain of recurrences at the specified
/// iteration number.
const SCEV *evaluateAtIteration(const SCEV *It, ScalarEvolution &SE) const;
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2acb45837c480..e87770e064f8a 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4520,6 +4520,18 @@ const SCEV *ScalarEvolution::getExistingSCEV(Value *V) {
return nullptr;
}
+const SCEVAddRecExpr *
+ScalarEvolution::getExistingAddRecExpr(ArrayRef<const SCEV *> Ops,
+ const Loop *L) {
+ FoldingSetNodeID ID;
+ ID.AddInteger(scAddRecExpr);
+ for (const SCEV *Op : Ops)
+ ID.AddPointer(Op);
+ ID.AddPointer(L);
+ void *IP = nullptr;
+ return static_cast<SCEVAddRecExpr *>(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
+}
+
/// Return a SCEV corresponding to -V = -1*V
const SCEV *ScalarEvolution::getNegativeSCEV(const SCEV *V,
SCEV::NoWrapFlags Flags) {
@@ -5703,19 +5715,46 @@ const SCEV *ScalarEvolution::createSimpleAffineAddRec(PHINode *PN,
return nullptr;
SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
- if (BO->IsNUW)
+ if (BO->IsNUW) {
Flags = setFlags(Flags, SCEV::FlagNUW);
- if (BO->IsNSW)
+ Flags = setFlags(Flags, SCEV::FlagNW);
+ }
+ if (BO->IsNSW) {
Flags = setFlags(Flags, SCEV::FlagNSW);
+ Flags = setFlags(Flags, SCEV::FlagNW);
+ }
const SCEV *StartVal = getSCEV(StartValueV);
+ SCEV::NoWrapFlags MinFlags = Flags;
+ auto *ExistingAR = getExistingAddRecExpr({StartVal, Accum}, L);
+ // If there's already an AddRec, use its flags to compute the minimal valid
+ // flags that hold for the users of the existing AddRec and the users of the
+ // current phi we are constructing an AddRec for.
+ if (ExistingAR)
+ MinFlags = maskFlags(ExistingAR->getNoWrapFlags(), Flags);
+
const SCEV *PHISCEV = getAddRecExpr(StartVal, Accum, L, Flags);
insertValueToMap(PN, PHISCEV);
+ // Check if the AddRec can never be poison, but avoid constructing new SCEVs
+ // for PN's operands, as this would instantiate the SCEV for the increment
+ // earlier.
+ bool NeverPoison = isAddRecNeverPoison(PN, L, false);
+ // If the AddRec is never poison, it is safe the use the flags from the IR
+ // unconditionally. But if it may be poison, force the AddRec's flags to the
+ // minimum valid set for both the existing AddRec and the current context.
+ if (!NeverPoison && ExistingAR) {
+ const_cast<SCEVAddRecExpr *>(ExistingAR)->forceSetNoWrapFlags(MinFlags);
+ UnsignedRanges.erase(ExistingAR);
+ SignedRanges.erase(ExistingAR);
+ ConstantMultipleCache.erase(ExistingAR);
+ }
+
if (auto *AR = dyn_cast<SCEVAddRecExpr>(PHISCEV)) {
- setNoWrapFlags(const_cast<SCEVAddRecExpr *>(AR),
- (SCEV::NoWrapFlags)(AR->getNoWrapFlags() |
- proveNoWrapViaConstantRanges(AR)));
+ SCEV::NoWrapFlags FlagsViaConstantRanges = proveNoWrapViaConstantRanges(AR);
+ setNoWrapFlags(
+ const_cast<SCEVAddRecExpr *>(AR),
+ (SCEV::NoWrapFlags)(AR->getNoWrapFlags() | FlagsViaConstantRanges));
}
// We can add Flags to the post-inc expression only if we
@@ -7269,9 +7308,10 @@ bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
return isGuaranteedToTransferExecutionTo(DefI, I);
}
-bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
+bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L,
+ bool CheckSCEVScope) {
// If we know that \c I can never be poison period, then that's enough.
- if (isSCEVExprNeverPoison(I))
+ if (CheckSCEVScope && isSCEVExprNeverPoison(I))
return true;
// If the loop only has one exit, then we know that, if the loop is entered,
diff --git a/llvm/test/Analysis/ScalarEvolution/iv-poison.ll b/llvm/test/Analysis/ScalarEvolution/iv-poison.ll
new file mode 100644
index 0000000000000..03fa233930cdf
--- /dev/null
+++ b/llvm/test/Analysis/ScalarEvolution/iv-poison.ll
@@ -0,0 +1,399 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 4
+; RUN: opt "-passes=print<scalar-evolution>" -disable-output -S %s 2>&1 | FileCheck %s
+
+; PR59777
+define i2 @iv_nsw_poison(i2 %arg) {
+; CHECK-LABEL: 'iv_nsw_poison'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison
+; CHECK-NEXT: %.07 = phi i2 [ 1, %bb ], [ %i, %bb1 ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%bb1> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %.0 = phi i2 [ 1, %bb ], [ %i2, %bb1 ]
+; CHECK-NEXT: --> {1,+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i = add nsw i2 %.07, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%bb1> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i2 = add i2 %.0, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%bb1> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison
+; CHECK-NEXT: Loop %bb1: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb1, %bb
+ %.07 = phi i2 [ 1, %bb ], [ %i, %bb1 ]
+ %.0 = phi i2 [ 1, %bb ], [ %i2, %bb1 ]
+ %i = add nsw i2 %.07, 1
+ %i2 = add i2 %.0, 1
+ %.not.not = icmp ult i2 %.07, %arg
+ br i1 %.not.not, label %common.ret, label %bb1
+
+common.ret: ; preds = %bb1
+ ret i2 %i2
+}
+
+define i4 @iv_nsw_poison2(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nsw_poison2'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison2
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison2
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ %iv.1.next = add nsw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i2 @iv_both_adds_nsw(i2 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw
+; CHECK-NEXT: %iv.0 = phi i2 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i2 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i2 %iv.0, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%loop> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i2 %iv.1, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%loop> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i2 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i2 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.0.next = add nsw i2 %iv.0, 1
+ %iv.1.next = add nsw i2 %iv.1, 1
+ %.not.not = icmp ult i2 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i2 %iv.1.next
+}
+
+define i4 @iv_both_adds_nsw_extra_use(i4 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw_extra_use'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw_extra_use
+; CHECK-NEXT: %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i4 %iv.0, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw_extra_use
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.0.next = add nsw i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %iv.1.next = add nsw i4 %iv.1, 1
+ call void @use(i4 %iv.1.next)
+ %.not.not = icmp ult i4 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i4 @iv_both_adds_nsw_extra_use_incs_reordered(i4 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw_extra_use_incs_reordered'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw_extra_use_incs_reordered
+; CHECK-NEXT: %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i4 %iv.0, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw_extra_use_incs_reordered
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.1.next = add nsw i4 %iv.1, 1
+ call void @use(i4 %iv.1.next)
+ %iv.0.next = add nsw i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %.not.not = icmp ult i4 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i4 @iv_nsw_poison_extra_use(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nsw_poison_extra_use'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison_extra_use
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison_extra_use
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %iv.1.next = add nsw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+declare void @use(i4)
+
+define i2 @iv_nuw_poison(i2 %arg, i2 %start) {
+; CHECK-LABEL: 'iv_nuw_poison'
+; CHECK-NEXT: Classifying expressions for: @iv_nuw_poison
+; CHECK-NEXT: %.07 = phi i2 [ %start, %bb ], [ %i, %bb1 ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %.0 = phi i2 [ %start, %bb ], [ %i2, %bb1 ]
+; CHECK-NEXT: --> {%start,+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i = add nuw i2 %.07, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i2 = add i2 %.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nuw_poison
+; CHECK-NEXT: Loop %bb1: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb1, %bb
+ %.07 = phi i2 [ %start, %bb ], [ %i, %bb1 ]
+ %.0 = phi i2 [ %start, %bb ], [ %i2, %bb1 ]
+ %i = add nuw i2 %.07, 1
+ %i2 = add i2 %.0, 1
+ %.not.not = icmp ult i2 %.07, %arg
+ br i1 %.not.not, label %common.ret, label %bb1
+
+common.ret: ; preds = %bb1
+ ret i2 %i2
+}
+
+define i4 @iv_nuw_poison2(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nuw_poison2'
+; CHECK-NEXT: Classifying expressions for: @iv_nuw_poison2
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nuw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nuw_poison2
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ %iv.1.next = add nuw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i2 @iv_both_adds_nuw(i2 %arg, i2 %start) {
+; CHECK-LABEL: 'iv_both_adds_nuw'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nuw
+; CHECK-NEXT: %iv.0 = phi i2 [ %start, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i2 [ %start, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nuw i2 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<nw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nuw i2 %iv.1, 1
+; CHECK-NEXT: --> {(1 +...
[truncated]
|
|
@llvm/pr-subscribers-llvm-analysis Author: Florian Hahn (fhahn) ChangesWhen constructing AddRecs from multiple equivalent phi nodes modulo This seems incorrect: uses of this AddRec are more poisonous in contexts This patch tries to ensures that the wrap flags for the constructed If the AddRec can never be poison, it should still be safe to use the Note the change in @iv_hoist_nuw_poison_extra_use fixes a mis-compile Patch is 27.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80430.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index af3ad822e0b0d..2cad21b8b2a4f 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -563,6 +563,11 @@ class ScalarEvolution {
/// Return an existing SCEV for V if there is one, otherwise return nullptr.
const SCEV *getExistingSCEV(Value *V);
+ /// Return the AddRec for \p Ops and \p L if there is one, otherwise return
+ /// nullptr.
+ const SCEVAddRecExpr *getExistingAddRecExpr(ArrayRef<const SCEV *> Ops,
+ const Loop *L);
+
const SCEV *getConstant(ConstantInt *V);
const SCEV *getConstant(const APInt &Val);
const SCEV *getConstant(Type *Ty, uint64_t V, bool isSigned = false);
@@ -2124,8 +2129,10 @@ class ScalarEvolution {
/// This is like \c isSCEVExprNeverPoison but it specifically works for
/// instructions that will get mapped to SCEV add recurrences. Return true
/// if \p I will never generate poison under the assumption that \p I is an
- /// add recurrence on the loop \p L.
- bool isAddRecNeverPoison(const Instruction *I, const Loop *L);
+ /// add recurrence on the loop \p L. Don't call \c isSCEVExprNeverPoison if \p
+ /// CheckSCEVScope is false.
+ bool isAddRecNeverPoison(const Instruction *I, const Loop *L,
+ bool CheckSCEVScope = true);
/// Similar to createAddRecFromPHI, but with the additional flexibility of
/// suggesting runtime overflow checks in case casts are encountered.
diff --git a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
index fd884f2a2f55b..fcfcb1c5af879 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolutionExpressions.h
@@ -392,6 +392,10 @@ class SCEVAddRecExpr : public SCEVNAryExpr {
SubclassData |= Flags;
}
+ /// Set the flags for the recurrence to \p Flags. This overwrites any existing
+ /// flags.
+ void forceSetNoWrapFlags(NoWrapFlags Flags) { SubclassData = Flags; }
+
/// Return the value of this chain of recurrences at the specified
/// iteration number.
const SCEV *evaluateAtIteration(const SCEV *It, ScalarEvolution &SE) const;
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2acb45837c480..e87770e064f8a 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4520,6 +4520,18 @@ const SCEV *ScalarEvolution::getExistingSCEV(Value *V) {
return nullptr;
}
+const SCEVAddRecExpr *
+ScalarEvolution::getExistingAddRecExpr(ArrayRef<const SCEV *> Ops,
+ const Loop *L) {
+ FoldingSetNodeID ID;
+ ID.AddInteger(scAddRecExpr);
+ for (const SCEV *Op : Ops)
+ ID.AddPointer(Op);
+ ID.AddPointer(L);
+ void *IP = nullptr;
+ return static_cast<SCEVAddRecExpr *>(UniqueSCEVs.FindNodeOrInsertPos(ID, IP));
+}
+
/// Return a SCEV corresponding to -V = -1*V
const SCEV *ScalarEvolution::getNegativeSCEV(const SCEV *V,
SCEV::NoWrapFlags Flags) {
@@ -5703,19 +5715,46 @@ const SCEV *ScalarEvolution::createSimpleAffineAddRec(PHINode *PN,
return nullptr;
SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
- if (BO->IsNUW)
+ if (BO->IsNUW) {
Flags = setFlags(Flags, SCEV::FlagNUW);
- if (BO->IsNSW)
+ Flags = setFlags(Flags, SCEV::FlagNW);
+ }
+ if (BO->IsNSW) {
Flags = setFlags(Flags, SCEV::FlagNSW);
+ Flags = setFlags(Flags, SCEV::FlagNW);
+ }
const SCEV *StartVal = getSCEV(StartValueV);
+ SCEV::NoWrapFlags MinFlags = Flags;
+ auto *ExistingAR = getExistingAddRecExpr({StartVal, Accum}, L);
+ // If there's already an AddRec, use its flags to compute the minimal valid
+ // flags that hold for the users of the existing AddRec and the users of the
+ // current phi we are constructing an AddRec for.
+ if (ExistingAR)
+ MinFlags = maskFlags(ExistingAR->getNoWrapFlags(), Flags);
+
const SCEV *PHISCEV = getAddRecExpr(StartVal, Accum, L, Flags);
insertValueToMap(PN, PHISCEV);
+ // Check if the AddRec can never be poison, but avoid constructing new SCEVs
+ // for PN's operands, as this would instantiate the SCEV for the increment
+ // earlier.
+ bool NeverPoison = isAddRecNeverPoison(PN, L, false);
+ // If the AddRec is never poison, it is safe the use the flags from the IR
+ // unconditionally. But if it may be poison, force the AddRec's flags to the
+ // minimum valid set for both the existing AddRec and the current context.
+ if (!NeverPoison && ExistingAR) {
+ const_cast<SCEVAddRecExpr *>(ExistingAR)->forceSetNoWrapFlags(MinFlags);
+ UnsignedRanges.erase(ExistingAR);
+ SignedRanges.erase(ExistingAR);
+ ConstantMultipleCache.erase(ExistingAR);
+ }
+
if (auto *AR = dyn_cast<SCEVAddRecExpr>(PHISCEV)) {
- setNoWrapFlags(const_cast<SCEVAddRecExpr *>(AR),
- (SCEV::NoWrapFlags)(AR->getNoWrapFlags() |
- proveNoWrapViaConstantRanges(AR)));
+ SCEV::NoWrapFlags FlagsViaConstantRanges = proveNoWrapViaConstantRanges(AR);
+ setNoWrapFlags(
+ const_cast<SCEVAddRecExpr *>(AR),
+ (SCEV::NoWrapFlags)(AR->getNoWrapFlags() | FlagsViaConstantRanges));
}
// We can add Flags to the post-inc expression only if we
@@ -7269,9 +7308,10 @@ bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
return isGuaranteedToTransferExecutionTo(DefI, I);
}
-bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L) {
+bool ScalarEvolution::isAddRecNeverPoison(const Instruction *I, const Loop *L,
+ bool CheckSCEVScope) {
// If we know that \c I can never be poison period, then that's enough.
- if (isSCEVExprNeverPoison(I))
+ if (CheckSCEVScope && isSCEVExprNeverPoison(I))
return true;
// If the loop only has one exit, then we know that, if the loop is entered,
diff --git a/llvm/test/Analysis/ScalarEvolution/iv-poison.ll b/llvm/test/Analysis/ScalarEvolution/iv-poison.ll
new file mode 100644
index 0000000000000..03fa233930cdf
--- /dev/null
+++ b/llvm/test/Analysis/ScalarEvolution/iv-poison.ll
@@ -0,0 +1,399 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 4
+; RUN: opt "-passes=print<scalar-evolution>" -disable-output -S %s 2>&1 | FileCheck %s
+
+; PR59777
+define i2 @iv_nsw_poison(i2 %arg) {
+; CHECK-LABEL: 'iv_nsw_poison'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison
+; CHECK-NEXT: %.07 = phi i2 [ 1, %bb ], [ %i, %bb1 ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%bb1> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %.0 = phi i2 [ 1, %bb ], [ %i2, %bb1 ]
+; CHECK-NEXT: --> {1,+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i = add nsw i2 %.07, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%bb1> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i2 = add i2 %.0, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%bb1> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison
+; CHECK-NEXT: Loop %bb1: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb1, %bb
+ %.07 = phi i2 [ 1, %bb ], [ %i, %bb1 ]
+ %.0 = phi i2 [ 1, %bb ], [ %i2, %bb1 ]
+ %i = add nsw i2 %.07, 1
+ %i2 = add i2 %.0, 1
+ %.not.not = icmp ult i2 %.07, %arg
+ br i1 %.not.not, label %common.ret, label %bb1
+
+common.ret: ; preds = %bb1
+ ret i2 %i2
+}
+
+define i4 @iv_nsw_poison2(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nsw_poison2'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison2
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison2
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ %iv.1.next = add nsw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i2 @iv_both_adds_nsw(i2 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw
+; CHECK-NEXT: %iv.0 = phi i2 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i2 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-2) S: [1,-2) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i2 %iv.0, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%loop> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i2 %iv.1, 1
+; CHECK-NEXT: --> {-2,+,1}<nuw><%loop> U: [-2,0) S: [-2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i2 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i2 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.0.next = add nsw i2 %iv.0, 1
+ %iv.1.next = add nsw i2 %iv.1, 1
+ %.not.not = icmp ult i2 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i2 %iv.1.next
+}
+
+define i4 @iv_both_adds_nsw_extra_use(i4 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw_extra_use'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw_extra_use
+; CHECK-NEXT: %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i4 %iv.0, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw_extra_use
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.0.next = add nsw i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %iv.1.next = add nsw i4 %iv.1, 1
+ call void @use(i4 %iv.1.next)
+ %.not.not = icmp ult i4 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i4 @iv_both_adds_nsw_extra_use_incs_reordered(i4 %arg) {
+; CHECK-LABEL: 'iv_both_adds_nsw_extra_use_incs_reordered'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nsw_extra_use_incs_reordered
+; CHECK-NEXT: %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%loop> U: [1,-8) S: [1,-8) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nsw i4 %iv.0, 1
+; CHECK-NEXT: --> {2,+,1}<nuw><%loop> U: [2,0) S: [2,0) Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_both_adds_nsw_extra_use_incs_reordered
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ 1, %bb ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ 1, %bb ], [ %iv.1.next, %loop ]
+ %iv.1.next = add nsw i4 %iv.1, 1
+ call void @use(i4 %iv.1.next)
+ %iv.0.next = add nsw i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %.not.not = icmp ult i4 %iv.0, %arg
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i4 @iv_nsw_poison_extra_use(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nsw_poison_extra_use'
+; CHECK-NEXT: Classifying expressions for: @iv_nsw_poison_extra_use
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nsw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nsw_poison_extra_use
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ call void @use(i4 %iv.0.next)
+ %iv.1.next = add nsw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+declare void @use(i4)
+
+define i2 @iv_nuw_poison(i2 %arg, i2 %start) {
+; CHECK-LABEL: 'iv_nuw_poison'
+; CHECK-NEXT: Classifying expressions for: @iv_nuw_poison
+; CHECK-NEXT: %.07 = phi i2 [ %start, %bb ], [ %i, %bb1 ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %.0 = phi i2 [ %start, %bb ], [ %i2, %bb1 ]
+; CHECK-NEXT: --> {%start,+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i = add nuw i2 %.07, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: %i2 = add i2 %.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%bb1> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb1: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nuw_poison
+; CHECK-NEXT: Loop %bb1: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %bb1: Unpredictable predicated backedge-taken count.
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb1, %bb
+ %.07 = phi i2 [ %start, %bb ], [ %i, %bb1 ]
+ %.0 = phi i2 [ %start, %bb ], [ %i2, %bb1 ]
+ %i = add nuw i2 %.07, 1
+ %i2 = add i2 %.0, 1
+ %.not.not = icmp ult i2 %.07, %arg
+ br i1 %.not.not, label %common.ret, label %bb1
+
+common.ret: ; preds = %bb1
+ ret i2 %i2
+}
+
+define i4 @iv_nuw_poison2(i4 %0, i4 %end, i4 %start) {
+; CHECK-LABEL: 'iv_nuw_poison2'
+; CHECK-NEXT: Classifying expressions for: @iv_nuw_poison2
+; CHECK-NEXT: %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add i4 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nuw i4 %iv.1, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: Determining loop execution counts for: @iv_nuw_poison2
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable constant max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable symbolic max backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable predicated backedge-taken count.
+;
+entry:
+ br label %loop
+
+loop:
+ %iv.0 = phi i4 [ %start, %entry ], [ %iv.0.next, %loop ]
+ %iv.1 = phi i4 [ %start, %entry ], [ %iv.1.next, %loop ]
+ %iv.0.next = add i4 %iv.0, 1
+ %iv.1.next = add nuw i4 %iv.1, 1
+ %.not.not = icmp ult i4 %iv.0, %end
+ br i1 %.not.not, label %exit, label %loop
+
+exit:
+ ret i4 %iv.1.next
+}
+
+define i2 @iv_both_adds_nuw(i2 %arg, i2 %start) {
+; CHECK-LABEL: 'iv_both_adds_nuw'
+; CHECK-NEXT: Classifying expressions for: @iv_both_adds_nuw
+; CHECK-NEXT: %iv.0 = phi i2 [ %start, %bb ], [ %iv.0.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1 = phi i2 [ %start, %bb ], [ %iv.1.next, %loop ]
+; CHECK-NEXT: --> {%start,+,1}<nuw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.0.next = add nuw i2 %iv.0, 1
+; CHECK-NEXT: --> {(1 + %start),+,1}<nw><%loop> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop: Computable }
+; CHECK-NEXT: %iv.1.next = add nuw i2 %iv.1, 1
+; CHECK-NEXT: --> {(1 +...
[truncated]
|
fhahn
added a commit
that referenced
this pull request
Feb 2, 2024
This patch adds a set of tests taken from/llvm/test/Transforms/IndVarSimplify/iv-poison.ll with multiple congruent IVs but different set of flags on the increments. Extra tests for #80430.
|
I think the underlying issue here is that our nowrap calculation for pre-inc addrecs is just incorrect -- based on a premise that does not generally hold. I had a patch for this here, but didn't get around to finish it: https://reviews.llvm.org/D148931 Would that fix the problem you ran into as well? I don't think that any fix that involves clearing existing nowrap flags is compatible with SCEVs core design. |
Yes I think that would also be sufficient ( I was a bit surprised that we don't do that already :)), but a heavier hammer, which is why I wanted to explore alternatives to only checking non-poison;
Yes right! Ideally I'd like to invalidate all users, completely remove the old SCEV and add it again with the new flags. Updated the patch to use |
I think the general approach is fragile, because it will only work for addrecs created from IR users. E.g. if SCEVExpander queries a canonical |
|
I'm also skeptical this is right. nowrap on an AddRec means the value can't wrap by construction, i.e. based on an upper bound on the trip count of the loop. The only overflow flags that are relevant are flags which allow narrowing that upper bound. |
agozillon
pushed a commit
to agozillon/llvm-project
that referenced
this pull request
Feb 5, 2024
This patch adds a set of tests taken from/llvm/test/Transforms/IndVarSimplify/iv-poison.ll with multiple congruent IVs but different set of flags on the increments. Extra tests for llvm#80430.
fhahn
added a commit
that referenced
this pull request
May 13, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to #90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one #80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
that referenced
this pull request
May 22, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to #90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one #80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
May 22, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
that referenced
this pull request
May 31, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to #90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one #80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Jun 25, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
that referenced
this pull request
Aug 1, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to #90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one #80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Oct 4, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u !fixup use raw pointer (const SCEV * + lower bits) for AddPointer. !fix formatting !fixup fix build failures after rebase, address comments !fixup isCanonical/getCanonical
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Oct 4, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Oct 4, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u !fixup use raw pointer (const SCEV * + lower bits) for AddPointer. !fix formatting !fixup fix build failures after rebase, address comments !fixup isCanonical/getCanonical
fhahn
added a commit
to fhahn/llvm-project
that referenced
this pull request
Oct 5, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to llvm#90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one llvm#80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u !fixup use raw pointer (const SCEV * + lower bits) for AddPointer. !fix formatting !fixup fix build failures after rebase, address comments !fixup isCanonical/getCanonical
fhahn
added a commit
that referenced
this pull request
Oct 9, 2024
This patch introduces SCEVUse, which is a tagged pointer containing the used const SCEV *, plus extra bits to store NUW/NSW flags that are only valid at the specific use. This was suggested by @nikic as an alternative to #90742. This patch just updates most SCEV infrastructure to operate on SCEVUse instead of const SCEV *. It does not introduce any code that makes use of the use-specific flags yet which I'll share as follow-ups. Note that this should be NFC, but currently there's at least one case where it is not (turn-to-invariant.ll), which I'll investigate once we agree on the overall direction. This PR at the moment also contains a commit that updates various SCEV clients to use `const SCEV *` instead of `const auto *`, to prepare for this patch. This reduces the number of changes needed, as SCEVUse will automatically convert to `const SCEV *`. This is a safe default, as it just drops the use-specific flags for the expression (it will not drop any use-specific flags for any of its operands though). This probably SCEVUse could probably also be used to address mis-compiles due to equivalent AddRecs modulo flags result in an AddRec with incorrect flags for some uses of some phis, e.g. the one #80430 attempted to fix Compile-time impact: stage1-O3: +0.06% stage1-ReleaseThinLTO: +0.07% stage1-ReleaseLTO-g: +0.07% stage2-O3: +0.11% https://llvm-compile-time-tracker.com/compare.php?from=ce055843e2be9643bd58764783a7bb69f6db8c9a&to=8c7f4e9e154ebc4862c4e2716cedc3c688352d7c&stat=instructions:u
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When constructing AddRecs from multiple equivalent phi nodes modulo
increment wrap flags, the resulting AddRec has the union of all
increment wrap flags as no-wrap flags.
This seems incorrect: uses of this AddRec are more poisonous in contexts
where an original phi node didn't have an increment with wrap flags.
This patch tries to ensures that the wrap flags for the constructed
AddRecs are valid for all phis that get mapped to an AddRec. To do so,
first check if there's already an existing AddRec. In that case, the
available wrap-flags are an upper bound on the wrap-flags; additional
wrap flags may make existing uses more poisonous.
If the AddRec can never be poison, it should still be safe to use the
full IR flags from IR.
Note the change in @iv_hoist_nuw_poison_extra_use fixes a mis-compile
found by Alive2.