[llvm] r306252 - Revert "[LoopSimplify] Factor the logic to form dedicated exits into a utility."

Daniel Jasper via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 25 23:07:16 PDT 2017


Sure thing. Sorry about that.

On Mon, Jun 26, 2017 at 12:33 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> When reverting, please mention the revision being reverted rather than
> just the subject. That makes it *much* easier to find reverts.
>
> On Sun, Jun 25, 2017 at 10:58 AM Daniel Jasper via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: djasper
>> Date: Sun Jun 25 10:58:25 2017
>> New Revision: 306252
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=306252&view=rev
>> Log:
>> Revert "[LoopSimplify] Factor the logic to form dedicated exits into a
>> utility."
>>
>> This leads to a segfault. Chandler already has a test case and should be
>> able to recommit with a fix soon.
>>
>> Modified:
>>     llvm/trunk/include/llvm/Transforms/Utils/LoopUtils.h
>>     llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp
>>     llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp
>>     llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-SimpleSwitch.ll
>>     llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-
>> TwoSwitches-Threshold.ll
>>     llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-TwoSwitches.ll
>>
>> Modified: llvm/trunk/include/llvm/Transforms/Utils/LoopUtils.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>> llvm/Transforms/Utils/LoopUtils.h?rev=306252&r1=
>> 306251&r2=306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/Transforms/Utils/LoopUtils.h (original)
>> +++ llvm/trunk/include/llvm/Transforms/Utils/LoopUtils.h Sun Jun 25
>> 10:58:25 2017
>> @@ -362,14 +362,6 @@ private:
>>  BasicBlock *InsertPreheaderForLoop(Loop *L, DominatorTree *DT, LoopInfo
>> *LI,
>>                                     bool PreserveLCSSA);
>>
>> -/// Ensure that all exit blocks of the loop are dedicated exits.
>> -///
>> -/// For any loop exit block with non-loop predecessors, we split the loop
>> -/// predecessors to use a dedicated loop exit block. We update the
>> dominator
>> -/// tree and loop info if provided, and will preserve LCSSA if requested.
>> -bool formDedicatedExitBlocks(Loop *L, DominatorTree *DT, LoopInfo *LI,
>> -                             bool PreserveLCSSA);
>> -
>>  /// Ensures LCSSA form for every instruction from the Worklist in the
>> scope of
>>  /// innermost containing loop.
>>  ///
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Transforms/Utils/LoopSimplify.cpp?rev=306252&r1=306251&r2=
>> 306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/LoopSimplify.cpp Sun Jun 25 10:58:25
>> 2017
>> @@ -72,6 +72,7 @@ using namespace llvm;
>>
>>  #define DEBUG_TYPE "loop-simplify"
>>
>> +STATISTIC(NumInserted, "Number of pre-header or exit blocks inserted");
>>  STATISTIC(NumNested  , "Number of nested loops split out");
>>
>>  // If the block isn't already, move the new block to right after some
>> 'outside
>> @@ -151,6 +152,37 @@ BasicBlock *llvm::InsertPreheaderForLoop
>>    return PreheaderBB;
>>  }
>>
>> +/// \brief Ensure that the loop preheader dominates all exit blocks.
>> +///
>> +/// This method is used to split exit blocks that have predecessors
>> outside of
>> +/// the loop.
>> +static BasicBlock *rewriteLoopExitBlock(Loop *L, BasicBlock *Exit,
>> +                                        DominatorTree *DT, LoopInfo *LI,
>> +                                        bool PreserveLCSSA) {
>> +  SmallVector<BasicBlock*, 8> LoopBlocks;
>> +  for (pred_iterator I = pred_begin(Exit), E = pred_end(Exit); I != E;
>> ++I) {
>> +    BasicBlock *P = *I;
>> +    if (L->contains(P)) {
>> +      // Don't do this if the loop is exited via an indirect branch.
>> +      if (isa<IndirectBrInst>(P->getTerminator())) return nullptr;
>> +
>> +      LoopBlocks.push_back(P);
>> +    }
>> +  }
>> +
>> +  assert(!LoopBlocks.empty() && "No edges coming in from outside the
>> loop?");
>> +  BasicBlock *NewExitBB = nullptr;
>> +
>> +  NewExitBB = SplitBlockPredecessors(Exit, LoopBlocks, ".loopexit", DT,
>> LI,
>> +                                     PreserveLCSSA);
>> +  if (!NewExitBB)
>> +    return nullptr;
>> +
>> +  DEBUG(dbgs() << "LoopSimplify: Creating dedicated exit block "
>> +               << NewExitBB->getName() << "\n");
>> +  return NewExitBB;
>> +}
>> +
>>  /// Add the specified block, and all of its predecessors, to the
>> specified set,
>>  /// if it's not already in there.  Stop predecessor traversal when we
>> reach
>>  /// StopBlock.
>> @@ -314,7 +346,16 @@ static Loop *separateNestedLoop(Loop *L,
>>
>>    // Split edges to exit blocks from the inner loop, if they emerged in
>> the
>>    // process of separating the outer one.
>> -  formDedicatedExitBlocks(L, DT, LI, PreserveLCSSA);
>> +  SmallVector<BasicBlock *, 8> ExitBlocks;
>> +  L->getExitBlocks(ExitBlocks);
>> +  SmallSetVector<BasicBlock *, 8> ExitBlockSet(ExitBlocks.begin(),
>> +                                               ExitBlocks.end());
>> +  for (BasicBlock *ExitBlock : ExitBlockSet) {
>> +    if (any_of(predecessors(ExitBlock),
>> +               [L](BasicBlock *BB) { return !L->contains(BB); })) {
>> +      rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA);
>> +    }
>> +  }
>>
>>    if (PreserveLCSSA) {
>>      // Fix LCSSA form for L. Some values, which previously were only
>> used inside
>> @@ -522,16 +563,29 @@ ReprocessLoop:
>>    BasicBlock *Preheader = L->getLoopPreheader();
>>    if (!Preheader) {
>>      Preheader = InsertPreheaderForLoop(L, DT, LI, PreserveLCSSA);
>> -    if (Preheader)
>> +    if (Preheader) {
>> +      ++NumInserted;
>>        Changed = true;
>> +    }
>>    }
>>
>>    // Next, check to make sure that all exit nodes of the loop only have
>>    // predecessors that are inside of the loop.  This check guarantees
>> that the
>>    // loop preheader/header will dominate the exit blocks.  If the exit
>> block has
>>    // predecessors from outside of the loop, split the edge now.
>> -  if (formDedicatedExitBlocks(L, DT, LI, PreserveLCSSA))
>> -    Changed = true;
>> +  SmallVector<BasicBlock*, 8> ExitBlocks;
>> +  L->getExitBlocks(ExitBlocks);
>> +
>> +  SmallSetVector<BasicBlock *, 8> ExitBlockSet(ExitBlocks.begin(),
>> +                                               ExitBlocks.end());
>> +  for (BasicBlock *ExitBlock : ExitBlockSet) {
>> +    if (any_of(predecessors(ExitBlock),
>> +               [L](BasicBlock *BB) { return !L->contains(BB); })) {
>> +      rewriteLoopExitBlock(L, ExitBlock, DT, LI, PreserveLCSSA);
>> +      ++NumInserted;
>> +      Changed = true;
>> +    }
>> +  }
>>
>>    // If the header has more than two predecessors at this point (from the
>>    // preheader and from multiple backedges), we must adjust the loop.
>> @@ -560,8 +614,10 @@ ReprocessLoop:
>>      // insert a new block that all backedges target, then make it jump
>> to the
>>      // loop header.
>>      LoopLatch = insertUniqueBackedgeBlock(L, Preheader, DT, LI);
>> -    if (LoopLatch)
>> +    if (LoopLatch) {
>> +      ++NumInserted;
>>        Changed = true;
>> +    }
>>    }
>>
>>    const DataLayout &DL = L->getHeader()->getModule()->getDataLayout();
>> @@ -589,22 +645,7 @@ ReprocessLoop:
>>    // loop-invariant instructions out of the way to open up more
>>    // opportunities, and the disadvantage of having the responsibility
>>    // to preserve dominator information.
>> -  auto HasUniqueExitBlock = [&]() {
>> -    BasicBlock *UniqueExit = nullptr;
>> -    for (auto *ExitingBB : ExitingBlocks)
>> -      for (auto *SuccBB : successors(ExitingBB)) {
>> -        if (L->contains(SuccBB))
>> -          continue;
>> -
>> -        if (!UniqueExit)
>> -          UniqueExit = SuccBB;
>> -        else if (UniqueExit != SuccBB)
>> -          return false;
>> -      }
>> -
>> -    return true;
>> -  };
>> -  if (HasUniqueExitBlock()) {
>> +  if (ExitBlockSet.size() == 1) {
>>      for (unsigned i = 0, e = ExitingBlocks.size(); i != e; ++i) {
>>        BasicBlock *ExitingBlock = ExitingBlocks[i];
>>        if (!ExitingBlock->getSinglePredecessor()) continue;
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Transforms/Utils/LoopUtils.cpp?rev=306252&r1=306251&r2=306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp Sun Jun 25 10:58:25
>> 2017
>> @@ -29,7 +29,6 @@
>>  #include "llvm/IR/ValueHandle.h"
>>  #include "llvm/Pass.h"
>>  #include "llvm/Support/Debug.h"
>> -#include "llvm/Transforms/Utils/BasicBlockUtils.h"
>>
>>  using namespace llvm;
>>  using namespace llvm::PatternMatch;
>> @@ -923,67 +922,6 @@ bool InductionDescriptor::isInductionPHI
>>    return true;
>>  }
>>
>> -bool llvm::formDedicatedExitBlocks(Loop *L, DominatorTree *DT, LoopInfo
>> *LI,
>> -                                   bool PreserveLCSSA) {
>> -  bool Changed = false;
>> -
>> -  // We re-use a vector for the in-loop predecesosrs.
>> -  SmallVector<BasicBlock *, 4> InLoopPredecessors;
>> -
>> -  auto RewriteExit = [&](BasicBlock *BB) {
>> -    // See if there are any non-loop predecessors of this exit block and
>> -    // keep track of the in-loop predecessors.
>> -    bool IsDedicatedExit = true;
>> -    for (auto *PredBB : predecessors(BB))
>> -      if (L->contains(PredBB)) {
>> -        if (isa<IndirectBrInst>(PredBB->getTerminator()))
>> -          // We cannot rewrite exiting edges from an indirectbr.
>> -          return false;
>> -
>> -        InLoopPredecessors.push_back(PredBB);
>> -      } else {
>> -        IsDedicatedExit = false;
>> -      }
>> -
>> -    // Nothing to do if this is already a dedicated exit.
>> -    if (IsDedicatedExit) {
>> -      InLoopPredecessors.clear();
>> -      return false;
>> -    }
>> -
>> -    assert(!InLoopPredecessors.empty() && "Must have *some* loop
>> predecessor!");
>> -    auto *NewExitBB = SplitBlockPredecessors(
>> -        BB, InLoopPredecessors, ".loopexit", DT, LI, PreserveLCSSA);
>> -
>> -    if (!NewExitBB)
>> -      DEBUG(dbgs() << "WARNING: Can't create a dedicated exit block for
>> loop: "
>> -                   << *L << "\n");
>> -    else
>> -      DEBUG(dbgs() << "LoopSimplify: Creating dedicated exit block "
>> -                   << NewExitBB->getName() << "\n");
>> -    InLoopPredecessors.clear();
>> -    return true;
>> -  };
>> -
>> -  // Walk the exit blocks directly rather than building up a data
>> structure for
>> -  // them, but only visit each one once.
>> -  SmallPtrSet<BasicBlock *, 4> Visited;
>> -  for (auto *BB : L->blocks())
>> -    for (auto *SuccBB : successors(BB)) {
>> -      // We're looking for exit blocks so skip in-loop successors.
>> -      if (L->contains(SuccBB))
>> -        continue;
>> -
>> -      // Visit each exit block exactly once.
>> -      if (!Visited.insert(SuccBB).second)
>> -        continue;
>> -
>> -      Changed |= RewriteExit(SuccBB);
>> -    }
>> -
>> -  return Changed;
>> -}
>> -
>>  /// \brief Returns the instructions that use values defined in the loop.
>>  SmallVector<Instruction *, 8> llvm::findDefsUsedOutsideOfLoop(Loop *L) {
>>    SmallVector<Instruction *, 8> UsedOutside;
>>
>> Modified: llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-
>> SimpleSwitch.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>> Transforms/LoopUnswitch/2011-11-18-SimpleSwitch.ll?rev=
>> 306252&r1=306251&r2=306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-SimpleSwitch.ll
>> (original)
>> +++ llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-SimpleSwitch.ll
>> Sun Jun 25 10:58:25 2017
>> @@ -2,6 +2,7 @@
>>  ; RUN: opt -loop-unswitch -disable-output -stats -info-output-file - <
>> %s | FileCheck --check-prefix=STATS %s
>>  ; RUN: opt -S -loop-unswitch -verify-loop-info -verify-dom-info < %s |
>> FileCheck %s
>>
>> +; STATS: 1 loop-simplify - Number of pre-header or exit blocks inserted
>>  ; STATS: 2 loop-unswitch - Number of switches unswitched
>>
>>  ; CHECK:      %1 = icmp eq i32 %c, 1
>>
>> Modified: llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-
>> TwoSwitches-Threshold.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>> Transforms/LoopUnswitch/2011-11-18-TwoSwitches-Threshold.
>> ll?rev=306252&r1=306251&r2=306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-TwoSwitches-Threshold.ll
>> (original)
>> +++ llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-TwoSwitches-Threshold.ll
>> Sun Jun 25 10:58:25 2017
>> @@ -2,6 +2,7 @@
>>  ; RUN: opt -loop-unswitch -loop-unswitch-threshold 13 -disable-output
>> -stats -info-output-file - < %s | FileCheck --check-prefix=STATS %s
>>  ; RUN: opt -S -loop-unswitch -loop-unswitch-threshold 13
>> -verify-loop-info -verify-dom-info < %s | FileCheck %s
>>
>> +; STATS: 1 loop-simplify - Number of pre-header or exit blocks inserted
>>  ; STATS: 1 loop-unswitch - Number of switches unswitched
>>
>>  ; ModuleID = '../llvm/test/Transforms/LoopUnswitch/2011-11-18-
>> TwoSwitches.ll'
>>
>> Modified: llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-
>> TwoSwitches.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>> Transforms/LoopUnswitch/2011-11-18-TwoSwitches.ll?rev=
>> 306252&r1=306251&r2=306252&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-TwoSwitches.ll
>> (original)
>> +++ llvm/trunk/test/Transforms/LoopUnswitch/2011-11-18-TwoSwitches.ll
>> Sun Jun 25 10:58:25 2017
>> @@ -2,6 +2,7 @@
>>  ; RUN: opt -loop-unswitch -loop-unswitch-threshold 1000 -disable-output
>> -stats -info-output-file - < %s | FileCheck --check-prefix=STATS %s
>>  ; RUN: opt -S -loop-unswitch -loop-unswitch-threshold 1000
>> -verify-loop-info -verify-dom-info < %s | FileCheck %s
>>
>> +; STATS: 1 loop-simplify - Number of pre-header or exit blocks inserted
>>  ; STATS: 3 loop-unswitch - Number of switches unswitched
>>
>>  ; CHECK:        %1 = icmp eq i32 %c, 1
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170626/b300381b/attachment.html>


More information about the llvm-commits mailing list