[llvm] r276714 - GVN-hoist: use a DFS numbering of instructions (PR28670)
Sebastian Pop via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 25 17:15:11 PDT 2016
Author: spop
Date: Mon Jul 25 19:15:10 2016
New Revision: 276714
URL: http://llvm.org/viewvc/llvm-project?rev=276714&view=rev
Log:
GVN-hoist: use a DFS numbering of instructions (PR28670)
Instead of DFS numbering basic blocks we now DFS number instructions that avoids
the costly operation of which instruction comes first in a basic block.
Patch mostly written by Daniel Berlin.
Differential Revision: https://reviews.llvm.org/D22777
Modified:
llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
llvm/trunk/test/Transforms/GVN/hoist.ll
Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=276714&r1=276713&r2=276714&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Mon Jul 25 19:15:10 2016
@@ -58,10 +58,10 @@ namespace {
// Provides a sorting function based on the execution order of two instructions.
struct SortByDFSIn {
private:
- DenseMap<const BasicBlock *, unsigned> &DFSNumber;
+ DenseMap<const Value *, unsigned> &DFSNumber;
public:
- SortByDFSIn(DenseMap<const BasicBlock *, unsigned> &D) : DFSNumber(D) {}
+ SortByDFSIn(DenseMap<const Value *, unsigned> &D) : DFSNumber(D) {}
// Returns true when A executes before B.
bool operator()(const Instruction *A, const Instruction *B) const {
@@ -72,18 +72,10 @@ public:
//
// assert(A != B);
- const BasicBlock *BA = A->getParent();
- const BasicBlock *BB = B->getParent();
- unsigned NA = DFSNumber[BA];
- unsigned NB = DFSNumber[BB];
- if (NA < NB)
- return true;
- if (NA == NB) {
- // Sort them in the order they occur in the same basic block.
- BasicBlock::const_iterator AI(A), BI(B);
- return std::distance(AI, BI) < 0;
- }
- return false;
+ unsigned ADFS = DFSNumber.lookup(A);
+ unsigned BDFS = DFSNumber.lookup(B);
+ assert (ADFS && ADFS);
+ return ADFS < BDFS;
}
};
@@ -201,10 +193,6 @@ public:
VN.setMemDep(MD);
bool Res = false;
- unsigned I = 0;
- for (const BasicBlock *BB : depth_first(&F.getEntryBlock()))
- DFSNumber.insert({BB, ++I});
-
// FIXME: use lazy evaluation of VN to avoid the fix-point computation.
while (1) {
// FIXME: only compute MemorySSA once. We need to update the analysis in
@@ -212,6 +200,12 @@ public:
MemorySSA M(F, AA, DT);
MSSA = &M;
+ // Perform DFS Numbering of instructions.
+ unsigned I = 0;
+ for (const BasicBlock *BB : depth_first(&F.getEntryBlock()))
+ for (auto &Inst: *BB)
+ DFSNumber.insert({&Inst, ++I});
+
auto HoistStat = hoistExpressions(F);
if (HoistStat.first + HoistStat.second == 0) {
return Res;
@@ -223,6 +217,9 @@ public:
VN.clear();
}
Res = true;
+
+ // DFS numbers change when instructions are hoisted: clear and recompute.
+ DFSNumber.clear();
}
return Res;
@@ -233,7 +230,7 @@ private:
AliasAnalysis *AA;
MemoryDependenceResults *MD;
const bool OptForMinSize;
- DenseMap<const BasicBlock *, unsigned> DFSNumber;
+ DenseMap<const Value *, unsigned> DFSNumber;
BBSideEffectsSet BBSideEffects;
MemorySSA *MSSA;
int HoistedCtr;
@@ -295,16 +292,14 @@ private:
}
/* Return true when I1 appears before I2 in the instructions of BB. */
- bool firstInBB(BasicBlock *BB, const Instruction *I1, const Instruction *I2) {
- for (Instruction &I : *BB) {
- if (&I == I1)
- return true;
- if (&I == I2)
- return false;
- }
-
- llvm_unreachable("I1 and I2 not found in BB");
+ bool firstInBB(const Instruction *I1, const Instruction *I2) {
+ assert (I1->getParent() == I2->getParent());
+ unsigned I1DFS = DFSNumber.lookup(I1);
+ unsigned I2DFS = DFSNumber.lookup(I2);
+ assert (I1DFS && I2DFS);
+ return I1DFS < I2DFS;
}
+
// Return true when there are users of Def in BB.
bool hasMemoryUseOnPath(MemoryAccess *Def, const BasicBlock *BB,
const Instruction *OldPt) {
@@ -328,7 +323,7 @@ private:
return true;
// It is only harmful to hoist when the use is before OldPt.
- if (firstInBB(UBB, MU->getMemoryInst(), OldPt))
+ if (firstInBB(MU->getMemoryInst(), OldPt))
return true;
}
@@ -442,7 +437,7 @@ private:
if (NewBB == DBB && !MSSA->isLiveOnEntryDef(D))
if (auto *UD = dyn_cast<MemoryUseOrDef>(D))
- if (firstInBB(DBB, NewPt, UD->getMemoryInst()))
+ if (firstInBB(NewPt, UD->getMemoryInst()))
// Cannot move the load or store to NewPt above its definition in D.
return false;
@@ -519,7 +514,7 @@ private:
if (BB == HoistBB) {
NewHoistBB = HoistBB;
- NewHoistPt = firstInBB(BB, Insn, HoistPt) ? Insn : HoistPt;
+ NewHoistPt = firstInBB(Insn, HoistPt) ? Insn : HoistPt;
} else {
NewHoistBB = DT->findNearestCommonDominator(HoistBB, BB);
if (NewHoistBB == BB)
Modified: llvm/trunk/test/Transforms/GVN/hoist.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/hoist.ll?rev=276714&r1=276713&r2=276714&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/hoist.ll (original)
+++ llvm/trunk/test/Transforms/GVN/hoist.ll Mon Jul 25 19:15:10 2016
@@ -631,17 +631,18 @@ if.end:
; should be hoisted.
; CHECK-LABEL: @hoistStores
; CHECK: zext
-; CHECK: trunc
-; CHECK: getelementptr
-; CHECK: load
-; CHECK: getelementptr
-; CHECK: store
-; CHECK: load
-; CHECK: load
-; CHECK: zext
-; CHECK: add
-; CHECK: store
-; CHECK: br
+; CHECK-NEXT: trunc
+; CHECK-NEXT: getelementptr
+; CHECK-NEXT: load
+; CHECK-NEXT: getelementptr
+; CHECK-NEXT: getelementptr
+; CHECK-NEXT: store
+; CHECK-NEXT: load
+; CHECK-NEXT: load
+; CHECK-NEXT: zext
+; CHECK-NEXT: add
+; CHECK-NEXT: store
+; CHECK-NEXT: br
; CHECK: if.then
; CHECK: br
More information about the llvm-commits
mailing list