[llvm] r306252 - Revert "[LoopSimplify] Factor the logic to form dedicated exits into a utility."
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 25 15:33:23 PDT 2017
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/20170625/b0e1eac0/attachment.html>
More information about the llvm-commits
mailing list