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

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 11:49:20 PST 2023


You continue on any non-identical instruction.  That instruction can be 
a throwing call.

Philip

On 1/30/23 11:23, Carrot Wei wrote:
> Hi Philip
>
> Do you have a reduced test case to show your problem?
>
> This patch tries to check exception throwing instructions in the
> function findLoadToHoistIntoPred. What's your case that I missed?
>
> thanks
> Guozhi Wei
>
> On Mon, Jan 30, 2023 at 10:40 AM Philip Reames
> <listmail at philipreames.com> wrote:
>> 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