[llvm] 86d5586 - [SCEVExpander] Recompute poison-generating flags on hoisting. PR57187
Max Kazantsev via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 12 22:56:46 PDT 2022
Author: Max Kazantsev
Date: 2022-09-13T12:56:35+07:00
New Revision: 86d5586d78d813a6921d786f7ddb1a41c6fb56e0
URL: https://github.com/llvm/llvm-project/commit/86d5586d78d813a6921d786f7ddb1a41c6fb56e0
DIFF: https://github.com/llvm/llvm-project/commit/86d5586d78d813a6921d786f7ddb1a41c6fb56e0.diff
LOG: [SCEVExpander] Recompute poison-generating flags on hoisting. PR57187
Instruction being hoisted could have nuw/nsw flags inferred from the old
context, and we cannot simply move it to the new location keeping them
because we are going to introduce new uses to them that didn't exist before.
Example in https://github.com/llvm/llvm-project/issues/57187 shows how
this can produce branch by poison from initially well-defined program.
This patch forcefully recomputes poison-generating flag in the new context.
Differential Revision: https://reviews.llvm.org/D132022
Reviewed By: fhahn, nikic
Added:
Modified:
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
llvm/test/Transforms/IndVarSimplify/X86/pr57187.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index 79004a4c535ac..27691770b28aa 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -248,8 +248,14 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
Instruction *getIVIncOperand(Instruction *IncV, Instruction *InsertPos,
bool allowScale);
- /// Utility for hoisting an IV increment.
- bool hoistIVInc(Instruction *IncV, Instruction *InsertPos);
+ /// Utility for hoisting \p IncV (with all subexpressions requried for its
+ /// computation) before \p InsertPos. If \p RecomputePoisonFlags is set, drops
+ /// all poison-generating flags from instructions being hoisted and tries to
+ /// re-infer them in the new location. It should be used when we are going to
+ /// introduce a new use in the new position that didn't exist before, and may
+ /// trigger new UB in case of poison.
+ bool hoistIVInc(Instruction *IncV, Instruction *InsertPos,
+ bool RecomputePoisonFlags = false);
/// replace congruent phis with their most canonical representative. Return
/// the number of phis eliminated.
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index 3e7e016b99b5e..568c6b5581cfa 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -1024,7 +1024,8 @@ void SCEVExpander::fixupInsertPoints(Instruction *I) {
/// hoistStep - Attempt to hoist a simple IV increment above InsertPos to make
/// it available to other uses in this loop. Recursively hoist any operands,
/// until we reach a value that dominates InsertPos.
-bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos) {
+bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
+ bool RecomputePoisonFlags) {
if (SE.DT.dominates(IncV, InsertPos))
return true;
@@ -1052,6 +1053,19 @@ bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos) {
for (Instruction *I : llvm::reverse(IVIncs)) {
fixupInsertPoints(I);
I->moveBefore(InsertPos);
+ if (!RecomputePoisonFlags)
+ continue;
+ // Drop flags that are potentially inferred from old context and infer flags
+ // in new context.
+ I->dropPoisonGeneratingFlags();
+ if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
+ if (auto Flags = SE.getStrengthenedNoWrapFlagsFromBinOp(OBO)) {
+ auto *BO = cast<BinaryOperator>(I);
+ BO->setHasNoUnsignedWrap(
+ ScalarEvolution::maskFlags(*Flags, SCEV::FlagNUW) == SCEV::FlagNUW);
+ BO->setHasNoSignedWrap(
+ ScalarEvolution::maskFlags(*Flags, SCEV::FlagNSW) == SCEV::FlagNSW);
+ }
}
return true;
}
@@ -2006,12 +2020,14 @@ SCEVExpander::replaceCongruentIVs(Loop *L, const DominatorTree *DT,
// with the original phi. It's worth eagerly cleaning up the
// common case of a single IV increment so that DeleteDeadPHIs
// can remove cycles that had postinc uses.
+ // Because we may potentially introduce a new use of OrigIV that didn't
+ // exist before at this point, its poison flags need readjustment.
const SCEV *TruncExpr =
SE.getTruncateOrNoop(SE.getSCEV(OrigInc), IsomorphicInc->getType());
if (OrigInc != IsomorphicInc &&
TruncExpr == SE.getSCEV(IsomorphicInc) &&
SE.LI.replacementPreservesLCSSAForm(IsomorphicInc, OrigInc) &&
- hoistIVInc(OrigInc, IsomorphicInc)) {
+ hoistIVInc(OrigInc, IsomorphicInc, /*RecomputePoisonFlags*/ true)) {
SCEV_DEBUG_WITH_TYPE(
DebugType, dbgs() << "INDVARS: Eliminated congruent iv.inc: "
<< *IsomorphicInc << '\n');
diff --git a/llvm/test/Transforms/IndVarSimplify/X86/pr57187.ll b/llvm/test/Transforms/IndVarSimplify/X86/pr57187.ll
index 9b587c9824f95..08633344276c1 100644
--- a/llvm/test/Transforms/IndVarSimplify/X86/pr57187.ll
+++ b/llvm/test/Transforms/IndVarSimplify/X86/pr57187.ll
@@ -4,8 +4,7 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
target triple = "x86_64-unknown-linux-gnu"
-; FIXME: This test is demonstrating a miscompile. The original program is well-defined,
-; and after opt for any start != 0 branch by poisoned add nuw nsw is introduced.
+; Make sure we don't branch by poison here.
define void @test(i32 %start) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
@@ -15,7 +14,7 @@ define void @test(i32 %start) {
; CHECK-NEXT: br label [[LOOP]]
; CHECK: loop:
; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ [[INDVARS_IV_NEXT:%.*]], [[BACKEDGE:%.*]] ], [ [[TMP0]], [[ENTRY:%.*]] ]
-; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], -1
+; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nsw i64 [[INDVARS_IV]], -1
; CHECK-NEXT: [[INDVARS:%.*]] = trunc i64 [[INDVARS_IV_NEXT]] to i32
; CHECK-NEXT: [[LOOP_EXIT_COND:%.*]] = icmp slt i32 [[INDVARS]], 11
; CHECK-NEXT: br i1 [[LOOP_EXIT_COND]], label [[EXIT:%.*]], label [[STUCK_PREHEADER:%.*]]
More information about the llvm-commits
mailing list