[llvm] r309988 - [NewGVN] Fix the case where we have a phi-of-ops which goes away.
Davide Italiano via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 3 14:17:49 PDT 2017
Author: davide
Date: Thu Aug 3 14:17:49 2017
New Revision: 309988
URL: http://llvm.org/viewvc/llvm-project?rev=309988&view=rev
Log:
[NewGVN] Fix the case where we have a phi-of-ops which goes away.
Patch by Daniel Berlin, fixes PR33196 (and probably something else).
Added:
llvm/trunk/test/Transforms/NewGVN/pr33196.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=309988&r1=309987&r2=309988&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Thu Aug 3 14:17:49 2017
@@ -471,8 +471,8 @@ class NewGVN {
DenseSet<const Instruction *> PHINodeUses;
// Map a temporary instruction we created to a parent block.
DenseMap<const Value *, BasicBlock *> TempToBlock;
- // Map between the temporary phis we created and the real instructions they
- // are known equivalent to.
+ // Map between the already in-program instructions and the temporary phis we
+ // created that they are known equivalent to.
DenseMap<const Value *, PHINode *> RealToTemp;
// In order to know when we should re-process instructions that have
// phi-of-ops, we track the set of expressions that they needed as
@@ -486,10 +486,14 @@ class NewGVN {
DenseMap<const Expression *, SmallPtrSet<Instruction *, 2>>
ExpressionToPhiOfOps;
// Map from basic block to the temporary operations we created
- DenseMap<const BasicBlock *, SmallVector<PHINode *, 8>> PHIOfOpsPHIs;
+ DenseMap<const BasicBlock *, SmallPtrSet<PHINode *, 2>> PHIOfOpsPHIs;
// Map from temporary operation to MemoryAccess.
DenseMap<const Instruction *, MemoryUseOrDef *> TempToMemory;
// Set of all temporary instructions we created.
+ // Note: This will include instructions that were just created during value
+ // numbering. The way to test if something is using them is to check
+ // RealToTemp.
+
DenseSet<Instruction *> AllTempInstructions;
// Mapping from predicate info we used to the instructions we used it with.
@@ -634,6 +638,7 @@ private:
const Expression *makePossiblePhiOfOps(Instruction *,
SmallPtrSetImpl<Value *> &);
void addPhiOfOps(PHINode *Op, BasicBlock *BB, Instruction *ExistingValue);
+ void removePhiOfOps(Instruction *I, PHINode *PHITemp);
// Value number an Instruction or MemoryPhi.
void valueNumberMemoryPhi(MemoryPhi *);
@@ -2414,11 +2419,22 @@ void NewGVN::processOutgoingEdges(Termin
}
}
+// Remove the PHI of Ops PHI for I
+void NewGVN::removePhiOfOps(Instruction *I, PHINode *PHITemp) {
+ InstrDFS.erase(PHITemp);
+ // It's still a temp instruction. We keep it in the array so it gets erased.
+ // However, it's no longer used by I, or in the block/
+ PHIOfOpsPHIs[getBlockForValue(PHITemp)].erase(PHITemp);
+ TempToBlock.erase(PHITemp);
+ RealToTemp.erase(I);
+}
+
+// Add PHI Op in BB as a PHI of operations version of ExistingValue.
void NewGVN::addPhiOfOps(PHINode *Op, BasicBlock *BB,
Instruction *ExistingValue) {
InstrDFS[Op] = InstrToDFSNum(ExistingValue);
AllTempInstructions.insert(Op);
- PHIOfOpsPHIs[BB].push_back(Op);
+ PHIOfOpsPHIs[BB].insert(Op);
TempToBlock[Op] = BB;
RealToTemp[ExistingValue] = Op;
}
@@ -2783,8 +2799,13 @@ void NewGVN::valueNumberInstruction(Inst
if (Symbolized && !isa<ConstantExpression>(Symbolized) &&
!isa<VariableExpression>(Symbolized) && PHINodeUses.count(I)) {
auto *PHIE = makePossiblePhiOfOps(I, Visited);
- if (PHIE)
+ // If we created a phi of ops, use it.
+ // If we couldn't create one, make sure we don't leave one lying around
+ if (PHIE) {
Symbolized = PHIE;
+ } else if (auto *Op = RealToTemp.lookup(I)) {
+ removePhiOfOps(I, Op);
+ }
}
} else {
@@ -3626,7 +3647,7 @@ bool NewGVN::eliminateInstructions(Funct
CC->swap(MembersLeft);
} else {
// If this is a singleton, we can skip it.
- if (CC->size() != 1 || RealToTemp.lookup(Leader)) {
+ if (CC->size() != 1 || RealToTemp.count(Leader)) {
// This is a stack because equality replacement/etc may place
// constants in the middle of the member list, and we want to use
// those constant values in preference to the current leader, over
Added: llvm/trunk/test/Transforms/NewGVN/pr33196.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/pr33196.ll?rev=309988&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/pr33196.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/pr33196.ll Thu Aug 3 14:17:49 2017
@@ -0,0 +1,72 @@
+; RUN: opt -S -newgvn %s | FileCheck %s
+
+; CHECK: define i32 @main() {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: %tmp = load i32, i32* @d, align 4
+; CHECK-NEXT: %tmp1 = load i32, i32* @c, align 4
+; CHECK-NEXT: %tobool = icmp eq i32 %tmp1, -1
+; CHECK-NEXT: br i1 %tobool, label %if.end, label %if.then
+; CHECK: if.then:
+; CHECK-NEXT: br label %L
+; CHECK: L:
+; CHECK-NEXT: %e.0 = phi i32 [ 0, %if.then ], [ %e.1, %if.then4 ]
+; CHECK-NEXT: br label %if.end
+; CHECK: if.end:
+; CHECK-NEXT: %e.1 = phi i32 [ %e.0, %L ], [ %tmp, %entry ]
+; CHECK-NEXT: store i32 %e.1, i32* @a, align 4
+; CHECK-NEXT: %tmp2 = load i32, i32* @b, align 4
+; CHECK-NEXT: store i32 0, i32* @b, align 4
+; CHECK-NEXT: %sext = shl i32 %tmp2, 16
+; CHECK-NEXT: %conv1 = ashr exact i32 %sext, 16
+; CHECK-NEXT: %add = add nsw i32 %conv1, %tmp1
+; CHECK-NEXT: %add2 = add nsw i32 %add, %e.1
+; CHECK-NEXT: store i32 %add2, i32* @a, align 4
+; CHECK-NEXT: %tobool3 = icmp eq i32 %add2, 0
+; CHECK-NEXT: br i1 %tobool3, label %if.end5, label %if.then4
+; CHECK: if.then4:
+; CHECK-NEXT: br label %L
+; CHECK: if.end5:
+; CHECK-NEXT: ret i32 0
+; CHECK-NEXT: }
+
+ at d = global i32 1, align 4
+ at c = common global i32 0, align 4
+ at a = common global i32 0, align 4
+ at b = common global i32 0, align 4
+
+define i32 @main() {
+entry:
+ %tmp = load i32, i32* @d, align 4
+ %tmp1 = load i32, i32* @c, align 4
+ %tobool = icmp eq i32 %tmp1, -1
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then: ; preds = %entry
+ br label %L
+
+L: ; preds = %if.then4, %if.then
+ %e.0 = phi i32 [ 0, %if.then ], [ %e.1, %if.then4 ]
+ br label %if.end
+
+if.end: ; preds = %L, %entry
+ %e.1 = phi i32 [ %e.0, %L ], [ %tmp, %entry ]
+ store i32 %e.1, i32* @a, align 4
+ %tmp2 = load i32, i32* @b, align 4
+ store i32 0, i32* @b, align 4
+ %sext = shl i32 %tmp2, 16
+ %conv1 = ashr exact i32 %sext, 16
+ %tmp3 = load i32, i32* @c, align 4
+ %add = add nsw i32 %conv1, %tmp3
+ %tmp4 = load i32, i32* @a, align 4
+ %and = and i32 %tmp4, %e.1
+ %add2 = add nsw i32 %add, %and
+ store i32 %add2, i32* @a, align 4
+ %tobool3 = icmp eq i32 %add2, 0
+ br i1 %tobool3, label %if.end5, label %if.then4
+
+if.then4: ; preds = %if.end
+ br label %L
+
+if.end5: ; preds = %if.end
+ ret i32 0
+}
More information about the llvm-commits
mailing list