[llvm-commits] [llvm] r148132 - /llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
Duncan Sands
baldrick at free.fr
Sat Jan 14 06:58:31 PST 2012
Hi Eli,
On 13/01/12 23:40, Eli Friedman wrote:
> On Fri, Jan 13, 2012 at 11:13 AM, Stepan Dyatkovskiy<stpworld at narod.ru> wrote:
>> Author: dyatkovskiy
>> Date: Fri Jan 13 13:13:54 2012
>> New Revision: 148132
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=148132&view=rev
>> Log:
>> LoopUnswitch: All helper data that is collected during loop-unswitch iterations was moved to separated class (LUAnalysisCache).
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
>
> I reverted this in r148149 due to a buildbot failure which showed up
> as a crash during loop unswitching in a bootstrap build. I'm not
> entirely sure what is going on (or whether it's actually the fault of
> this commit) because I can't reproduce the issue on my machine; I'll
> see what additional information I can gather.
I also saw this on a local buildbot, but couldn't reproduce it outside
of the buildbot environment. Valgrind didn't show anything.
Ciao, Duncan.
>
> -Eli
>
>> Modified: llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp?rev=148132&r1=148131&r2=148132&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp Fri Jan 13 13:13:54 2012
>> @@ -65,6 +65,58 @@
>> cl::init(100), cl::Hidden);
>>
>> namespace {
>> +
>> + class LUAnalysisCache {
>> +
>> + typedef DenseMap<const SwitchInst*, SmallPtrSet<const Value *,8> >
>> + UnswitchedValsMap;
>> +
>> + typedef UnswitchedValsMap::iterator UnswitchedValsIt;
>> +
>> + struct LoopProperties {
>> + unsigned CanBeUnswitchedCount;
>> + unsigned SizeEstimation;
>> + UnswitchedValsMap UnswitchedVals;
>> + };
>> +
>> + typedef DenseMap<const Loop*, LoopProperties> LoopPropsMap;
>> + typedef LoopPropsMap::iterator LoopPropsMapIt;
>> +
>> + LoopPropsMap LoopsProperties;
>> + UnswitchedValsMap* CurLoopInstructions;
>> + LoopProperties* CurrentLoopProperties;
>> +
>> + unsigned MaxSize;
>> +
>> + public:
>> +
>> + LUAnalysisCache() :
>> + CurLoopInstructions(NULL), CurrentLoopProperties(NULL),
>> + MaxSize(Threshold)
>> + {}
>> +
>> + // Analyze loop. Check its size, calculate is it possible to unswitch
>> + // it. Returns true if we can unswitch this loop.
>> + bool countLoop(const Loop* L);
>> +
>> + // Clean all data related to given loop.
>> + void forgetLoop(const Loop* L);
>> +
>> + // Mark case value as unswitched.
>> + // Since SI instruction can be partly unswitched, in order to avoid
>> + // extra unswitching in cloned loops keep track all unswitched values.
>> + void setUnswitched(const SwitchInst* SI, const Value* V);
>> +
>> + // Check was this case value unswitched before or not.
>> + bool isUnswitched(const SwitchInst* SI, const Value* V);
>> +
>> + // Clone all loop-unswitch related loop properties.
>> + // Redistribute unswitching quotas.
>> + // Note, that new loop data is stored inside the VMap.
>> + void cloneData(const Loop* NewLoop, const Loop* OldLoop,
>> + const ValueToValueMapTy& VMap);
>> + };
>> +
>> class LoopUnswitch : public LoopPass {
>> LoopInfo *LI; // Loop information
>> LPPassManager *LPM;
>> @@ -73,20 +125,20 @@
>> // after RewriteLoopBodyWithConditionConstant rewrites first loop.
>> std::vector<Loop*> LoopProcessWorklist;
>>
>> + // TODO: This few lines are here for cosmetic purposes only.
>> + // Will be removed with the next commit.
>> struct LoopProperties {
>> unsigned CanBeUnswitchedCount;
>> unsigned SizeEstimation;
>> };
>>
>> + // TODO: This few lines are here for cosmetic purposes only.
>> + // Will be removed with the next commit.
>> typedef DenseMap<const Loop*, LoopProperties> LoopPropsMap;
>> typedef LoopPropsMap::iterator LoopPropsMapIt;
>> LoopPropsMap LoopsProperties;
>>
>> - // Max size of code we can produce on remained iterations.
>> - unsigned MaxSize;
>> -
>> - // FIXME: Consider custom class for this.
>> - std::map<const SwitchInst*, SmallPtrSet<const Value *,8> > UnswitchedVals;
>> + LUAnalysisCache BranchesInfo;
>>
>> bool OptimizeForSize;
>> bool redoLoop;
>> @@ -106,7 +158,7 @@
>> public:
>> static char ID; // Pass ID, replacement for typeid
>> explicit LoopUnswitch(bool Os = false) :
>> - LoopPass(ID), MaxSize(Threshold), OptimizeForSize(Os), redoLoop(false),
>> + LoopPass(ID), OptimizeForSize(Os), redoLoop(false),
>> currentLoop(NULL), DT(NULL), loopHeader(NULL),
>> loopPreheader(NULL) {
>> initializeLoopUnswitchPass(*PassRegistry::getPassRegistry());
>> @@ -132,24 +184,7 @@
>> private:
>>
>> virtual void releaseMemory() {
>> -
>> - LoopPropsMapIt LIt = LoopsProperties.find(currentLoop);
>> -
>> - if (LIt != LoopsProperties.end()) {
>> - LoopProperties& Props = LIt->second;
>> - MaxSize += Props.CanBeUnswitchedCount * Props.SizeEstimation;
>> - LoopsProperties.erase(LIt);
>> - }
>> -
>> - // We need to forget about all switches in the current loop.
>> - // FIXME: Do it better than enumerating all blocks of code
>> - // and see if it is a switch instruction.
>> - for (Loop::block_iterator I = currentLoop->block_begin(),
>> - E = currentLoop->block_end(); I != E; ++I) {
>> - SwitchInst* SI = dyn_cast<SwitchInst>((*I)->getTerminator());
>> - if (SI)
>> - UnswitchedVals.erase(SI);
>> - }
>> + BranchesInfo.forgetLoop(currentLoop);
>> }
>>
>> /// RemoveLoopFromWorklist - If the specified loop is on the loop worklist,
>> @@ -161,15 +196,6 @@
>> LoopProcessWorklist.erase(I);
>> }
>>
>> - /// For new loop switches we clone info about values that was
>> - /// already unswitched and has redundant successors.
>> - /// Note, that new loop data is stored inside the VMap.
>> - void CloneUnswitchedVals(const ValueToValueMapTy& VMap,
>> - const BasicBlock* SrcBB);
>> -
>> - bool CountLoop(const Loop* L);
>> - void CloneLoopProperties(const Loop* NewLoop, const Loop* OldLoop);
>> -
>> void initLoopData() {
>> loopHeader = currentLoop->getHeader();
>> loopPreheader = currentLoop->getLoopPreheader();
>> @@ -201,6 +227,113 @@
>>
>> };
>> }
>> +
>> +// Analyze loop. Check its size, calculate is it possible to unswitch
>> +// it. Returns true if we can unswitch this loop.
>> +bool LUAnalysisCache::countLoop(const Loop* L) {
>> +
>> + std::pair<LoopPropsMapIt, bool> InsertRes =
>> + LoopsProperties.insert(std::make_pair(L, LoopProperties()));
>> +
>> + LoopProperties& Props = InsertRes.first->second;
>> +
>> + if (InsertRes.second) {
>> + // New loop.
>> +
>> + // Limit the number of instructions to avoid causing significant code
>> + // expansion, and the number of basic blocks, to avoid loops with
>> + // large numbers of branches which cause loop unswitching to go crazy.
>> + // This is a very ad-hoc heuristic.
>> +
>> + // FIXME: This is overly conservative because it does not take into
>> + // consideration code simplification opportunities and code that can
>> + // be shared by the resultant unswitched loops.
>> + CodeMetrics Metrics;
>> + for (Loop::block_iterator I = L->block_begin(),
>> + E = L->block_end();
>> + I != E; ++I)
>> + Metrics.analyzeBasicBlock(*I);
>> +
>> + Props.SizeEstimation = std::min(Metrics.NumInsts, Metrics.NumBlocks * 5);
>> + Props.CanBeUnswitchedCount = MaxSize / (Props.SizeEstimation);
>> + MaxSize -= Props.SizeEstimation * Props.CanBeUnswitchedCount;
>> + }
>> +
>> + if (!Props.CanBeUnswitchedCount) {
>> + DEBUG(dbgs()<< "NOT unswitching loop %"
>> +<< L->getHeader()->getName()<< ", cost too high:"
>> +<< L->getBlocks().size()<< "\n");
>> +
>> + return false;
>> + }
>> +
>> + // Be careful. This links are good only before new loop addition.
>> + CurrentLoopProperties =&Props;
>> + CurLoopInstructions =&Props.UnswitchedVals;
>> +
>> + return true;
>> +}
>> +
>> +// Clean all data related to given loop.
>> +void LUAnalysisCache::forgetLoop(const Loop* L) {
>> +
>> + LoopPropsMapIt LIt = LoopsProperties.find(L);
>> +
>> + if (LIt != LoopsProperties.end()) {
>> + LoopProperties& Props = LIt->second;
>> + MaxSize += Props.CanBeUnswitchedCount * Props.SizeEstimation;
>> + LoopsProperties.erase(LIt);
>> + }
>> +
>> + CurrentLoopProperties = NULL;
>> + CurLoopInstructions = NULL;
>> +}
>> +
>> +// Mark case value as unswitched.
>> +// Since SI instruction can be partly unswitched, in order to avoid
>> +// extra unswitching in cloned loops keep track all unswitched values.
>> +void LUAnalysisCache::setUnswitched(const SwitchInst* SI, const Value* V) {
>> +
>> + (*CurLoopInstructions)[SI].insert(V);
>> +}
>> +
>> +// Check was this case value unswitched before or not.
>> +bool LUAnalysisCache::isUnswitched(const SwitchInst* SI, const Value* V) {
>> + return (*CurLoopInstructions)[SI].count(V);
>> +}
>> +
>> +// Clone all loop-unswitch related loop properties.
>> +// Redistribute unswitching quotas.
>> +// Note, that new loop data is stored inside the VMap.
>> +void LUAnalysisCache::cloneData(const Loop* NewLoop, const Loop* OldLoop,
>> + const ValueToValueMapTy& VMap) {
>> +
>> + LoopProperties& NewLoopProps = LoopsProperties[NewLoop];
>> + LoopProperties& OldLoopProps = *CurrentLoopProperties;
>> + UnswitchedValsMap& Insts = OldLoopProps.UnswitchedVals;
>> +
>> + // Reallocate "can-be-unswitched quota"
>> +
>> + --OldLoopProps.CanBeUnswitchedCount;
>> + unsigned Quota = OldLoopProps.CanBeUnswitchedCount;
>> + NewLoopProps.CanBeUnswitchedCount = Quota / 2;
>> + OldLoopProps.CanBeUnswitchedCount = Quota - Quota / 2;
>> +
>> + NewLoopProps.SizeEstimation = OldLoopProps.SizeEstimation;
>> +
>> + // Clone unswitched values info:
>> + // for new loop switches we clone info about values that was
>> + // already unswitched and has redundant successors.
>> + for (UnswitchedValsIt I = Insts.begin(); I != Insts.end(); ++I) {
>> + const SwitchInst* OldInst = I->first;
>> + Value* NewI = VMap.lookup(OldInst);
>> + const SwitchInst* NewInst = cast_or_null<SwitchInst>(NewI);
>> + assert(NewInst&& "All instructions that are in SrcBB must be in VMap.");
>> +
>> + NewLoopProps.UnswitchedVals[NewInst] = OldLoopProps.UnswitchedVals[OldInst];
>> + }
>> +}
>> +
>> char LoopUnswitch::ID = 0;
>> INITIALIZE_PASS_BEGIN(LoopUnswitch, "loop-unswitch", "Unswitch loops",
>> false, false)
>> @@ -286,7 +419,7 @@
>>
>> // Probably we reach the quota of branches for this loop. If so
>> // stop unswitching.
>> - if (!CountLoop(currentLoop))
>> + if (!BranchesInfo.countLoop(currentLoop))
>> return false;
>>
>> // Loop over all of the basic blocks in the loop. If we find an interior
>> @@ -324,7 +457,7 @@
>> // some not yet unswitched. Let's find the first not yet unswitched one.
>> for (unsigned i = 1; i< NumCases; ++i) {
>> Constant* UnswitchValCandidate = SI->getCaseValue(i);
>> - if (!UnswitchedVals[SI].count(UnswitchValCandidate)) {
>> + if (!BranchesInfo.isUnswitched(SI, UnswitchValCandidate)) {
>> UnswitchVal = UnswitchValCandidate;
>> break;
>> }
>> @@ -356,75 +489,6 @@
>> return Changed;
>> }
>>
>> -/// For new loop switches we clone info about values that was
>> -/// already unswitched and has redundant successors.
>> -/// Not that new loop data is stored inside the VMap.
>> -void LoopUnswitch::CloneUnswitchedVals(const ValueToValueMapTy& VMap,
>> - const BasicBlock* SrcBB) {
>> -
>> - const SwitchInst* SI = dyn_cast<SwitchInst>(SrcBB->getTerminator());
>> - if (SI&& UnswitchedVals.count(SI)) {
>> - // Don't clone a totally simplified switch.
>> - if (isa<Constant>(SI->getCondition()))
>> - return;
>> - Value* I = VMap.lookup(SI);
>> - assert(I&& "All instructions that are in SrcBB must be in VMap.");
>> - UnswitchedVals[cast<SwitchInst>(I)] = UnswitchedVals[SI];
>> - }
>> -}
>> -
>> -bool LoopUnswitch::CountLoop(const Loop* L) {
>> - std::pair<LoopPropsMapIt, bool> InsertRes =
>> - LoopsProperties.insert(std::make_pair(L, LoopProperties()));
>> -
>> - LoopProperties& Props = InsertRes.first->second;
>> -
>> - if (InsertRes.second) {
>> - // New loop.
>> -
>> - // Limit the number of instructions to avoid causing significant code
>> - // expansion, and the number of basic blocks, to avoid loops with
>> - // large numbers of branches which cause loop unswitching to go crazy.
>> - // This is a very ad-hoc heuristic.
>> -
>> - // FIXME: This is overly conservative because it does not take into
>> - // consideration code simplification opportunities and code that can
>> - // be shared by the resultant unswitched loops.
>> - CodeMetrics Metrics;
>> - for (Loop::block_iterator I = L->block_begin(),
>> - E = L->block_end();
>> - I != E; ++I)
>> - Metrics.analyzeBasicBlock(*I);
>> -
>> - Props.SizeEstimation = std::min(Metrics.NumInsts, Metrics.NumBlocks * 5);
>> - Props.CanBeUnswitchedCount = MaxSize / (Props.SizeEstimation);
>> - MaxSize -= Props.SizeEstimation * Props.CanBeUnswitchedCount;
>> - }
>> -
>> - if (!Props.CanBeUnswitchedCount) {
>> - DEBUG(dbgs()<< "NOT unswitching loop %"
>> -<< L->getHeader()->getName()<< ", cost too high:"
>> -<< L->getBlocks().size()<< "\n");
>> -
>> - return false;
>> - }
>> - return true;
>> -}
>> -
>> -void LoopUnswitch::CloneLoopProperties(
>> - const Loop* NewLoop, const Loop* OldLoop) {
>> -
>> - LoopProperties& OldLoopProps = LoopsProperties[OldLoop];
>> - LoopProperties& NewLoopProps = LoopsProperties[NewLoop];
>> -
>> - --OldLoopProps.CanBeUnswitchedCount;
>> - unsigned Quota = OldLoopProps.CanBeUnswitchedCount;
>> - NewLoopProps.CanBeUnswitchedCount = Quota / 2;
>> - OldLoopProps.CanBeUnswitchedCount = Quota - Quota / 2;
>> -
>> - NewLoopProps.SizeEstimation = OldLoopProps.SizeEstimation;
>> -}
>> -
>> /// isTrivialLoopExitBlock - Check to see if all paths from BB exit the
>> /// loop with no side effects (including infinite loops).
>> ///
>> @@ -529,7 +593,7 @@
>>
>> // Check that it was not unswitched before, since already unswitched
>> // trivial vals are looks trivial too.
>> - if (UnswitchedVals[SI].count(CaseVal))
>> + if (BranchesInfo.isUnswitched(SI, CaseVal))
>> continue;
>> LoopExitBB = LoopExitCandidate;
>> if (Val) *Val = CaseVal;
>> @@ -743,11 +807,6 @@
>> for (unsigned i = 0, e = LoopBlocks.size(); i != e; ++i) {
>> BasicBlock *NewBB = CloneBasicBlock(LoopBlocks[i], VMap, ".us", F);
>>
>> - // Inherit simplified switches info for NewBB
>> - // We needn't pass NewBB since its instructions are already contained
>> - // inside the VMap.
>> - CloneUnswitchedVals(VMap, LoopBlocks[i]);
>> -
>> NewBlocks.push_back(NewBB);
>> VMap[LoopBlocks[i]] = NewBB; // Keep the BB mapping.
>> LPM->cloneBasicBlockSimpleAnalysis(LoopBlocks[i], NewBB, L);
>> @@ -760,7 +819,11 @@
>>
>> // Now we create the new Loop object for the versioned loop.
>> Loop *NewLoop = CloneLoop(L, L->getParentLoop(), VMap, LI, LPM);
>> - CloneLoopProperties(NewLoop, L);
>> +
>> + // Recalculate unswitching quota, inherit simplified switches info for NewBB,
>> + // Probably clone more loop-unswitch related loop properties.
>> + BranchesInfo.cloneData(NewLoop, L, VMap);
>> +
>> Loop *ParentLoop = L->getParentLoop();
>> if (ParentLoop) {
>> // Make sure to add the cloned preheader and exit blocks to the parent loop
>> @@ -1075,7 +1138,7 @@
>> BasicBlock *SISucc = SI->getSuccessor(DeadCase);
>> BasicBlock *Latch = L->getLoopLatch();
>>
>> - UnswitchedVals[SI].insert(Val);
>> + BranchesInfo.setUnswitched(SI, Val);
>>
>> if (!SI->findCaseDest(SISucc)) continue; // Edge is critical.
>> // If the DeadCase successor dominates the loop latch, then the
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> 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