[llvm] 9933a2e - [SCEVExpander] Move LCSSA fixup to ::expand.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 29 12:50:27 PDT 2022
Author: Florian Hahn
Date: 2022-09-29T20:49:56+01:00
New Revision: 9933a2e9fd0c37dcdce5952fab7e486d3cf2d336
URL: https://github.com/llvm/llvm-project/commit/9933a2e9fd0c37dcdce5952fab7e486d3cf2d336
DIFF: https://github.com/llvm/llvm-project/commit/9933a2e9fd0c37dcdce5952fab7e486d3cf2d336.diff
LOG: [SCEVExpander] Move LCSSA fixup to ::expand.
Move LCSSA fixup from ::expandCodeForImpl to ::expand(). This has
the advantage that we directly preserve LCSSA nodes here instead of
relying on doing so in rememberInstruction. It also ensures that we
don't add the non-LCSSA-safe value to InsertedExpressions.
Alternative to D132704.
Fixes #57000.
Reviewed By: efriedma
Differential Revision: https://reviews.llvm.org/D134739
Added:
Modified:
llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll
llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index 94f205345622e..aa4e0ac7001dd 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -499,10 +499,9 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
void fixupInsertPoints(Instruction *I);
- /// If required, create LCSSA PHIs for \p Users' operand \p OpIdx. If new
- /// LCSSA PHIs have been created, return the LCSSA PHI available at \p User.
- /// If no PHIs have been created, return the unchanged operand \p OpIdx.
- Value *fixupLCSSAFormFor(Instruction *User, unsigned OpIdx);
+ /// Create LCSSA PHIs for \p V, if it is required for uses at the Builder's
+ /// current insertion point.
+ Value *fixupLCSSAFormFor(Value *V);
};
/// Helper to remove instructions inserted during SCEV expansion, unless they
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index bc2779aee1c06..84db277b2aa1b 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -14,6 +14,7 @@
#include "llvm/Transforms/Utils/ScalarEvolutionExpander.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
@@ -1742,30 +1743,6 @@ Value *SCEVExpander::expandCodeForImpl(const SCEV *SH, Type *Ty) {
// Expand the code for this SCEV.
Value *V = expand(SH);
- if (PreserveLCSSA) {
- if (auto *Inst = dyn_cast<Instruction>(V)) {
- // Create a temporary instruction to at the current insertion point, so we
- // can hand it off to the helper to create LCSSA PHIs if required for the
- // new use.
- // FIXME: Ideally formLCSSAForInstructions (used in fixupLCSSAFormFor)
- // would accept a insertion point and return an LCSSA phi for that
- // insertion point, so there is no need to insert & remove the temporary
- // instruction.
- Type *ToTy;
- if (Inst->getType()->isIntegerTy())
- ToTy = Inst->getType()->getPointerTo();
- else
- ToTy = Type::getInt32Ty(Inst->getContext());
- Instruction *Tmp = CastInst::CreateBitOrPointerCast(
- Inst, ToTy, "tmp.lcssa.user", &*Builder.GetInsertPoint());
- V = fixupLCSSAFormFor(Tmp, 0);
-
- // Clean up temporary instruction.
- Tmp->eraseFromParent();
- }
- }
-
- InsertedExpressions[std::make_pair(SH, &*Builder.GetInsertPoint())] = V;
if (Ty) {
assert(SE.getTypeSizeInBits(Ty) == SE.getTypeSizeInBits(SH->getType()) &&
"non-trivial casts should be done with the SCEVs directly!");
@@ -1870,9 +1847,10 @@ Value *SCEVExpander::expand(const SCEV *S) {
// Expand the expression into instructions.
Value *V = FindValueInExprValueMap(S, InsertPt);
- if (!V)
+ if (!V) {
V = visit(S);
- else {
+ V = fixupLCSSAFormFor(V);
+ } else {
// If we're reusing an existing instruction, we are effectively CSEing two
// copies of the instruction (with potentially
diff erent flags). As such,
// we need to drop any poison generating flags unless we can prove that
@@ -1899,18 +1877,6 @@ void SCEVExpander::rememberInstruction(Value *I) {
InsertedValues.insert(V);
};
DoInsert(I);
-
- if (!PreserveLCSSA)
- return;
-
- if (auto *Inst = dyn_cast<Instruction>(I)) {
- // A new instruction has been added, which might introduce new uses outside
- // a defining loop. Fix LCSSA from for each operand of the new instruction,
- // if required.
- for (unsigned OpIdx = 0, OpEnd = Inst->getNumOperands(); OpIdx != OpEnd;
- OpIdx++)
- fixupLCSSAFormFor(Inst, OpIdx);
- }
}
/// replaceCongruentIVs - Check for congruent phis in this loop header and
@@ -2541,21 +2507,36 @@ Value *SCEVExpander::expandUnionPredicate(const SCEVUnionPredicate *Union,
return Builder.CreateOr(Checks);
}
-Value *SCEVExpander::fixupLCSSAFormFor(Instruction *User, unsigned OpIdx) {
- assert(PreserveLCSSA);
- SmallVector<Instruction *, 1> ToUpdate;
-
- auto *OpV = User->getOperand(OpIdx);
- auto *OpI = dyn_cast<Instruction>(OpV);
- if (!OpI)
- return OpV;
+Value *SCEVExpander::fixupLCSSAFormFor(Value *V) {
+ auto *DefI = dyn_cast<Instruction>(V);
+ if (!PreserveLCSSA || !DefI)
+ return V;
- Loop *DefLoop = SE.LI.getLoopFor(OpI->getParent());
- Loop *UseLoop = SE.LI.getLoopFor(User->getParent());
+ Instruction *InsertPt = &*Builder.GetInsertPoint();
+ Loop *DefLoop = SE.LI.getLoopFor(DefI->getParent());
+ Loop *UseLoop = SE.LI.getLoopFor(InsertPt->getParent());
if (!DefLoop || UseLoop == DefLoop || DefLoop->contains(UseLoop))
- return OpV;
+ return V;
- ToUpdate.push_back(OpI);
+ // Create a temporary instruction to at the current insertion point, so we
+ // can hand it off to the helper to create LCSSA PHIs if required for the
+ // new use.
+ // FIXME: Ideally formLCSSAForInstructions (used in fixupLCSSAFormFor)
+ // would accept a insertion point and return an LCSSA phi for that
+ // insertion point, so there is no need to insert & remove the temporary
+ // instruction.
+ Type *ToTy;
+ if (DefI->getType()->isIntegerTy())
+ ToTy = DefI->getType()->getPointerTo();
+ else
+ ToTy = Type::getInt32Ty(DefI->getContext());
+ Instruction *User =
+ CastInst::CreateBitOrPointerCast(DefI, ToTy, "tmp.lcssa.user", InsertPt);
+ auto RemoveUserOnExit =
+ make_scope_exit([User]() { User->eraseFromParent(); });
+
+ SmallVector<Instruction *, 1> ToUpdate;
+ ToUpdate.push_back(DefI);
SmallVector<PHINode *, 16> PHIsToRemove;
formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE, Builder, &PHIsToRemove);
for (PHINode *PN : PHIsToRemove) {
@@ -2566,7 +2547,7 @@ Value *SCEVExpander::fixupLCSSAFormFor(Instruction *User, unsigned OpIdx) {
PN->eraseFromParent();
}
- return User->getOperand(OpIdx);
+ return User->getOperand(0);
}
namespace {
diff --git a/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll b/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll
index 5e7b5adf89337..4b288c2a891bc 100644
--- a/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll
+++ b/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll
@@ -86,3 +86,59 @@ exit:
%res.lcssa2 = phi i64 [ %res.lcssa, %loop1.latch ]
ret i64 %res.lcssa2
}
+
+; Check that it does not crash because the incoming values of an LCSSA phi are
+; equal.
+define void @pr57000(i64 %a) {
+; CHECK-LABEL: @pr57000(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[LOOP_1_OUTER:%.*]]
+; CHECK: loop.1.loopexit:
+; CHECK-NEXT: br label [[LOOP_1_OUTER]]
+; CHECK: loop.1.outer:
+; CHECK-NEXT: br label [[LOOP_1:%.*]]
+; CHECK: loop.1:
+; CHECK-NEXT: [[CMP:%.*]] = icmp sle i64 [[A:%.*]], 2546175499358690212
+; CHECK-NEXT: [[CMP_EXT:%.*]] = zext i1 [[CMP]] to i8
+; CHECK-NEXT: br i1 [[CMP]], label [[LOOP_1]], label [[LOOP_2_HEADER_PREHEADER:%.*]]
+; CHECK: loop.2.header.preheader:
+; CHECK-NEXT: [[CMP_LCSSA:%.*]] = phi i1 [ [[CMP]], [[LOOP_1]] ]
+; CHECK-NEXT: [[CMP_EXT_LCSSA:%.*]] = phi i8 [ [[CMP_EXT]], [[LOOP_1]] ]
+; CHECK-NEXT: br label [[LOOP_2_HEADER_OUTER:%.*]]
+; CHECK: loop.2.header.outer:
+; CHECK-NEXT: br label [[LOOP_2_HEADER:%.*]]
+; CHECK: loop.2.header:
+; CHECK-NEXT: switch i8 [[CMP_EXT_LCSSA]], label [[LOOP_1_LOOPEXIT:%.*]] [
+; CHECK-NEXT: i8 -1, label [[LOOP_2_LATCH:%.*]]
+; CHECK-NEXT: i8 1, label [[LOOP_2_LATCH]]
+; CHECK-NEXT: i8 4, label [[LOOP_2_HEADER]]
+; CHECK-NEXT: ]
+; CHECK: loop.2.latch:
+; CHECK-NEXT: [[CMP_TRUNC_LCSSA1:%.*]] = phi i1 [ [[CMP_LCSSA]], [[LOOP_2_HEADER]] ], [ [[CMP_LCSSA]], [[LOOP_2_HEADER]] ]
+; CHECK-NEXT: call void @use(i1 [[CMP_TRUNC_LCSSA1]])
+; CHECK-NEXT: br label [[LOOP_2_HEADER_OUTER]]
+;
+entry:
+ br label %loop.1
+
+loop.1:
+ %p.1 = phi i1 [ 0 , %entry ], [ %p.1, %loop.1 ], [ %p.2, %loop.2.header ]
+ %cmp = icmp sle i64 %a, 2546175499358690212
+ %cmp.ext = zext i1 %cmp to i8
+ br i1 %cmp, label %loop.1, label %loop.2.header
+
+loop.2.header:
+ %p.2 = phi i1 [ %p.1, %loop.1 ], [ %p.2, %loop.2.header ], [ %cmp, %loop.2.latch ]
+ %cmp.trunc = trunc i8 %cmp.ext to i1
+ switch i8 %cmp.ext, label %loop.1 [
+ i8 -1, label %loop.2.latch
+ i8 1, label %loop.2.latch
+ i8 4, label %loop.2.header
+ ]
+
+loop.2.latch:
+ call void @use(i1 %cmp.trunc)
+ br label %loop.2.header
+}
+
+declare void @use(i1)
diff --git a/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll b/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll
index a1cb8cecdf504..2d961eae68cc0 100644
--- a/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll
+++ b/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll
@@ -23,7 +23,6 @@ define i16 @test(ptr %arg, i64 %N) {
; CHECK-NEXT: [[C_3:%.*]] = call i1 @cond()
; CHECK-NEXT: br i1 [[C_3]], label [[LOOP_3_PREHEADER:%.*]], label [[INNER_LATCH:%.*]]
; CHECK: loop.3.preheader:
-; CHECK-NEXT: [[L_1_LCSSA7:%.*]] = phi ptr [ [[L_1]], [[INNER_BB]] ]
; CHECK-NEXT: [[L_1_LCSSA:%.*]] = phi ptr [ [[L_1]], [[INNER_BB]] ]
; CHECK-NEXT: [[L_2_LCSSA:%.*]] = phi ptr [ [[L_2]], [[INNER_BB]] ]
; CHECK-NEXT: [[TMP0:%.*]] = add i64 [[N:%.*]], 1
@@ -31,12 +30,12 @@ define i16 @test(ptr %arg, i64 %N) {
; CHECK-NEXT: br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_MEMCHECK:%.*]]
; CHECK: vector.memcheck:
; CHECK-NEXT: [[UGLYGEP:%.*]] = getelementptr i8, ptr [[L_2_LCSSA]], i64 2
-; CHECK-NEXT: [[UGLYGEP3:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 2
+; CHECK-NEXT: [[UGLYGEP5:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 2
; CHECK-NEXT: [[TMP1:%.*]] = shl i64 [[N]], 1
; CHECK-NEXT: [[TMP2:%.*]] = add i64 [[TMP1]], 4
-; CHECK-NEXT: [[UGLYGEP6:%.*]] = getelementptr i8, ptr [[L_1_LCSSA7]], i64 [[TMP2]]
+; CHECK-NEXT: [[UGLYGEP6:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 [[TMP2]]
; CHECK-NEXT: [[BOUND0:%.*]] = icmp ult ptr [[L_2_LCSSA]], [[UGLYGEP6]]
-; CHECK-NEXT: [[BOUND1:%.*]] = icmp ult ptr [[UGLYGEP3]], [[UGLYGEP]]
+; CHECK-NEXT: [[BOUND1:%.*]] = icmp ult ptr [[UGLYGEP5]], [[UGLYGEP]]
; CHECK-NEXT: [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]]
; CHECK-NEXT: br i1 [[FOUND_CONFLICT]], label [[SCALAR_PH]], label [[VECTOR_PH:%.*]]
; CHECK: vector.ph:
@@ -81,11 +80,11 @@ define i16 @test(ptr %arg, i64 %N) {
; CHECK: exit.loopexit:
; CHECK-NEXT: br label [[EXIT:%.*]]
; CHECK: exit.loopexit1:
-; CHECK-NEXT: [[L_1_LCSSA4:%.*]] = phi ptr [ [[L_1]], [[INNER_LATCH]] ]
+; CHECK-NEXT: [[L_1_LCSSA3:%.*]] = phi ptr [ [[L_1]], [[INNER_LATCH]] ]
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
-; CHECK-NEXT: [[L_15:%.*]] = phi ptr [ [[L_1_LCSSA4]], [[EXIT_LOOPEXIT1]] ], [ [[L_1_LCSSA]], [[EXIT_LOOPEXIT]] ]
-; CHECK-NEXT: [[L_3:%.*]] = load i16, ptr [[L_15]], align 2
+; CHECK-NEXT: [[L_14:%.*]] = phi ptr [ [[L_1_LCSSA3]], [[EXIT_LOOPEXIT1]] ], [ [[L_1_LCSSA]], [[EXIT_LOOPEXIT]] ]
+; CHECK-NEXT: [[L_3:%.*]] = load i16, ptr [[L_14]], align 2
; CHECK-NEXT: ret i16 [[L_3]]
;
entry:
More information about the llvm-commits
mailing list