[llvm] 5f1448f - [GVN] Improve PRE on load instructions

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 10:40:01 PST 2023


I believe this patch to be incorrect, and need reverted.

The case I'm concerned about is that this patch appears to hoist a load 
over instructions which may throw.  The load at the original location 
may be dynamically dead because the exception path is always taken.  
Moving said load into the executed path can introduce a new fault in the 
program.

Philip

On 1/25/23 11:47, Guozhi Wei via llvm-commits wrote:
> Author: Guozhi Wei
> Date: 2023-01-25T19:45:01Z
> New Revision: 5f1448fe1585b5677d5f0064e4eeac3b493d8a18
>
> URL: https://github.com/llvm/llvm-project/commit/5f1448fe1585b5677d5f0064e4eeac3b493d8a18
> DIFF: https://github.com/llvm/llvm-project/commit/5f1448fe1585b5677d5f0064e4eeac3b493d8a18.diff
>
> LOG: [GVN] Improve PRE on load instructions
>
> This patch implements the enhancement proposed by
> https://github.com/llvm/llvm-project/issues/59312.
>
> Suppose we have following code
>
>     v0 = load %addr
>     br %LoadBB
>
>   LoadBB:
>     v1 = load %addr
>     ...
>
>   PredBB:
>     ...
>     br %cond, label %LoadBB, label %SuccBB
>
>   SuccBB:
>     v2 = load %addr
>     ...
>
> Instruction v1 in LoadBB is partially redundant, edge (PredBB, LoadBB) is a
> critical edge. SuccBB is another successor of PredBB, it contains another load
> v2 which is identical to v1. Current GVN splits the critical edge
> (PredBB, LoadBB) and inserts a new load in it. A better method is move the load
> of v2 into PredBB, then v1 can be changed to a PHI instruction.
>
> If there are two or more similar predecessors, like the test case in the bug
> entry, current GVN simply gives up because otherwise it needs to split multiple
> critical edges. But we can move all loads in successor blocks into predecessors.
>
> Differential Revision: https://reviews.llvm.org/D141712
>
> Added:
>      
>
> Modified:
>      llvm/include/llvm/Transforms/Scalar/GVN.h
>      llvm/lib/Transforms/Scalar/GVN.cpp
>      llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll
>      llvm/test/Transforms/GVN/PRE/2017-06-28-pre-load-dbgloc.ll
>      llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
>      llvm/test/Transforms/GVN/PRE/pre-load.ll
>      llvm/test/Transforms/GVN/PRE/volatile.ll
>      llvm/test/Transforms/GVN/condprop.ll
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
> index 4666a53156163..44cf9155dc559 100644
> --- a/llvm/include/llvm/Transforms/Scalar/GVN.h
> +++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
> @@ -329,6 +329,11 @@ class GVNPass : public PassInfoMixin<GVNPass> {
>                                  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);
>   
> @@ -342,7 +347,8 @@ class GVNPass : public PassInfoMixin<GVNPass> {
>     /// AvailableLoads (connected by Phis if needed).
>     void eliminatePartiallyRedundantLoad(
>         LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
> -      MapVector<BasicBlock *, Value *> &AvailableLoads);
> +      MapVector<BasicBlock *, Value *> &AvailableLoads,
> +      MapVector<BasicBlock *, LoadInst *> *CriticalEdgePredAndLoad);
>   
>     // Other helper routines
>     bool processInstruction(Instruction *I);
>
> diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
> index 6158894e34370..c01e9e5d0af6a 100644
> --- a/llvm/lib/Transforms/Scalar/GVN.cpp
> +++ b/llvm/lib/Transforms/Scalar/GVN.cpp
> @@ -94,6 +94,8 @@ STATISTIC(NumGVNSimpl, "Number of instructions simplified");
>   STATISTIC(NumGVNEqProp, "Number of equalities propagated");
>   STATISTIC(NumPRELoad, "Number of loads PRE'd");
>   STATISTIC(NumPRELoopLoad, "Number of loop loads PRE'd");
> +STATISTIC(NumPRELoadMoved2CEPred,
> +          "Number of loads moved to predecessor of a critical edge in PRE");
>   
>   STATISTIC(IsValueFullyAvailableInBlockNumSpeculationsMax,
>             "Number of blocks speculated as available in "
> @@ -127,6 +129,11 @@ static cl::opt<uint32_t> MaxNumVisitedInsts(
>       cl::desc("Max number of visited instructions when trying to find "
>                "dominating value of select dependency (default = 100)"));
>   
> +static cl::opt<uint32_t> 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;
> @@ -930,6 +937,21 @@ static bool IsValueFullyAvailableInBlock(
>     return !UnavailableBB;
>   }
>   
> +/// If the specified (BB, OldValue) exists in ValuesPerBlock, replace its value
> +/// with NewValue, otherwise we don't change it.
> +static void replaceValuesPerBlockEntry(
> +    SmallVectorImpl<AvailableValueInBlock> &ValuesPerBlock, BasicBlock *BB,
> +    Value *OldValue, Value *NewValue) {
> +  for (AvailableValueInBlock &V : ValuesPerBlock) {
> +    if (V.BB == BB) {
> +      if ((V.AV.isSimpleValue() && V.AV.getSimpleValue() == OldValue) ||
> +         (V.AV.isCoercedLoadValue() && V.AV.getCoercedLoadValue() == OldValue))
> +        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.
> @@ -1314,9 +1336,63 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load, LoadDepVect &Deps,
>            "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 || Term->isExceptionalTerminator())
> +    return nullptr;
> +  auto *SuccBB = Term->getSuccessor(0);
> +  if (SuccBB == LoadBB)
> +    SuccBB = Term->getSuccessor(1);
> +  if (!SuccBB->getSinglePredecessor())
> +    return nullptr;
> +
> +  unsigned int NumInsts = MaxNumInsnsPerBlock;
> +  for (Instruction &Inst : *SuccBB) {
> +    if (--NumInsts == 0)
> +      return nullptr;
> +
> +    if (!Inst.isIdenticalTo(Load))
> +      continue;
> +
> +    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<LoadInst>(&Inst);
> +
> +    // Otherwise there is something in the same BB clobbers the memory, we can't
> +    // move this and later load to PredBB.
> +    return nullptr;
> +  }
> +
> +  return nullptr;
> +}
> +
>   void GVNPass::eliminatePartiallyRedundantLoad(
>       LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
> -    MapVector<BasicBlock *, Value *> &AvailableLoads) {
> +    MapVector<BasicBlock *, Value *> &AvailableLoads,
> +    MapVector<BasicBlock *, LoadInst *> *CriticalEdgePredAndLoad) {
>     for (const auto &AvailableLoad : AvailableLoads) {
>       BasicBlock *UnavailableBlock = AvailableLoad.first;
>       Value *LoadPtr = AvailableLoad.second;
> @@ -1370,6 +1446,24 @@ void GVNPass::eliminatePartiallyRedundantLoad(
>           AvailableValueInBlock::get(UnavailableBlock, NewLoad));
>       MD->invalidateCachedPointerInfo(LoadPtr);
>       LLVM_DEBUG(dbgs() << "GVN INSERTED " << *NewLoad << '\n');
> +
> +    // For PredBB in CriticalEdgePredAndLoad we need to replace the uses of old
> +    // load instruction with the new created load instruction.
> +    if (CriticalEdgePredAndLoad) {
> +      auto I = CriticalEdgePredAndLoad->find(UnavailableBlock);
> +      if (I != CriticalEdgePredAndLoad->end()) {
> +        ++NumPRELoadMoved2CEPred;
> +        ICF->insertInstructionTo(NewLoad, UnavailableBlock);
> +        LoadInst *OldLoad = I->second;
> +        OldLoad->replaceAllUsesWith(NewLoad);
> +        replaceValuesPerBlockEntry(ValuesPerBlock, OldLoad->getParent(),
> +                                   OldLoad, NewLoad);
> +        if (uint32_t ValNo = VN.lookup(OldLoad, false))
> +          removeFromLeaderTable(ValNo, OldLoad, OldLoad->getParent());
> +        // To avoid deleting an instruction from
> diff erent BB, we just leave
> +        // the dead load here, it will be deleted in next iteration.
> +      }
> +    }
>     }
>   
>     // Perform PHI construction.
> @@ -1456,7 +1550,12 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>     for (BasicBlock *UnavailableBB : UnavailableBlocks)
>       FullyAvailableBlocks[UnavailableBB] = AvailabilityState::Unavailable;
>   
> -  SmallVector<BasicBlock *, 4> CriticalEdgePred;
> +  // The edge from Pred to LoadBB is a critical edge will be splitted.
> +  SmallVector<BasicBlock *, 4> 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<BasicBlock *, LoadInst *> 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.
> @@ -1496,7 +1595,10 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>             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;
> @@ -1504,31 +1606,37 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>     }
>   
>     // 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;
> @@ -1536,6 +1644,9 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>                         << 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();
> @@ -1592,7 +1703,7 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>       }
>       // 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
> @@ -1617,7 +1728,8 @@ bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
>       VN.lookupOrAdd(I);
>     }
>   
> -  eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, PredLoads);
> +  eliminatePartiallyRedundantLoad(Load, ValuesPerBlock, PredLoads,
> +                                  &CriticalEdgePredAndLoad);
>     ++NumPRELoad;
>     return true;
>   }
> @@ -1696,7 +1808,8 @@ bool GVNPass::performLoopLoadPRE(LoadInst *Load,
>     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;
>   }
>
> diff  --git a/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll b/llvm/test/Transforms/GVN/PRE/2011-06-01-NonLocalMemdepMiscompile.ll
> index 951ee21a4594b..3f0475dc79ca2 100644
> --- 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 @@ bb15:
>   ; 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
> index fc13782b66c88..b2b0216ed8f72 100644
> --- 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 @@ define i32 @test(ptr readonly %desc) local_unnamed_addr #0 !dbg !4 {
>   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 @@ declare i32 @bar(ptr, ptr) local_unnamed_addr #1
>   !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/2018-06-08-pre-load-dbgloc-no-null-opt.ll b/llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
> index fe6099ebf38d6..2f63ed0016c2b 100644
> --- a/llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
> +++ b/llvm/test/Transforms/GVN/PRE/2018-06-08-pre-load-dbgloc-no-null-opt.ll
> @@ -35,9 +35,9 @@ define i32 @test_no_null_opt(ptr readonly %desc) local_unnamed_addr #0 !dbg !4 {
>   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:
> -; ALL: load ptr, ptr null, align {{[0-9]+}}, !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
> @@ -45,8 +45,7 @@ cond.false:
>     br label %cond.end, !dbg !9
>   
>   cond.end:
> -; ALL: phi ptr [ %0, %cond.false ], [ %.pre, %entry.cond.end_crit_edge ]
> -; ALL: phi ptr [ %1, %cond.false ], [ null, %entry.cond.end_crit_edge ]
> +; ALL: phi ptr [ %0, %cond.false ], [ null, %entry ]
>   
>     %2 = phi ptr [ %1, %cond.false ], [ null, %entry ], !dbg !9
>     %3 = load ptr, ptr %desc, align 8, !dbg !10
> @@ -75,5 +74,5 @@ declare i32 @bar(ptr, ptr) local_unnamed_addr #1
>   !11 = !DILocation(line: 15, column: 34, scope: !4)
>   
>   ;ALL: [[SCOPE:![0-9]+]] = distinct  !DISubprogram(name: "test_no_null_opt",{{.*}}
> -;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
> index 5d031d317e39d..623f7c03d2e0e 100644
> --- a/llvm/test/Transforms/GVN/PRE/pre-load.ll
> +++ b/llvm/test/Transforms/GVN/PRE/pre-load.ll
> @@ -687,18 +687,14 @@ define i32 @test15(ptr noalias nocapture readonly dereferenceable(8) align 4 %x,
>   ; 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 @@ define i32 @test16(ptr noalias nocapture readonly dereferenceable(8) align 4 %x,
>   ; 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 @@ define void @test17(ptr %p1, ptr %p2, ptr %p3, ptr %p4)
>   ; 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
>   ;
>   {
> @@ -843,3 +835,172 @@ bb201:
>     store i64 %v5, ptr %p4, align 8
>     ret void
>   }
> +
> +; The output value from %if.then block is %dec, not loaded %v1.
> +; So ValuesPerBlock[%if.then] should not be replaced when the load instruction
> +; is moved to %entry.
> +define void @test18(i1 %cond, ptr %p1, ptr %p2) {
> +; CHECK-LABEL: @test18(
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    [[V2_PRE:%.*]] = load i16, ptr [[P1:%.*]], align 2
> +; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
> +; CHECK:       if.then:
> +; CHECK-NEXT:    [[DEC:%.*]] = add i16 [[V2_PRE]], -1
> +; CHECK-NEXT:    store i16 [[DEC]], ptr [[P1]], align 2
> +; CHECK-NEXT:    br label [[IF_END]]
> +; CHECK:       if.end:
> +; CHECK-NEXT:    [[V2:%.*]] = phi i16 [ [[DEC]], [[IF_THEN]] ], [ [[V2_PRE]], [[ENTRY:%.*]] ]
> +; CHECK-NEXT:    store i16 [[V2]], ptr [[P2:%.*]], align 2
> +; CHECK-NEXT:    ret void
> +;
> +entry:
> +  br i1 %cond, label %if.end, label %if.then
> +
> +if.then:
> +  %v1 = load i16, ptr %p1
> +  %dec = add i16 %v1, -1
> +  store i16 %dec, ptr %p1
> +  br label %if.end
> +
> +if.end:
> +  %v2 = load i16, ptr %p1
> +  store i16 %v2, ptr %p2
> +  ret void
> +}
> +
> +; PRE of load instructions should not cross exception handling instructions.
> +define void @test19(i1 %cond, ptr %p1, ptr %p2)
> +; CHECK-LABEL: @test19(
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    br i1 [[COND:%.*]], label [[THEN:%.*]], label [[ELSE:%.*]]
> +; CHECK:       then:
> +; CHECK-NEXT:    [[V2:%.*]] = load i64, ptr [[P2:%.*]], align 8
> +; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[V2]], 1
> +; CHECK-NEXT:    store i64 [[ADD]], ptr [[P1:%.*]], align 8
> +; CHECK-NEXT:    br label [[END:%.*]]
> +; CHECK:       else:
> +; CHECK-NEXT:    invoke void @f()
> +; CHECK-NEXT:    to label [[ELSE_END_CRIT_EDGE:%.*]] unwind label [[LPAD:%.*]]
> +; CHECK:       else.end_crit_edge:
> +; CHECK-NEXT:    [[V1_PRE:%.*]] = load i64, ptr [[P1]], align 8
> +; CHECK-NEXT:    br label [[END]]
> +; CHECK:       end:
> +; CHECK-NEXT:    [[V1:%.*]] = phi i64 [ [[V1_PRE]], [[ELSE_END_CRIT_EDGE]] ], [ [[ADD]], [[THEN]] ]
> +; CHECK-NEXT:    [[AND:%.*]] = and i64 [[V1]], 100
> +; CHECK-NEXT:    store i64 [[AND]], ptr [[P2]], align 8
> +; CHECK-NEXT:    ret void
> +; CHECK:       lpad:
> +; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
> +; CHECK-NEXT:    cleanup
> +; CHECK-NEXT:    [[V3:%.*]] = load i64, ptr [[P1]], align 8
> +; CHECK-NEXT:    [[OR:%.*]] = or i64 [[V3]], 200
> +; CHECK-NEXT:    store i64 [[OR]], ptr [[P1]], align 8
> +; CHECK-NEXT:    resume { ptr, i32 } [[LP]]
> +;
> +  personality ptr @__CxxFrameHandler3 {
> +entry:
> +  br i1 %cond, label %then, label %else
> +
> +then:
> +  %v2 = load i64, ptr %p2
> +  %add = add i64 %v2, 1
> +  store i64 %add, ptr %p1
> +  br label %end
> +
> +else:
> +  invoke void @f()
> +  to label %end unwind label %lpad
> +
> +end:
> +  %v1 = load i64, ptr %p1
> +  %and = and i64 %v1, 100
> +  store i64 %and, ptr %p2
> +  ret void
> +
> +lpad:
> +  %lp = landingpad { ptr, i32 }
> +  cleanup
> +  %v3 = load i64, ptr %p1
> +  %or = or i64 %v3, 200
> +  store i64 %or, ptr %p1
> +  resume { ptr, i32 } %lp
> +}
> +
> +; A predecessor BB has both successors to the same BB, for simplicity we don't
> +; handle it, nothing should be changed.
> +define void @test20(i1 %cond, i1 %cond2, ptr %p1, ptr %p2) {
> +; CHECK-LABEL: @test20(
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
> +; CHECK:       if.then:
> +; CHECK-NEXT:    [[V1:%.*]] = load i16, ptr [[P1:%.*]], align 2
> +; CHECK-NEXT:    [[DEC:%.*]] = add i16 [[V1]], -1
> +; CHECK-NEXT:    store i16 [[DEC]], ptr [[P1]], align 2
> +; CHECK-NEXT:    br label [[IF_END:%.*]]
> +; CHECK:       if.else:
> +; CHECK-NEXT:    br i1 [[COND2:%.*]], label [[IF_END]], label [[IF_END]]
> +; CHECK:       if.end:
> +; CHECK-NEXT:    [[V2:%.*]] = load i16, ptr [[P1]], align 2
> +; CHECK-NEXT:    store i16 [[V2]], ptr [[P2:%.*]], align 2
> +; CHECK-NEXT:    ret void
> +;
> +entry:
> +  br i1 %cond, label %if.then, label %if.else
> +
> +if.then:
> +  %v1 = load i16, ptr %p1
> +  %dec = add i16 %v1, -1
> +  store i16 %dec, ptr %p1
> +  br label %if.end
> +
> +if.else:
> +  br i1 %cond2, label %if.end, label %if.end
> +
> +if.end:
> +  %v2 = load i16, ptr %p1
> +  store i16 %v2, ptr %p2
> +  ret void
> +}
> +
> +; More edges from the same BB to LoadBB. Don't change anything.
> +define void @test21(i1 %cond, i32 %code, ptr %p1, ptr %p2) {
> +; CHECK-LABEL: @test21(
> +; CHECK-NEXT:  entry:
> +; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
> +; CHECK:       if.then:
> +; CHECK-NEXT:    [[V1:%.*]] = load i16, ptr [[P1:%.*]], align 2
> +; CHECK-NEXT:    [[DEC:%.*]] = add i16 [[V1]], -1
> +; CHECK-NEXT:    store i16 [[DEC]], ptr [[P1]], align 2
> +; CHECK-NEXT:    br label [[IF_END:%.*]]
> +; CHECK:       if.else:
> +; CHECK-NEXT:    switch i32 [[CODE:%.*]], label [[IF_END]] [
> +; CHECK-NEXT:    i32 1, label [[IF_END]]
> +; CHECK-NEXT:    i32 2, label [[IF_END]]
> +; CHECK-NEXT:    i32 3, label [[IF_END]]
> +; CHECK-NEXT:    ]
> +; CHECK:       if.end:
> +; CHECK-NEXT:    [[V2:%.*]] = load i16, ptr [[P1]], align 2
> +; CHECK-NEXT:    store i16 [[V2]], ptr [[P2:%.*]], align 2
> +; CHECK-NEXT:    ret void
> +;
> +entry:
> +  br i1 %cond, label %if.then, label %if.else
> +
> +if.then:
> +  %v1 = load i16, ptr %p1
> +  %dec = add i16 %v1, -1
> +  store i16 %dec, ptr %p1
> +  br label %if.end
> +
> +if.else:
> +  switch i32 %code, label %if.end [
> +  i32 1, label %if.end
> +  i32 2, label %if.end
> +  i32 3, label %if.end
> +  ]
> +
> +if.end:
> +  %v2 = load i16, ptr %p1
> +  store i16 %v2, ptr %p2
> +  ret void
> +}
>
> diff  --git a/llvm/test/Transforms/GVN/PRE/volatile.ll b/llvm/test/Transforms/GVN/PRE/volatile.ll
> index 432bf82461267..532019119fdb7 100644
> --- a/llvm/test/Transforms/GVN/PRE/volatile.ll
> +++ b/llvm/test/Transforms/GVN/PRE/volatile.ll
> @@ -122,18 +122,14 @@ exit:
>   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
> index 0269e1a95a1bd..6b1e4d1060109 100644
> --- a/llvm/test/Transforms/GVN/condprop.ll
> +++ b/llvm/test/Transforms/GVN/condprop.ll
> @@ -521,19 +521,15 @@ define i32 @test13(ptr %ptr1, ptr %ptr2) {
>   ; 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:
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list