diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h --- a/llvm/include/llvm/Transforms/Scalar/GVN.h +++ b/llvm/include/llvm/Transforms/Scalar/GVN.h @@ -330,6 +330,11 @@ AvailValInBlkVect &ValuesPerBlock, UnavailBlkVect &UnavailableBlocks); + /// Given a critical edge from Pred to LoadBB, find a load instruction + /// which is identical to Load from another successor of Pred. + LoadInst *findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB, + LoadInst *Load); + bool PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock, UnavailBlkVect &UnavailableBlocks); @@ -343,7 +348,8 @@ /// AvailableLoads (connected by Phis if needed). void eliminatePartiallyRedundantLoad( LoadInst *Load, AvailValInBlkVect &ValuesPerBlock, - MapVector &AvailableLoads); + MapVector &AvailableLoads, + MapVector *CriticalEdgePredAndLoad); // Other helper routines bool processInstruction(Instruction *I); diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -122,6 +122,11 @@ "into) when deducing if a value is fully available or not in GVN " "(default = 600)")); +static cl::opt MaxNumInsnsPerBlock( + "gvn-max-num-insns", cl::Hidden, cl::init(100), + cl::desc("Max number of instructions to scan in each basic block in GVN " + "(default = 100)")); + struct llvm::GVNPass::Expression { uint32_t opcode; bool commutative = false; @@ -920,6 +925,19 @@ return !UnavailableBB; } +/// If the specified BB exists in ValuesPerBlock, replace its value with +/// NewValue. +static void replaceValuesPerBlockEntry( + SmallVectorImpl &ValuesPerBlock, BasicBlock *BB, + Value *NewValue) { + for (AvailableValueInBlock &V : ValuesPerBlock) { + if (V.BB == BB) { + V = AvailableValueInBlock::get(BB, NewValue); + return; + } + } +} + /// Given a set of loads specified by ValuesPerBlock, /// construct SSA form, allowing us to eliminate Load. This returns the value /// that should be used at Load's definition site. @@ -1347,9 +1365,60 @@ "post condition violation"); } +/// Given the following code, v1 is partially available on some edges, but not +/// available on the edge from PredBB. This function tries to find if there is +/// another identical load in the other successor of PredBB. +/// +/// v0 = load %addr +/// br %LoadBB +/// +/// LoadBB: +/// v1 = load %addr +/// ... +/// +/// PredBB: +/// ... +/// br %cond, label %LoadBB, label %SuccBB +/// +/// SuccBB: +/// v2 = load %addr +/// ... +/// +LoadInst *GVNPass::findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB, + LoadInst *Load) { + // For simplicity we handle a Pred has 2 successors only. + auto *Term = Pred->getTerminator(); + if (Term->getNumSuccessors() != 2) + return nullptr; + auto *SuccBB = Term->getSuccessor(0); + if (SuccBB == LoadBB) + SuccBB = Term->getSuccessor(1); + if (!SuccBB->getSinglePredecessor()) + return nullptr; + + int NumInsts = MaxNumInsnsPerBlock; + for (Instruction &Inst : *SuccBB) { + if (Inst.isIdenticalTo(Load)) { + MemDepResult Dep = MD->getDependency(&Inst); + // If an identical load doesn't depends on any local instructions, it can + // be safely moved to PredBB. + if (Dep.isNonLocal()) + return cast(&Inst); + + return nullptr; + } + + if (--NumInsts == 0) + return nullptr; + } + + return nullptr; +} + void GVNPass::eliminatePartiallyRedundantLoad( LoadInst *Load, AvailValInBlkVect &ValuesPerBlock, - MapVector &AvailableLoads) { + MapVector &AvailableLoads, + MapVector *CriticalEdgePredAndLoad) { for (const auto &AvailableLoad : AvailableLoads) { BasicBlock *UnavailableBlock = AvailableLoad.first; Value *LoadPtr = AvailableLoad.second; @@ -1403,6 +1472,21 @@ AvailableValueInBlock::get(UnavailableBlock, NewLoad)); MD->invalidateCachedPointerInfo(LoadPtr); LLVM_DEBUG(dbgs() << "GVN INSERTED " << *NewLoad << '\n'); + + // For PredBB in CriticalEdgePredAndLoad we need to delete the already found + // load instruction which is now redundant. + if (CriticalEdgePredAndLoad) { + auto I = CriticalEdgePredAndLoad->find(UnavailableBlock); + if (I != CriticalEdgePredAndLoad->end()) { + LoadInst *OldLoad = I->second; + OldLoad->replaceAllUsesWith(NewLoad); + replaceValuesPerBlockEntry(ValuesPerBlock, OldLoad->getParent(), + NewLoad); + markInstructionForDeletion(OldLoad); + if (uint32_t ValNo = VN.lookup(OldLoad, false)) + removeFromLeaderTable(ValNo, OldLoad, OldLoad->getParent()); + } + } } // Perform PHI construction. @@ -1489,7 +1573,12 @@ for (BasicBlock *UnavailableBB : UnavailableBlocks) FullyAvailableBlocks[UnavailableBB] = AvailabilityState::Unavailable; - SmallVector CriticalEdgePred; + // The edge from Pred to LoadBB is a critical edge will be splitted. + SmallVector CriticalEdgePredSplit; + // The edge from Pred to LoadBB is a critical edge, another successor of Pred + // contains a load can be moved to Pred. This data structure maps the Pred to + // the movable load. + MapVector CriticalEdgePredAndLoad; for (BasicBlock *Pred : predecessors(LoadBB)) { // If any predecessor block is an EH pad that does not allow non-PHI // instructions before the terminator, we can't PRE the load. @@ -1529,39 +1618,53 @@ return false; } - CriticalEdgePred.push_back(Pred); + if (LoadInst *LI = findLoadToHoistIntoPred(Pred, LoadBB, Load)) { + CriticalEdgePredAndLoad[Pred] = LI; + } else + CriticalEdgePredSplit.push_back(Pred); } else { // Only add the predecessors that will not be split for now. PredLoads[Pred] = nullptr; } + + // Early check for non profitable PRE load. + unsigned NumInsertPreds = PredLoads.size() + CriticalEdgePredSplit.size(); + if (NumInsertPreds > 1) + return false; } // Decide whether PRE is profitable for this load. - unsigned NumUnavailablePreds = PredLoads.size() + CriticalEdgePred.size(); + unsigned NumInsertPreds = PredLoads.size() + CriticalEdgePredSplit.size(); + unsigned NumUnavailablePreds = NumInsertPreds + + CriticalEdgePredAndLoad.size(); assert(NumUnavailablePreds != 0 && "Fully available value should already be eliminated!"); - // If this load is unavailable in multiple predecessors, reject it. + // If we need to insert new load in multiple predecessors, reject it. // FIXME: If we could restructure the CFG, we could make a common pred with // all the preds that don't have an available Load and insert a new load into // that one block. - if (NumUnavailablePreds != 1) + if (NumInsertPreds > 1) return false; // Now we know where we will insert load. We must ensure that it is safe // to speculatively execute the load at that points. if (MustEnsureSafetyOfSpeculativeExecution) { - if (CriticalEdgePred.size()) + if (CriticalEdgePredSplit.size()) if (!isSafeToSpeculativelyExecute(Load, LoadBB->getFirstNonPHI(), AC, DT)) return false; for (auto &PL : PredLoads) if (!isSafeToSpeculativelyExecute(Load, PL.first->getTerminator(), AC, DT)) return false; + for (auto &CEP : CriticalEdgePredAndLoad) + if (!isSafeToSpeculativelyExecute(Load, CEP.first->getTerminator(), AC, + DT)) + return false; } // Split critical edges, and update the unavailable predecessors accordingly. - for (BasicBlock *OrigPred : CriticalEdgePred) { + for (BasicBlock *OrigPred : CriticalEdgePredSplit) { BasicBlock *NewPred = splitCriticalEdges(OrigPred, LoadBB); assert(!PredLoads.count(OrigPred) && "Split edges shouldn't be in map!"); PredLoads[NewPred] = nullptr; @@ -1569,6 +1672,9 @@ << LoadBB->getName() << '\n'); } + for (auto &CEP : CriticalEdgePredAndLoad) + PredLoads[CEP.first] = nullptr; + // Check if the load can safely be moved to all the unavailable predecessors. bool CanDoPRE = true; const DataLayout &DL = Load->getModule()->getDataLayout(); @@ -1625,7 +1731,7 @@ } // HINT: Don't revert the edge-splitting as following transformation may // also need to split these critical edges. - return !CriticalEdgePred.empty(); + return !CriticalEdgePredSplit.empty(); } // Okay, we can eliminate this load by inserting a reload in the predecessor @@ -1650,7 +1756,8 @@ VN.lookupOrAdd(I); } - eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, PredLoads); + eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, PredLoads, + &CriticalEdgePredAndLoad); ++NumPRELoad; return true; } @@ -1729,7 +1836,8 @@ AvailableLoads[Preheader] = LoadPtr; LLVM_DEBUG(dbgs() << "GVN REMOVING PRE LOOP LOAD: " << *Load << '\n'); - eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, AvailableLoads); + eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, AvailableLoads, + /*CriticalEdgePredAndLoad*/ nullptr); ++NumPRELoopLoad; return true; } @@ -2700,7 +2808,6 @@ --BI; for (auto *I : InstrsToErase) { - assert(I->getParent() == BB && "Removing instruction from wrong block?"); LLVM_DEBUG(dbgs() << "GVN removed: " << *I << '\n'); salvageKnowledge(I, AC); salvageDebugInfo(*I); diff --git a/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll b/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll --- a/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll +++ b/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll @@ -57,7 +57,7 @@ ; CHECK-NEXT: br label %bb15 ; CHECK-LABEL: bb15: -; CHECK: %tmp17 = phi i8 [ %tmp8, %bb15split ], [ %tmp17.pre, %bb1.bb15_crit_edge ] +; CHECK: %tmp17 = phi i8 [ %tmp12.pre3, %bb15split ], [ %tmp17.pre, %bb1.bb15_crit_edge ] bb19: ; preds = %bb15 ret i1 %tmp18 diff --git a/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll b/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll --- a/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll +++ b/llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll @@ -1,6 +1,6 @@ ; This test checks if debug loc is propagated to load/store created by GVN/Instcombine. -; RUN: opt < %s -passes=gvn -S | FileCheck %s --check-prefixes=ALL,GVN -; RUN: opt < %s -passes=gvn,instcombine -S | FileCheck %s --check-prefixes=ALL,INSTCOMBINE +; RUN: opt < %s -passes=gvn -S | FileCheck %s --check-prefixes=ALL +; RUN: opt < %s -passes=gvn,instcombine -S | FileCheck %s --check-prefixes=ALL ; struct node { ; int *v; @@ -35,10 +35,9 @@ entry: %tobool = icmp eq ptr %desc, null br i1 %tobool, label %cond.end, label %cond.false, !dbg !9 -; ALL: br i1 %tobool, label %entry.cond.end_crit_edge, label %cond.false, !dbg [[LOC_15_6:![0-9]+]] -; ALL: entry.cond.end_crit_edge: -; GVN: %.pre = load ptr, ptr null, align 8, !dbg [[LOC_16_13:![0-9]+]] -; INSTCOMBINE:store ptr poison, ptr null, align 4294967296, !dbg [[LOC_16_13:![0-9]+]] +; ALL: %.pre = load ptr, ptr %desc, align 8, !dbg [[LOC_16_13:![0-9]+]] +; ALL: br i1 %tobool, label %cond.end, label %cond.false, !dbg [[LOC_15_6:![0-9]+]] +; ALL: cond.false: cond.false: %0 = load ptr, ptr %desc, align 8, !dbg !11 @@ -72,5 +71,5 @@ !11 = !DILocation(line: 15, column: 34, scope: !4) ;ALL: [[SCOPE:![0-9]+]] = distinct !DISubprogram(name: "test",{{.*}} -;ALL: [[LOC_15_6]] = !DILocation(line: 15, column: 6, scope: [[SCOPE]]) ;ALL: [[LOC_16_13]] = !DILocation(line: 16, column: 13, scope: [[SCOPE]]) +;ALL: [[LOC_15_6]] = !DILocation(line: 15, column: 6, scope: [[SCOPE]]) diff --git a/llvm/test/Transforms/GVN/PRE/pre-load.ll b/llvm/test/Transforms/GVN/PRE/pre-load.ll --- a/llvm/test/Transforms/GVN/PRE/pre-load.ll +++ b/llvm/test/Transforms/GVN/PRE/pre-load.ll @@ -687,18 +687,14 @@ ; CHECK-LABEL: @test15( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[A:%.*]], 0 -; CHECK-NEXT: br i1 [[TOBOOL]], label [[ENTRY_IF_END_CRIT_EDGE:%.*]], label [[IF_THEN:%.*]] -; CHECK: entry.if.end_crit_edge: ; CHECK-NEXT: [[VV_PRE:%.*]] = load i32, ptr [[X:%.*]], align 4 -; CHECK-NEXT: br label [[IF_END:%.*]] +; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]] ; CHECK: if.then: -; CHECK-NEXT: [[UU:%.*]] = load i32, ptr [[X]], align 4 -; CHECK-NEXT: store i32 [[UU]], ptr [[R:%.*]], align 4 +; CHECK-NEXT: store i32 [[VV_PRE]], ptr [[R:%.*]], align 4 ; CHECK-NEXT: br label [[IF_END]] ; CHECK: if.end: -; CHECK-NEXT: [[VV:%.*]] = phi i32 [ [[VV_PRE]], [[ENTRY_IF_END_CRIT_EDGE]] ], [ [[UU]], [[IF_THEN]] ] ; CHECK-NEXT: call void @f() -; CHECK-NEXT: ret i32 [[VV]] +; CHECK-NEXT: ret i32 [[VV_PRE]] ; entry: @@ -728,18 +724,14 @@ ; CHECK-LABEL: @test16( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[A:%.*]], 0 -; CHECK-NEXT: br i1 [[TOBOOL]], label [[ENTRY_IF_END_CRIT_EDGE:%.*]], label [[IF_THEN:%.*]] -; CHECK: entry.if.end_crit_edge: ; CHECK-NEXT: [[VV_PRE:%.*]] = load i32, ptr [[X:%.*]], align 4 -; CHECK-NEXT: br label [[IF_END:%.*]] +; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]] ; CHECK: if.then: -; CHECK-NEXT: [[UU:%.*]] = load i32, ptr [[X]], align 4 -; CHECK-NEXT: store i32 [[UU]], ptr [[R:%.*]], align 4 +; CHECK-NEXT: store i32 [[VV_PRE]], ptr [[R:%.*]], align 4 ; CHECK-NEXT: br label [[IF_END]] ; CHECK: if.end: -; CHECK-NEXT: [[VV:%.*]] = phi i32 [ [[VV_PRE]], [[ENTRY_IF_END_CRIT_EDGE]] ], [ [[UU]], [[IF_THEN]] ] ; CHECK-NEXT: call void @f() -; CHECK-NEXT: ret i32 [[VV]] +; CHECK-NEXT: ret i32 [[VV_PRE]] ; entry: @@ -787,22 +779,22 @@ ; CHECK-NEXT: store i64 [[V2]], ptr [[P1]], align 8 ; CHECK-NEXT: br label [[BB3:%.*]] ; CHECK: bb3: -; CHECK-NEXT: [[V3:%.*]] = load i64, ptr [[P1]], align 8 +; CHECK-NEXT: [[V3:%.*]] = phi i64 [ [[V3_PRE:%.*]], [[BB200]] ], [ [[V3_PRE1:%.*]], [[BB100]] ], [ [[V2]], [[BB2]] ] ; CHECK-NEXT: store i64 [[V3]], ptr [[P2:%.*]], align 8 ; CHECK-NEXT: ret void ; CHECK: bb100: ; CHECK-NEXT: [[COND3:%.*]] = call i1 @foo() +; CHECK-NEXT: [[V3_PRE1]] = load i64, ptr [[P1]], align 8 ; CHECK-NEXT: br i1 [[COND3]], label [[BB3]], label [[BB101:%.*]] ; CHECK: bb101: -; CHECK-NEXT: [[V4:%.*]] = load i64, ptr [[P1]], align 8 -; CHECK-NEXT: store i64 [[V4]], ptr [[P3:%.*]], align 8 +; CHECK-NEXT: store i64 [[V3_PRE1]], ptr [[P3:%.*]], align 8 ; CHECK-NEXT: ret void ; CHECK: bb200: ; CHECK-NEXT: [[COND4:%.*]] = call i1 @bar() +; CHECK-NEXT: [[V3_PRE]] = load i64, ptr [[P1]], align 8 ; CHECK-NEXT: br i1 [[COND4]], label [[BB3]], label [[BB201:%.*]] ; CHECK: bb201: -; CHECK-NEXT: [[V5:%.*]] = load i64, ptr [[P1]], align 8 -; CHECK-NEXT: store i64 [[V5]], ptr [[P4:%.*]], align 8 +; CHECK-NEXT: store i64 [[V3_PRE]], ptr [[P4:%.*]], align 8 ; CHECK-NEXT: ret void ; { diff --git a/llvm/test/Transforms/GVN/PRE/volatile.ll b/llvm/test/Transforms/GVN/PRE/volatile.ll --- a/llvm/test/Transforms/GVN/PRE/volatile.ll +++ b/llvm/test/Transforms/GVN/PRE/volatile.ll @@ -122,18 +122,14 @@ define i32 @test7(i1 %c, ptr noalias nocapture %p, ptr noalias nocapture %q) { ; CHECK-LABEL: @test7( ; CHECK-NEXT: entry: -; CHECK-NEXT: br i1 [[C:%.*]], label [[ENTRY_HEADER_CRIT_EDGE:%.*]], label [[SKIP:%.*]] -; CHECK: entry.header_crit_edge: ; CHECK-NEXT: [[Y_PRE:%.*]] = load i32, ptr [[P:%.*]], align 4 -; CHECK-NEXT: br label [[HEADER:%.*]] +; CHECK-NEXT: br i1 [[C:%.*]], label [[HEADER:%.*]], label [[SKIP:%.*]] ; CHECK: skip: -; CHECK-NEXT: [[Y1:%.*]] = load i32, ptr [[P]], align 4 -; CHECK-NEXT: call void @use(i32 [[Y1]]) +; CHECK-NEXT: call void @use(i32 [[Y_PRE]]) ; CHECK-NEXT: br label [[HEADER]] ; CHECK: header: -; CHECK-NEXT: [[Y:%.*]] = phi i32 [ [[Y_PRE]], [[ENTRY_HEADER_CRIT_EDGE]] ], [ [[Y]], [[HEADER]] ], [ [[Y1]], [[SKIP]] ] ; CHECK-NEXT: [[X:%.*]] = load volatile i32, ptr [[Q:%.*]], align 4 -; CHECK-NEXT: [[ADD:%.*]] = sub i32 [[Y]], [[X]] +; CHECK-NEXT: [[ADD:%.*]] = sub i32 [[Y_PRE]], [[X]] ; CHECK-NEXT: [[CND:%.*]] = icmp eq i32 [[ADD]], 0 ; CHECK-NEXT: br i1 [[CND]], label [[EXIT:%.*]], label [[HEADER]] ; CHECK: exit: diff --git a/llvm/test/Transforms/GVN/condprop.ll b/llvm/test/Transforms/GVN/condprop.ll --- a/llvm/test/Transforms/GVN/condprop.ll +++ b/llvm/test/Transforms/GVN/condprop.ll @@ -521,19 +521,15 @@ ; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i32, ptr [[PTR2:%.*]], i32 1 ; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i32, ptr [[PTR2]], i32 2 ; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[PTR1:%.*]], [[PTR2]] -; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[ENTRY_END_CRIT_EDGE:%.*]] -; CHECK: entry.end_crit_edge: ; CHECK-NEXT: [[VAL2_PRE:%.*]] = load i32, ptr [[GEP2]], align 4 -; CHECK-NEXT: br label [[END:%.*]] +; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[END:%.*]] ; CHECK: if: -; CHECK-NEXT: [[VAL1:%.*]] = load i32, ptr [[GEP2]], align 4 ; CHECK-NEXT: br label [[END]] ; CHECK: end: -; CHECK-NEXT: [[VAL2:%.*]] = phi i32 [ [[VAL1]], [[IF]] ], [ [[VAL2_PRE]], [[ENTRY_END_CRIT_EDGE]] ] -; CHECK-NEXT: [[PHI1:%.*]] = phi ptr [ [[PTR2]], [[IF]] ], [ [[GEP1]], [[ENTRY_END_CRIT_EDGE]] ] -; CHECK-NEXT: [[PHI2:%.*]] = phi i32 [ [[VAL1]], [[IF]] ], [ 0, [[ENTRY_END_CRIT_EDGE]] ] +; CHECK-NEXT: [[PHI1:%.*]] = phi ptr [ [[PTR2]], [[IF]] ], [ [[GEP1]], [[ENTRY:%.*]] ] +; CHECK-NEXT: [[PHI2:%.*]] = phi i32 [ [[VAL2_PRE]], [[IF]] ], [ 0, [[ENTRY]] ] ; CHECK-NEXT: store i32 0, ptr [[PHI1]], align 4 -; CHECK-NEXT: [[RET:%.*]] = add i32 [[PHI2]], [[VAL2]] +; CHECK-NEXT: [[RET:%.*]] = add i32 [[PHI2]], [[VAL2_PRE]] ; CHECK-NEXT: ret i32 [[RET]] ; entry: