From: Dan Gohman Date: Sat, 2 May 2009 18:29:22 +0000 (+0000) Subject: Previously, RecursivelyDeleteDeadInstructions provided an option X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=afc36a9520971832dfbebc0333593bf5d3098296;p=oota-llvm.git Previously, RecursivelyDeleteDeadInstructions provided an option of returning a list of pointers to Values that are deleted. This was unsafe, because the pointers in the list are, by nature of what RecursivelyDeleteDeadInstructions does, always dangling. Replace this with a simple callback mechanism. This may eventually be removed if all clients can reasonably be expected to use CallbackVH. Use this to factor out the dead-phi-cycle-elimination code from LSR utility function, and generalize it to use the RecursivelyDeleteTriviallyDeadInstructions utility function. This makes LSR more aggressive about eliminating dead PHI cycles; adjust tests to either be less trivial or to simply expect fewer instructions. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@70636 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Transforms/Utils/BasicBlockUtils.h b/include/llvm/Transforms/Utils/BasicBlockUtils.h index a629b119bc3..6105416a407 100644 --- a/include/llvm/Transforms/Utils/BasicBlockUtils.h +++ b/include/llvm/Transforms/Utils/BasicBlockUtils.h @@ -25,6 +25,7 @@ namespace llvm { class Instruction; class Pass; class AliasAnalysis; +class ValueDeletionListener; /// DeleteDeadBlock - Delete the specified block, which must have no /// predecessors. @@ -36,8 +37,14 @@ void DeleteDeadBlock(BasicBlock *BB); /// when all entries to the PHI nodes in a block are guaranteed equal, such as /// when the block has exactly one predecessor. void FoldSingleEntryPHINodes(BasicBlock *BB); - - + +/// DeleteDeadPHIs - Examine each PHI in the given block and delete it if it +/// is dead. Also recursively delete any operands that become dead as +/// a result. This includes tracing the def-use list from the PHI to see if +/// it is ultimately unused or if it reaches an unused cycle. If a +/// ValueDeletionListener is specified, it is notified of the deletions. +void DeleteDeadPHIs(BasicBlock *BB, ValueDeletionListener *VDL = 0); + /// MergeBlockIntoPredecessor - Attempts to merge a block into its predecessor, /// if possible. The return value indicates success or failure. bool MergeBlockIntoPredecessor(BasicBlock* BB, Pass* P = 0); diff --git a/include/llvm/Transforms/Utils/Local.h b/include/llvm/Transforms/Utils/Local.h index c3088a2c7c8..4cb46b0c4b6 100644 --- a/include/llvm/Transforms/Utils/Local.h +++ b/include/llvm/Transforms/Utils/Local.h @@ -50,16 +50,41 @@ bool ConstantFoldTerminator(BasicBlock *BB); /// bool isInstructionTriviallyDead(Instruction *I); - +/// ValueDeletionListener - A simple abstract interface for delivering +/// notifications when Values are deleted. +/// +/// @todo Consider whether ValueDeletionListener can be made obsolete by +/// requiring clients to use CallbackVH instead. +class ValueDeletionListener { +public: + /// ValueWillBeDeleted - This method is called shortly before the specified + /// value will be deleted. + virtual void ValueWillBeDeleted(Value *V) = 0; + +protected: + virtual ~ValueDeletionListener(); +}; + /// RecursivelyDeleteTriviallyDeadInstructions - If the specified value is a /// trivially dead instruction, delete it. If that makes any of its operands /// trivially dead, delete them too, recursively. /// -/// If DeadInst is specified, the vector is filled with the instructions that -/// are actually deleted. +/// If a ValueDeletionListener is specified, it is notified of instructions that +/// are actually deleted (before they are actually deleted). void RecursivelyDeleteTriviallyDeadInstructions(Value *V, - SmallVectorImpl *DeadInst = 0); - + ValueDeletionListener *VDL = 0); + +/// RecursivelyDeleteDeadPHINode - If the specified value is an effectively +/// dead PHI node, due to being a def-use chain of single-use nodes that +/// either forms a cycle or is terminated by a trivially dead instruction, +/// delete it. If that makes any of its operands trivially dead, delete them +/// too, recursively. +/// +/// If a ValueDeletionListener is specified, it is notified of instructions that +/// are actually deleted (before they are actually deleted). +void RecursivelyDeleteDeadPHINode(PHINode *PN, + ValueDeletionListener *VDL = 0); + //===----------------------------------------------------------------------===// // Control Flow Graph Restructuring. // diff --git a/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/lib/Transforms/Scalar/LoopStrengthReduce.cpp index e5502002713..b9406651525 100644 --- a/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -32,6 +32,7 @@ #include "llvm/Support/Debug.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/ValueHandle.h" #include "llvm/Target/TargetLowering.h" #include using namespace llvm; @@ -2138,6 +2139,7 @@ ICmpInst *LoopStrengthReduce::ChangeCompareStride(Loop *L, ICmpInst *Cond, CondUse = &IVUsesByStride[*NewStride].Users.back(); CondStride = NewStride; ++NumEliminated; + Changed = true; } return Cond; @@ -2501,44 +2503,21 @@ bool LoopStrengthReduce::runOnLoop(Loop *L, LPPassManager &LPM) { StrideOrder.clear(); // Clean up after ourselves - if (!DeadInsts.empty()) { + if (!DeadInsts.empty()) DeleteTriviallyDeadInstructions(); - BasicBlock::iterator I = L->getHeader()->begin(); - while (PHINode *PN = dyn_cast(I++)) { - // At this point, we know that we have killed one or more IV users. - // It is worth checking to see if the cannonical indvar is also - // dead, so that we can remove it as well. - // - // We can remove a PHI if it is on a cycle in the def-use graph - // where each node in the cycle has degree one, i.e. only one use, - // and is an instruction with no side effects. - // - // FIXME: this needs to eliminate an induction variable even if it's being - // compared against some value to decide loop termination. - if (!PN->hasOneUse()) - continue; - - SmallPtrSet PHIs; - for (Instruction *J = dyn_cast(*PN->use_begin()); - J && J->hasOneUse() && !J->mayWriteToMemory(); - J = dyn_cast(*J->use_begin())) { - // If we find the original PHI, we've discovered a cycle. - if (J == PN) { - // Break the cycle and mark the PHI for deletion. - SE->deleteValueFromRecords(PN); - PN->replaceAllUsesWith(UndefValue::get(PN->getType())); - DeadInsts.push_back(PN); - Changed = true; - break; - } - // If we find a PHI more than once, we're on a cycle that - // won't prove fruitful. - if (isa(J) && !PHIs.insert(cast(J))) - break; - } + // At this point, it is worth checking to see if any recurrence PHIs are also + // dead, so that we can remove them as well. To keep ScalarEvolution + // current, use a ValueDeletionListener class. + struct LSRListener : public ValueDeletionListener { + ScalarEvolution &SE; + explicit LSRListener(ScalarEvolution &se) : SE(se) {} + + virtual void ValueWillBeDeleted(Value *V) { + SE.deleteValueFromRecords(V); } - DeleteTriviallyDeadInstructions(); - } + } VDL(*SE); + DeleteDeadPHIs(L->getHeader(), &VDL); + return Changed; } diff --git a/lib/Transforms/Utils/BasicBlockUtils.cpp b/lib/Transforms/Utils/BasicBlockUtils.cpp index 97460bf1add..076f89e3374 100644 --- a/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -22,6 +22,8 @@ #include "llvm/Analysis/LoopInfo.h" #include "llvm/Analysis/Dominators.h" #include "llvm/Target/TargetData.h" +#include "llvm/Transforms/Utils/Local.h" +#include "llvm/Support/ValueHandle.h" #include using namespace llvm; @@ -73,6 +75,24 @@ void llvm::FoldSingleEntryPHINodes(BasicBlock *BB) { } +/// DeleteDeadPHIs - Examine each PHI in the given block and delete it if it +/// is dead. Also recursively delete any operands that become dead as +/// a result. This includes tracing the def-use list from the PHI to see if +/// it is ultimately unused or if it reaches an unused cycle. If a +/// ValueDeletionListener is specified, it is notified of the deletions. +void llvm::DeleteDeadPHIs(BasicBlock *BB, ValueDeletionListener *VDL) { + // Recursively deleting a PHI may cause multiple PHIs to be deleted + // or RAUW'd undef, so use an array of WeakVH for the PHIs to delete. + SmallVector PHIs; + for (BasicBlock::iterator I = BB->begin(); + PHINode *PN = dyn_cast(I); ++I) + PHIs.push_back(PN); + + for (unsigned i = 0, e = PHIs.size(); i != e; ++i) + if (PHINode *PN = dyn_cast_or_null(PHIs[i].operator Value*())) + RecursivelyDeleteDeadPHINode(PN, VDL); +} + /// MergeBlockIntoPredecessor - Attempts to merge a block into its predecessor, /// if possible. The return value indicates success or failure. bool llvm::MergeBlockIntoPredecessor(BasicBlock* BB, Pass* P) { diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index 4be1b8717d2..3b36362bb39 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -19,6 +19,7 @@ #include "llvm/Instructions.h" #include "llvm/Intrinsics.h" #include "llvm/IntrinsicInst.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/Analysis/ConstantFolding.h" #include "llvm/Analysis/DebugInfo.h" #include "llvm/Target/TargetData.h" @@ -177,14 +178,18 @@ bool llvm::isInstructionTriviallyDead(Instruction *I) { return false; } +/// ~ValueDeletionListener - A trivial dtor, defined out of line to give the +/// class a home. +llvm::ValueDeletionListener::~ValueDeletionListener() {} + /// RecursivelyDeleteTriviallyDeadInstructions - If the specified value is a /// trivially dead instruction, delete it. If that makes any of its operands /// trivially dead, delete them too, recursively. /// -/// If DeadInst is specified, the vector is filled with the instructions that -/// are actually deleted. +/// If a ValueDeletionListener is specified, it is notified of instructions that +/// are actually deleted (before they are actually deleted). void llvm::RecursivelyDeleteTriviallyDeadInstructions(Value *V, - SmallVectorImpl *DeadInst) { + ValueDeletionListener *VDL) { Instruction *I = dyn_cast(V); if (!I || !I->use_empty() || !isInstructionTriviallyDead(I)) return; @@ -197,8 +202,8 @@ void llvm::RecursivelyDeleteTriviallyDeadInstructions(Value *V, DeadInsts.pop_back(); // If the client wanted to know, tell it about deleted instructions. - if (DeadInst) - DeadInst->push_back(I); + if (VDL) + VDL->ValueWillBeDeleted(I); // Null out all of the instruction's operands to see if any operand becomes // dead as we go. @@ -220,6 +225,38 @@ void llvm::RecursivelyDeleteTriviallyDeadInstructions(Value *V, } } +/// RecursivelyDeleteDeadPHINode - If the specified value is an effectively +/// dead PHI node, due to being a def-use chain of single-use nodes that +/// either forms a cycle or is terminated by a trivially dead instruction, +/// delete it. If that makes any of its operands trivially dead, delete them +/// too, recursively. +/// +/// If a ValueDeletionListener is specified, it is notified of instructions that +/// are actually deleted (before they are actually deleted). +void +llvm::RecursivelyDeleteDeadPHINode(PHINode *PN, ValueDeletionListener *VDL) { + + // We can remove a PHI if it is on a cycle in the def-use graph + // where each node in the cycle has degree one, i.e. only one use, + // and is an instruction with no side effects. + if (!PN->hasOneUse()) + return; + + SmallPtrSet PHIs; + PHIs.insert(PN); + for (Instruction *J = cast(*PN->use_begin()); + J->hasOneUse() && !J->mayWriteToMemory(); + J = cast(*J->use_begin())) + // If we find a PHI more than once, we're on a cycle that + // won't prove fruitful. + if (PHINode *JP = dyn_cast(J)) + if (!PHIs.insert(cast(JP))) { + // Break the cycle and delete the PHI and its operands. + JP->replaceAllUsesWith(UndefValue::get(JP->getType())); + RecursivelyDeleteTriviallyDeadInstructions(JP, VDL); + break; + } +} //===----------------------------------------------------------------------===// // Control Flow Graph Restructuring... diff --git a/test/CodeGen/ARM/2008-11-19-ScavengerAssert.ll b/test/CodeGen/ARM/2008-11-19-ScavengerAssert.ll index 0a73b3fc257..7b7ea6bcc49 100644 --- a/test/CodeGen/ARM/2008-11-19-ScavengerAssert.ll +++ b/test/CodeGen/ARM/2008-11-19-ScavengerAssert.ll @@ -1,5 +1,4 @@ -; RUN: llvm-as < %s | llc -mtriple=arm-apple-darwin9 -; RUN: llvm-as < %s | llc -mtriple=arm-apple-darwin9 -stats |& grep asm-printer | grep 186 +; RUN: llvm-as < %s | llc -mtriple=arm-apple-darwin9 -stats |& grep asm-printer | grep 184 %"struct.Adv5::Ekin<3>" = type <{ i8 }> %"struct.Adv5::X::Energyflux<3>" = type { double } diff --git a/test/CodeGen/X86/mmx-vzmovl-2.ll b/test/CodeGen/X86/mmx-vzmovl-2.ll index f0b5cc3d808..4dd1e47394f 100644 --- a/test/CodeGen/X86/mmx-vzmovl-2.ll +++ b/test/CodeGen/X86/mmx-vzmovl-2.ll @@ -17,6 +17,7 @@ bb554: ; preds = %bb554, %entry %tmp555 = and <2 x i32> %1, < i32 -1, i32 0 > ; <<2 x i32>> [#uses=1] %2 = bitcast <2 x i32> %tmp555 to <1 x i64> ; <<1 x i64>> [#uses=1] %3 = call <1 x i64> @llvm.x86.mmx.psrli.q(<1 x i64> %0, i32 32) nounwind readnone ; <<1 x i64>> [#uses=1] + store <1 x i64> %sum.0.reg2mem.0, <1 x i64>* null %tmp558 = add <1 x i64> %sum.0.reg2mem.0, %2 ; <<1 x i64>> [#uses=1] %4 = call <1 x i64> @llvm.x86.mmx.psrli.q(<1 x i64> %tmp558, i32 32) nounwind readnone ; <<1 x i64>> [#uses=1] %tmp562 = add <1 x i64> %4, %3 ; <<1 x i64>> [#uses=1] diff --git a/test/CodeGen/X86/pre-split4.ll b/test/CodeGen/X86/pre-split4.ll index 013402dd101..97401b3e7d5 100644 --- a/test/CodeGen/X86/pre-split4.ll +++ b/test/CodeGen/X86/pre-split4.ll @@ -14,6 +14,8 @@ bb: ; preds = %bb, %entry %2 = tail call double @sin(double %k.0.reg2mem.0) nounwind readonly ; [#uses=1] %3 = mul double 0.000000e+00, %2 ; [#uses=1] %4 = fdiv double 1.000000e+00, %3 ; [#uses=1] + store double %Flint.0.reg2mem.0, double* null + store double %twoThrd.0.reg2mem.0, double* null %5 = add double %4, %Flint.0.reg2mem.0 ; [#uses=1] %6 = add double %k.0.reg2mem.0, 1.000000e+00 ; [#uses=1] br label %bb diff --git a/test/Transforms/LoopStrengthReduce/pr2570.ll b/test/Transforms/LoopStrengthReduce/pr2570.ll index 23ae7238b0b..ce0c3bf5c98 100644 --- a/test/Transforms/LoopStrengthReduce/pr2570.ll +++ b/test/Transforms/LoopStrengthReduce/pr2570.ll @@ -1,4 +1,4 @@ -; RUN: llvm-as < %s | opt -loop-reduce | llvm-dis | grep {phi\\>} | count 14 +; RUN: llvm-as < %s | opt -loop-reduce | llvm-dis | grep {phi\\>} | count 10 ; PR2570 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32"