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