[llvm-commits] [llvm] r52688 - in /llvm/trunk: lib/Transforms/Utils/UnrollLoop.cpp test/Transforms/LoopUnroll/multiple-phis.ll test/Transforms/LoopUnroll/pr2253.ll
Evan Cheng
evan.cheng at apple.com
Tue Jun 24 18:24:25 PDT 2008
This causes these annoying dejagnu failures since the files are now
empty. Perhaps we should just xfail them for now?
FAIL: /Users/echeng/LLVM/llvm/test/Transforms/LoopUnroll/multiple-
phis.ll:
Does not have a RUN line
FAIL: /Users/echeng/LLVM/llvm/test/Transforms/LoopUnroll/pr2253.ll:
Does not have a RUN line
Evan
On Jun 24, 2008, at 1:44 PM, Dan Gohman wrote:
> Author: djg
> Date: Tue Jun 24 15:44:42 2008
> New Revision: 52688
>
> URL: http://llvm.org/viewvc/llvm-project?rev=52688&view=rev
> Log:
> Revert 52645, the loop unroller changes. It caused a regression in
> 252.eon.
>
> Removed:
> llvm/trunk/test/Transforms/LoopUnroll/multiple-phis.ll
> llvm/trunk/test/Transforms/LoopUnroll/pr2253.ll
> Modified:
> llvm/trunk/lib/Transforms/Utils/UnrollLoop.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/UnrollLoop.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/UnrollLoop.cpp?rev=52688&r1=52687&r2=52688&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/lib/Transforms/Utils/UnrollLoop.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/UnrollLoop.cpp Tue Jun 24
> 15:44:42 2008
> @@ -22,7 +22,6 @@
> #include "llvm/Transforms/Utils/UnrollLoop.h"
> #include "llvm/BasicBlock.h"
> #include "llvm/ADT/Statistic.h"
> -#include "llvm/ADT/STLExtras.h"
> #include "llvm/Analysis/ConstantFolding.h"
> #include "llvm/Analysis/LoopPass.h"
> #include "llvm/Support/Debug.h"
> @@ -107,17 +106,13 @@
> ///
> /// If a LoopPassManager is passed in, and the loop is fully
> removed, it will be
> /// removed from the LoopPassManager as well. LPM can also be NULL.
> -bool llvm::UnrollLoop(Loop *L, unsigned Count, LoopInfo* LI,
> - LPPassManager* LPM) {
> +bool llvm::UnrollLoop(Loop *L, unsigned Count, LoopInfo* LI,
> LPPassManager* LPM) {
> assert(L->isLCSSAForm());
>
> BasicBlock *Header = L->getHeader();
> BasicBlock *LatchBlock = L->getLoopLatch();
> BranchInst *BI = dyn_cast<BranchInst>(LatchBlock->getTerminator());
> -
> - Function *Func = Header->getParent();
> - Function::iterator BBInsertPt =
> next(Function::iterator(LatchBlock));
> -
> +
> if (!BI || BI->isUnconditional()) {
> // The loop-rotate pass can be helpful to avoid this in many
> cases.
> DOUT << " Can't unroll; loop not terminated by a conditional
> branch.\n";
> @@ -173,148 +168,162 @@
> DOUT << "!\n";
> }
>
> - // Make a copy of the original LoopBlocks list so we can keep
> referring
> - // to it while hacking on the loop.
> std::vector<BasicBlock*> LoopBlocks = L->getBlocks();
>
> - bool ContinueOnTrue = BI->getSuccessor(0) == Header;
> + bool ContinueOnTrue = L->contains(BI->getSuccessor(0));
> BasicBlock *LoopExit = BI->getSuccessor(ContinueOnTrue);
>
> // For the first iteration of the loop, we should use the
> precloned values for
> // PHI nodes. Insert associations now.
> typedef DenseMap<const Value*, Value*> ValueMapTy;
> ValueMapTy LastValueMap;
> + std::vector<PHINode*> OrigPHINode;
> for (BasicBlock::iterator I = Header->begin(); isa<PHINode>(I); +
> +I) {
> PHINode *PN = cast<PHINode>(I);
> + OrigPHINode.push_back(PN);
> if (Instruction *I =
> dyn_cast<Instruction>(PN-
> >getIncomingValueForBlock(LatchBlock)))
> if (L->contains(I->getParent()))
> LastValueMap[I] = I;
> }
>
> - // Keep track of all the headers and latches that we create.
> These are
> - // needed by the logic that inserts the branches to connect all the
> - // new blocks.
> std::vector<BasicBlock*> Headers;
> std::vector<BasicBlock*> Latches;
> - Headers.reserve(Count);
> - Latches.reserve(Count);
> Headers.push_back(Header);
> Latches.push_back(LatchBlock);
>
> - // Iterate through all but the first iterations, cloning blocks
> from
> - // the first iteration to populate the subsequent iterations.
> for (unsigned It = 1; It != Count; ++It) {
> char SuffixBuffer[100];
> sprintf(SuffixBuffer, ".%d", It);
>
> std::vector<BasicBlock*> NewBlocks;
> - NewBlocks.reserve(LoopBlocks.size());
>
> - // Iterate through all the blocks in the original loop.
> - for (std::vector<BasicBlock*>::const_iterator BBI =
> LoopBlocks.begin(),
> - E = LoopBlocks.end(); BBI != E; ++BBI) {
> - bool SuppressExitEdges = false;
> - BasicBlock *BB = *BBI;
> + for (std::vector<BasicBlock*>::iterator BB = LoopBlocks.begin(),
> + E = LoopBlocks.end(); BB != E; ++BB) {
> ValueMapTy ValueMap;
> - BasicBlock *New = CloneBasicBlock(BB, ValueMap, SuffixBuffer);
> - NewBlocks.push_back(New);
> - Func->getBasicBlockList().insert(BBInsertPt, New);
> - L->addBasicBlockToLoop(New, LI->getBase());
> + BasicBlock *New = CloneBasicBlock(*BB, ValueMap, SuffixBuffer);
> + Header->getParent()->getBasicBlockList().push_back(New);
>
> - // Special handling for the loop header block.
> - if (BB == Header) {
> - // Keep track of new headers as we create them, so that we
> can insert
> - // the proper branches later.
> - Headers[It] = New;
> -
> - // Loop over all of the PHI nodes in the block, changing
> them to use
> - // the incoming values from the previous block.
> - for (BasicBlock::iterator I = Header->begin();
> isa<PHINode>(I); ++I) {
> - PHINode *NewPHI = cast<PHINode>(ValueMap[I]);
> + // Loop over all of the PHI nodes in the block, changing them
> to use the
> + // incoming values from the previous block.
> + if (*BB == Header)
> + for (unsigned i = 0, e = OrigPHINode.size(); i != e; ++i) {
> + PHINode *NewPHI = cast<PHINode>(ValueMap[OrigPHINode[i]]);
> Value *InVal = NewPHI->getIncomingValueForBlock(LatchBlock);
> if (Instruction *InValI = dyn_cast<Instruction>(InVal))
> if (It > 1 && L->contains(InValI->getParent()))
> InVal = LastValueMap[InValI];
> - ValueMap[I] = InVal;
> + ValueMap[OrigPHINode[i]] = InVal;
> New->getInstList().erase(NewPHI);
> }
> - }
> -
> - // Special handling for the loop latch block.
> - if (BB == LatchBlock) {
> - // Keep track of new latches as we create them, so that we
> can insert
> - // the proper branches later.
> - Latches[It] = New;
> -
> - // If knowledge of the trip count and/or multiple will
> allow us
> - // to emit unconditional branches in some of the new latch
> blocks,
> - // those blocks shouldn't be referenced by PHIs that
> reference
> - // the original latch.
> - unsigned NextIt = (It + 1) % Count;
> - SuppressExitEdges =
> - NextIt != BreakoutTrip &&
> - (TripMultiple == 0 || NextIt % TripMultiple != 0);
> - }
>
> // Update our running map of newest clones
> - LastValueMap[BB] = New;
> + LastValueMap[*BB] = New;
> for (ValueMapTy::iterator VI = ValueMap.begin(), VE =
> ValueMap.end();
> VI != VE; ++VI)
> LastValueMap[VI->first] = VI->second;
>
> - // Add incoming values to phi nodes that reference this
> block. The last
> - // latch block may need to be referenced by the first header,
> and any
> - // block with an exit edge may be referenced from outside the
> loop.
> - for (Value::use_iterator UI = BB->use_begin(), UE = BB-
> >use_end();
> - UI != UE; ) {
> - PHINode *PN = dyn_cast<PHINode>(*UI++);
> - if (PN &&
> - ((BB == LatchBlock && It == Count - 1 && !
> CompletelyUnroll) ||
> - (!SuppressExitEdges && !L->contains(PN-
> >getParent())))) {
> - Value *InVal = PN->getIncomingValueForBlock(BB);
> - // If this value was defined in the loop, take the value
> defined
> - // by the last iteration of the loop.
> - ValueMapTy::iterator VI = LastValueMap.find(InVal);
> - if (VI != LastValueMap.end())
> - InVal = VI->second;
> - PN->addIncoming(InVal, New);
> + L->addBasicBlockToLoop(New, LI->getBase());
> +
> + // Add phi entries for newly created values to all exit
> blocks except
> + // the successor of the latch block. The successor of the
> exit block will
> + // be updated specially after unrolling all the way.
> + if (*BB != LatchBlock)
> + for (Value::use_iterator UI = (*BB)->use_begin(), UE =
> (*BB)->use_end();
> + UI != UE;) {
> + Instruction *UseInst = cast<Instruction>(*UI);
> + ++UI;
> + if (isa<PHINode>(UseInst) && !L->contains(UseInst-
> >getParent())) {
> + PHINode *phi = cast<PHINode>(UseInst);
> + Value *Incoming = phi->getIncomingValueForBlock(*BB);
> + phi->addIncoming(Incoming, New);
> + }
> }
> +
> + // Keep track of new headers and latches as we create them,
> so that
> + // we can insert the proper branches later.
> + if (*BB == Header)
> + Headers.push_back(New);
> + if (*BB == LatchBlock) {
> + Latches.push_back(New);
> +
> + // Also, clear out the new latch's back edge so that it
> doesn't look
> + // like a new loop, so that it's amenable to being merged
> with adjacent
> + // blocks later on.
> + TerminatorInst *Term = New->getTerminator();
> + assert(L->contains(Term->getSuccessor(!ContinueOnTrue)));
> + assert(Term->getSuccessor(ContinueOnTrue) == LoopExit);
> + Term->setSuccessor(!ContinueOnTrue, NULL);
> }
> +
> + NewBlocks.push_back(New);
> }
>
> // Remap all instructions in the most recent iteration
> - for (unsigned i = 0, e = NewBlocks.size(); i != e; ++i)
> + for (unsigned i = 0; i < NewBlocks.size(); ++i)
> for (BasicBlock::iterator I = NewBlocks[i]->begin(),
> E = NewBlocks[i]->end(); I != E; ++I)
> RemapInstruction(I, LastValueMap);
> }
> +
> + // The latch block exits the loop. If there are any PHI nodes in
> the
> + // successor blocks, update them to use the appropriate values
> computed as the
> + // last iteration of the loop.
> + if (Count != 1) {
> + SmallPtrSet<PHINode*, 8> Users;
> + for (Value::use_iterator UI = LatchBlock->use_begin(),
> + UE = LatchBlock->use_end(); UI != UE; ++UI)
> + if (PHINode *phi = dyn_cast<PHINode>(*UI))
> + Users.insert(phi);
> +
> + BasicBlock *LastIterationBB =
> cast<BasicBlock>(LastValueMap[LatchBlock]);
> + for (SmallPtrSet<PHINode*,8>::iterator SI = Users.begin(), SE =
> Users.end();
> + SI != SE; ++SI) {
> + PHINode *PN = *SI;
> + Value *InVal = PN->removeIncomingValue(LatchBlock, false);
> + // If this value was defined in the loop, take the value
> defined by the
> + // last iteration of the loop.
> + if (Instruction *InValI = dyn_cast<Instruction>(InVal)) {
> + if (L->contains(InValI->getParent()))
> + InVal = LastValueMap[InVal];
> + }
> + PN->addIncoming(InVal, LastIterationBB);
> + }
> + }
> +
> + // Now, if we're doing complete unrolling, loop over the PHI
> nodes in the
> + // original block, setting them to their incoming values.
> + if (CompletelyUnroll) {
> + BasicBlock *Preheader = L->getLoopPreheader();
> + for (unsigned i = 0, e = OrigPHINode.size(); i != e; ++i) {
> + PHINode *PN = OrigPHINode[i];
> + PN->replaceAllUsesWith(PN-
> >getIncomingValueForBlock(Preheader));
> + Header->getInstList().erase(PN);
> + }
> + }
>
> // Now that all the basic blocks for the unrolled iterations are
> in place,
> // set up the branches to connect them.
> - for (unsigned It = 0; It != Count; ++It) {
> + for (unsigned i = 0, e = Latches.size(); i != e; ++i) {
> // The original branch was replicated in each unrolled iteration.
> - BranchInst *Term = cast<BranchInst>(Latches[It]-
> >getTerminator());
> + BranchInst *Term = cast<BranchInst>(Latches[i]->getTerminator());
>
> // The branch destination.
> - unsigned NextIt = (It + 1) % Count;
> - BasicBlock *Dest = Headers[NextIt];
> + unsigned j = (i + 1) % e;
> + BasicBlock *Dest = Headers[j];
> bool NeedConditional = true;
> - bool HasExit = true;
>
> - // For a complete unroll, make the last iteration end with an
> - // unconditional branch to the exit block.
> - if (CompletelyUnroll && NextIt == 0) {
> + // For a complete unroll, make the last iteration end with a
> branch
> + // to the exit block.
> + if (CompletelyUnroll && j == 0) {
> Dest = LoopExit;
> NeedConditional = false;
> }
>
> // If we know the trip count or a multiple of it, we can safely
> use an
> // unconditional branch for some iterations.
> - if (NextIt != BreakoutTrip &&
> - (TripMultiple == 0 || NextIt % TripMultiple != 0)) {
> + if (j != BreakoutTrip && (TripMultiple == 0 || j %
> TripMultiple != 0)) {
> NeedConditional = false;
> - HasExit = false;
> }
>
> if (NeedConditional) {
> @@ -329,50 +338,24 @@
> std::replace(Headers.begin(), Headers.end(), Dest, Fold);
> }
> }
> -
> - // Special handling for the first iteration. If the first latch
> is
> - // now unconditionally branching to the second header, then it is
> - // no longer an exit node. Delete PHI references to it both from
> - // the first header and from outsie the loop.
> - if (It == 0)
> - for (Value::use_iterator UI = LatchBlock->use_begin(),
> - UE = LatchBlock->use_end(); UI != UE; ) {
> - PHINode *PN = dyn_cast<PHINode>(*UI++);
> - if (PN && (PN->getParent() == Header ? Count > 1 : !HasExit))
> - PN->removeIncomingValue(LatchBlock);
> - }
> }
>
> - // At this point, unrolling is complete and the code is well
> formed.
> - // Now, do some simplifications.
> -
> - // If we're doing complete unrolling, loop over the PHI nodes in
> the
> - // original block, setting them to their incoming values.
> - if (CompletelyUnroll) {
> - BasicBlock *Preheader = L->getLoopPreheader();
> - for (BasicBlock::iterator I = Header->begin();
> isa<PHINode>(I); ) {
> - PHINode *PN = cast<PHINode>(I++);
> - PN->replaceAllUsesWith(PN-
> >getIncomingValueForBlock(Preheader));
> - Header->getInstList().erase(PN);
> - }
> - }
> -
> - // We now do a quick sweep over the inserted code, doing constant
> - // propagation and dead code elimination as we go.
> - for (Loop::block_iterator BI = L->block_begin(), BBE = L-
> >block_end();
> - BI != BBE; ++BI) {
> - BasicBlock *BB = *BI;
> - for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I !=
> E; ) {
> + // At this point, the code is well formed. We now do a quick
> sweep over the
> + // inserted code, doing constant propagation and dead code
> elimination as we
> + // go.
> + const std::vector<BasicBlock*> &NewLoopBlocks = L->getBlocks();
> + for (std::vector<BasicBlock*>::const_iterator BB =
> NewLoopBlocks.begin(),
> + BBE = NewLoopBlocks.end(); BB != BBE; ++BB)
> + for (BasicBlock::iterator I = (*BB)->begin(), E = (*BB)->end();
> I != E; ) {
> Instruction *Inst = I++;
>
> if (isInstructionTriviallyDead(Inst))
> - BB->getInstList().erase(Inst);
> + (*BB)->getInstList().erase(Inst);
> else if (Constant *C = ConstantFoldInstruction(Inst)) {
> Inst->replaceAllUsesWith(C);
> - BB->getInstList().erase(Inst);
> + (*BB)->getInstList().erase(Inst);
> }
> }
> - }
>
> NumCompletelyUnrolled += CompletelyUnroll;
> ++NumUnrolled;
>
> Removed: llvm/trunk/test/Transforms/LoopUnroll/multiple-phis.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopUnroll/multiple-phis.ll?rev=52687&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/Transforms/LoopUnroll/multiple-phis.ll (original)
> +++ llvm/trunk/test/Transforms/LoopUnroll/multiple-phis.ll (removed)
> @@ -1,51 +0,0 @@
> -; RUN: llvm-as < %s | opt -loop-unroll -unroll-count 6 -unroll-
> threshold 300 | llvm-dis > %t
> -; RUN: grep {br label \%bbe} %t | count 12
> -; RUN: grep {br i1 \%z} %t | count 3
> -; RUN: grep {br i1 \%q} %t | count 6
> -; RUN: grep call %t | count 12
> -; RUN: grep urem %t | count 6
> -; RUN: grep store %t | count 6
> -; RUN: grep phi %t | count 11
> -; RUN: grep {lcssa = phi} %t | count 2
> -
> -; This testcase uses
> -; - an unknown tripcount, but a known trip multiple of 2.
> -; - an unroll count of 6, so we should get 3 conditional branches
> -; in the loop.
> -; - values defined inside the loop and used outside, by phis that
> -; also use values defined elsewhere outside the loop.
> -; - a phi inside the loop that only uses values defined
> -; inside the loop and is only used inside the loop.
> -
> -declare i32 @foo()
> -declare i32 @bar()
> -
> -define i32 @fib(i32 %n, i1 %a, i32* %p) nounwind {
> -entry:
> - %n2 = mul i32 %n, 2
> - br i1 %a, label %bb, label %return
> -
> -bb: ; loop header block
> - %t0 = phi i32 [ 0, %entry ], [ %t1, %bbe ]
> - %td = urem i32 %t0, 7
> - %q = trunc i32 %td to i1
> - br i1 %q, label %bbt, label %bbf
> -bbt:
> - %bbtv = call i32 @foo()
> - br label %bbe
> -bbf:
> - %bbfv = call i32 @bar()
> - br label %bbe
> -bbe: ; loop latch block
> - %bbpv = phi i32 [ %bbtv, %bbt ], [ %bbfv, %bbf ]
> - store i32 %bbpv, i32* %p
> - %t1 = add i32 %t0, 1
> - %z = icmp ne i32 %t1, %n2
> - br i1 %z, label %bb, label %return
> -
> -return:
> - %f = phi i32 [ -2, %entry ], [ %t0, %bbe ]
> - %g = phi i32 [ -3, %entry ], [ %t1, %bbe ]
> - %h = mul i32 %f, %g
> - ret i32 %h
> -}
>
> Removed: llvm/trunk/test/Transforms/LoopUnroll/pr2253.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopUnroll/pr2253.ll?rev=52687&view=auto
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/test/Transforms/LoopUnroll/pr2253.ll (original)
> +++ llvm/trunk/test/Transforms/LoopUnroll/pr2253.ll (removed)
> @@ -1,21 +0,0 @@
> -; RUN: llvm-as < %s | opt -loop-unroll -unroll-count 2 | llvm-dis |
> grep add | count 2
> -; PR2253
> -
> -; There's a use outside the loop, and the PHI needs an incoming
> edge for
> -; each unrolled iteration, since the trip count is unknown and any
> iteration
> -; could exit.
> -
> -define i32 @fib(i32 %n) nounwind {
> -entry:
> - br i1 false, label %bb, label %return
> -
> -bb:
> - %t0 = phi i32 [ 0, %entry ], [ %t1, %bb ]
> - %t1 = add i32 %t0, 1
> - %c = icmp ne i32 %t0, %n
> - br i1 %c, label %bb, label %return
> -
> -return:
> - %f2.0.lcssa = phi i32 [ -1, %entry ], [ %t0, %bb ]
> - ret i32 %f2.0.lcssa
> -}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list