[llvm] 05b44f7 - [LCSSA] Provide option for caller to clean up unused PHIs.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 1 12:44:09 PDT 2020
Author: Florian Hahn
Date: 2020-08-01T20:43:19+01:00
New Revision: 05b44f7eaebfbca19999fde149c4c586fc965015
URL: https://github.com/llvm/llvm-project/commit/05b44f7eaebfbca19999fde149c4c586fc965015
DIFF: https://github.com/llvm/llvm-project/commit/05b44f7eaebfbca19999fde149c4c586fc965015.diff
LOG: [LCSSA] Provide option for caller to clean up unused PHIs.
formLCSSAForInstructions is used by SCEVExpander, which tracks all
inserted instructions including LCSSA phis using asserting value
handles. This means cleanup needs to happen in the caller.
Extend formLCSSAForInstructions to take an optional pointer to a
vector. If this argument is non-nullptr, instead of directly deleting
the phis, add them to the vector, so the caller can process them.
This should address various PPC buildbot failures, including
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/40567
Added:
llvm/test/CodeGen/PowerPC/hardware-loops-crash.ll
Modified:
llvm/include/llvm/Transforms/Utils/LoopUtils.h
llvm/lib/Transforms/Utils/LCSSA.cpp
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index c6a8b27811ed..70c8c84c857b 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -74,9 +74,14 @@ bool formDedicatedExitBlocks(Loop *L, DominatorTree *DT, LoopInfo *LI,
/// changes to CFG, preserved.
///
/// Returns true if any modifications are made.
-bool formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
- const DominatorTree &DT, const LoopInfo &LI,
- ScalarEvolution *SE, IRBuilderBase &Builder);
+///
+/// This function may introduce unused PHI nodes. If \p PHIsToRemove is not
+/// nullptr, those are added to it (before removing, the caller has to check if
+/// they still do not have any uses). Otherwise the PHIs are directly removed.
+bool formLCSSAForInstructions(
+ SmallVectorImpl<Instruction *> &Worklist, const DominatorTree &DT,
+ const LoopInfo &LI, ScalarEvolution *SE, IRBuilderBase &Builder,
+ SmallVectorImpl<PHINode *> *PHIsToRemove = nullptr);
/// Put loop into LCSSA form.
///
diff --git a/llvm/lib/Transforms/Utils/LCSSA.cpp b/llvm/lib/Transforms/Utils/LCSSA.cpp
index 9c606251ae0f..630aadadbbce 100644
--- a/llvm/lib/Transforms/Utils/LCSSA.cpp
+++ b/llvm/lib/Transforms/Utils/LCSSA.cpp
@@ -78,10 +78,10 @@ static bool isExitBlock(BasicBlock *BB,
/// rewrite the uses.
bool llvm::formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
const DominatorTree &DT, const LoopInfo &LI,
- ScalarEvolution *SE,
- IRBuilderBase &Builder) {
+ ScalarEvolution *SE, IRBuilderBase &Builder,
+ SmallVectorImpl<PHINode *> *PHIsToRemove) {
SmallVector<Use *, 16> UsesToRewrite;
- SmallSetVector<PHINode *, 16> PHIsToRemove;
+ SmallSetVector<PHINode *, 16> LocalPHIsToRemove;
PredIteratorCache PredCache;
bool Changed = false;
@@ -257,22 +257,28 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
SmallVector<PHINode *, 2> NeedDbgValues;
for (PHINode *PN : AddedPHIs)
if (PN->use_empty())
- PHIsToRemove.insert(PN);
+ LocalPHIsToRemove.insert(PN);
else
NeedDbgValues.push_back(PN);
insertDebugValuesForPHIs(InstBB, NeedDbgValues);
Changed = true;
}
- // Remove PHI nodes that did not have any uses rewritten. We need to redo the
- // use_empty() check here, because even if the PHI node wasn't used when added
- // to PHIsToRemove, later added PHI nodes can be using it. This cleanup is
- // not guaranteed to handle trees/cycles of PHI nodes that only are used by
- // each other. Such situations has only been noticed when the input IR
- // contains unreachable code, and leaving some extra redundant PHI nodes in
- // such situations is considered a minor problem.
- for (PHINode *PN : PHIsToRemove)
- if (PN->use_empty())
- PN->eraseFromParent();
+
+ // Remove PHI nodes that did not have any uses rewritten or add them to
+ // PHIsToRemove, so the caller can remove them after some additional cleanup.
+ // We need to redo the use_empty() check here, because even if the PHI node
+ // wasn't used when added to LocalPHIsToRemove, later added PHI nodes can be
+ // using it. This cleanup is not guaranteed to handle trees/cycles of PHI
+ // nodes that only are used by each other. Such situations has only been
+ // noticed when the input IR contains unreachable code, and leaving some extra
+ // redundant PHI nodes in such situations is considered a minor problem.
+ if (PHIsToRemove) {
+ PHIsToRemove->append(LocalPHIsToRemove.begin(), LocalPHIsToRemove.end());
+ } else {
+ for (PHINode *PN : LocalPHIsToRemove)
+ if (PN->use_empty())
+ PN->eraseFromParent();
+ }
return Changed;
}
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index a8302b7ccfc1..aaa28feb32b8 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -2537,7 +2537,16 @@ Value *SCEVExpander::fixupLCSSAFormFor(Instruction *User, unsigned OpIdx) {
return OpV;
ToUpdate.push_back(OpI);
- formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE, Builder);
+ SmallVector<PHINode *, 16> PHIsToRemove;
+ formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE, Builder, &PHIsToRemove);
+ for (PHINode *PN : PHIsToRemove) {
+ if (!PN->use_empty())
+ continue;
+ InsertedValues.erase(PN);
+ InsertedPostIncValues.erase(PN);
+ PN->eraseFromParent();
+ }
+
return User->getOperand(OpIdx);
}
diff --git a/llvm/test/CodeGen/PowerPC/hardware-loops-crash.ll b/llvm/test/CodeGen/PowerPC/hardware-loops-crash.ll
new file mode 100644
index 000000000000..24e9592a6c2b
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/hardware-loops-crash.ll
@@ -0,0 +1,101 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -hardware-loops -S -verify-loop-lcssa | FileCheck %s
+
+target datalayout = "E-m:e-i64:64-n32:64"
+target triple = "ppc64-unknown-linux-elf"
+
+declare i1 @cond() readnone
+
+; Make sure we do not crash on the test.
+
+define void @test() {
+; CHECK-LABEL: @test(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[WHILE_COND:%.*]]
+; CHECK: while.cond:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: br label [[FOR_INC:%.*]]
+; CHECK: for.inc:
+; CHECK-NEXT: [[C_0:%.*]] = call i1 @cond()
+; CHECK-NEXT: br i1 [[C_0]], label [[WHILE_COND25:%.*]], label [[FOR_BODY]]
+; CHECK: while.cond25:
+; CHECK-NEXT: [[INDVAR:%.*]] = phi i64 [ [[INDVAR_NEXT:%.*]], [[LAND_RHS:%.*]] ], [ 0, [[FOR_INC]] ]
+; CHECK-NEXT: [[INDVARS_IV349:%.*]] = phi i64 [ [[INDVARS_IV_NEXT350:%.*]], [[LAND_RHS]] ], [ 50, [[FOR_INC]] ]
+; CHECK-NEXT: [[CMP26_NOT:%.*]] = icmp eq i64 [[INDVARS_IV349]], 0
+; CHECK-NEXT: br i1 [[CMP26_NOT]], label [[WHILE_END187:%.*]], label [[LAND_RHS]]
+; CHECK: land.rhs:
+; CHECK-NEXT: [[INDVARS_IV_NEXT350]] = add nsw i64 [[INDVARS_IV349]], -1
+; CHECK-NEXT: [[C_1:%.*]] = call i1 @cond()
+; CHECK-NEXT: [[INDVAR_NEXT]] = add i64 [[INDVAR]], 1
+; CHECK-NEXT: br i1 [[C_1]], label [[WHILE_COND25]], label [[WHILE_END:%.*]]
+; CHECK: while.end:
+; CHECK-NEXT: [[INDVAR_LCSSA1:%.*]] = phi i64 [ [[INDVAR]], [[LAND_RHS]] ]
+; CHECK-NEXT: [[C_2:%.*]] = call i1 @cond()
+; CHECK-NEXT: br i1 [[C_2]], label [[WHILE_END187]], label [[WHILE_COND35_PREHEADER:%.*]]
+; CHECK: while.cond35.preheader:
+; CHECK-NEXT: [[TMP0:%.*]] = mul nsw i64 [[INDVAR_LCSSA1]], -1
+; CHECK-NEXT: [[TMP1:%.*]] = add i64 [[TMP0]], 51
+; CHECK-NEXT: call void @llvm.set.loop.iterations.i64(i64 [[TMP1]])
+; CHECK-NEXT: br label [[WHILE_COND35:%.*]]
+; CHECK: while.cond35:
+; CHECK-NEXT: [[TMP2:%.*]] = call i1 @llvm.loop.decrement.i64(i64 1)
+; CHECK-NEXT: br i1 [[TMP2]], label [[LAND_RHS37:%.*]], label [[IF_END51:%.*]]
+; CHECK: land.rhs37:
+; CHECK-NEXT: br label [[WHILE_COND35]]
+; CHECK: if.end51:
+; CHECK-NEXT: br label [[WHILE_COND_BACKEDGE:%.*]]
+; CHECK: while.cond.backedge:
+; CHECK-NEXT: br label [[WHILE_COND]]
+; CHECK: while.end187:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %while.cond
+
+while.cond: ; preds = %while.cond.backedge, %entry
+ br label %for.body
+
+for.body: ; preds = %for.inc, %while.cond
+ br label %for.inc
+
+for.inc: ; preds = %for.body
+ %c.0 = call i1 @cond()
+ br i1 %c.0, label %while.cond25, label %for.body
+
+while.cond25: ; preds = %land.rhs, %for.inc
+ %indvars.iv349 = phi i64 [ %indvars.iv.next350, %land.rhs ], [ 50, %for.inc ]
+ %cmp26.not = icmp eq i64 %indvars.iv349, 0
+ br i1 %cmp26.not, label %while.end187, label %land.rhs
+
+land.rhs: ; preds = %while.cond25
+ %indvars.iv.next350 = add nsw i64 %indvars.iv349, -1
+ %c.1 = call i1 @cond()
+ br i1 %c.1, label %while.cond25, label %while.end
+
+while.end: ; preds = %land.rhs
+ %c.2 = call i1 @cond()
+ br i1 %c.2, label %while.end187, label %while.cond35.preheader
+
+while.cond35.preheader: ; preds = %while.end
+ %0 = and i64 %indvars.iv349, 4294967295
+ br label %while.cond35
+
+while.cond35: ; preds = %land.rhs37, %while.cond35.preheader
+ %indvars.iv351 = phi i64 [ %0, %while.cond35.preheader ], [ %indvars.iv.next352, %land.rhs37 ]
+ %cmp36 = icmp sgt i64 %indvars.iv351, 0
+ br i1 %cmp36, label %land.rhs37, label %if.end51
+
+land.rhs37: ; preds = %while.cond35
+ %indvars.iv.next352 = add nsw i64 %indvars.iv351, -1
+ br label %while.cond35
+
+if.end51: ; preds = %while.cond35
+ br label %while.cond.backedge
+
+while.cond.backedge: ; preds = %if.end51
+ br label %while.cond
+
+while.end187: ; preds = %while.end, %while.cond25
+ ret void
+}
More information about the llvm-commits
mailing list