[llvm] r181532 - [GVN] Split critical-edge on the fly, instead of postpone edge-splitting to next

[Shuxin Yang]

Yes,  but take place in PRE part which only account for very small 
portion of compile time.
Splitting critical edge on the fly dose not help a lot.

On 5/31/13 9:48 AM, Rafael Espíndola wrote:
> can the same be done in line 2466? We are still scheduling splits for
> the next iteration in there.
>
> On 9 May 2013 14:34, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>> Author: shuxin_yang
>> Date: Thu May  9 13:34:27 2013
>> New Revision: 181532
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=181532&view=rev
>> Log:
>> [GVN] Split critical-edge on the fly, instead of postpone edge-splitting to next
>>    iteration.
>>
>>    This on step toward non-iterative GVN. My local hack suggests that getting rid
>> of iteration will speedup GVN by 30%+ on a medium sized input (2k LOC, C++).
>> I cannot explain why not 2x or more at this moment.
>>
>> Modified:
>>      llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=181532&r1=181531&r2=181532&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu May  9 13:34:27 2013
>> @@ -45,6 +45,7 @@
>>   #include "llvm/Target/TargetLibraryInfo.h"
>>   #include "llvm/Transforms/Utils/BasicBlockUtils.h"
>>   #include "llvm/Transforms/Utils/SSAUpdater.h"
>> +#include <vector>
>>   using namespace llvm;
>>   using namespace PatternMatch;
>>
>> @@ -692,6 +693,7 @@ namespace {
>>       void cleanupGlobalSets();
>>       void verifyRemoved(const Instruction *I) const;
>>       bool splitCriticalEdges();
>> +    BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
>>       unsigned replaceAllDominatedUsesWith(Value *From, Value *To,
>>                                            const BasicBlockEdge &Root);
>>       bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root);
>> @@ -1513,7 +1515,7 @@ bool GVN::PerformLoadPRE(LoadInst *LI, A
>>     for (unsigned i = 0, e = UnavailableBlocks.size(); i != e; ++i)
>>       FullyAvailableBlocks[UnavailableBlocks[i]] = false;
>>
>> -  SmallVector<std::pair<TerminatorInst*, unsigned>, 4> NeedToSplit;
>> +  SmallVector<BasicBlock *, 4> CriticalEdgePred;
>>     for (pred_iterator PI = pred_begin(LoadBB), E = pred_end(LoadBB);
>>          PI != E; ++PI) {
>>       BasicBlock *Pred = *PI;
>> @@ -1536,20 +1538,14 @@ bool GVN::PerformLoadPRE(LoadInst *LI, A
>>           return false;
>>         }
>>
>> -      unsigned SuccNum = GetSuccessorNumber(Pred, LoadBB);
>> -      NeedToSplit.push_back(std::make_pair(Pred->getTerminator(), SuccNum));
>> +      CriticalEdgePred.push_back(Pred);
>>       }
>>     }
>>
>> -  if (!NeedToSplit.empty()) {
>> -    toSplit.append(NeedToSplit.begin(), NeedToSplit.end());
>> -    return false;
>> -  }
>> -
>>     // Decide whether PRE is profitable for this load.
>>     unsigned NumUnavailablePreds = PredLoads.size();
>>     assert(NumUnavailablePreds != 0 &&
>> -         "Fully available value should be eliminated above!");
>> +         "Fully available value should already be eliminated!");
>>
>>     // If this load is unavailable in multiple predecessors, reject it.
>>     // FIXME: If we could restructure the CFG, we could make a common pred with
>> @@ -1558,6 +1554,17 @@ bool GVN::PerformLoadPRE(LoadInst *LI, A
>>     if (NumUnavailablePreds != 1)
>>         return false;
>>
>> +  // Split critical edges, and update the unavailable predecessors accordingly.
>> +  for (SmallVector<BasicBlock *, 4>::iterator I = CriticalEdgePred.begin(),
>> +         E = CriticalEdgePred.end(); I != E; I++) {
>> +    BasicBlock *OrigPred = *I;
>> +    BasicBlock *NewPred = splitCriticalEdges(OrigPred, LoadBB);
>> +    PredLoads.erase(OrigPred);
>> +    PredLoads[NewPred] = 0;
>> +    DEBUG(dbgs() << "Split critical edge " << OrigPred->getName() << "->"
>> +                 << LoadBB->getName() << '\n');
>> +  }
>> +
>>     // Check if the load can safely be moved to all the unavailable predecessors.
>>     bool CanDoPRE = true;
>>     SmallVector<Instruction*, 8> NewInsts;
>> @@ -1594,7 +1601,9 @@ bool GVN::PerformLoadPRE(LoadInst *LI, A
>>         if (MD) MD->removeInstruction(I);
>>         I->eraseFromParent();
>>       }
>> -    return false;
>> +    // HINT:Don't revert the edge-splitting as following transformation may
>> +    // also need to split these critial edges.
>> +    return !CriticalEdgePred.empty();
>>     }
>>
>>     // Okay, we can eliminate this load by inserting a reload in the predecessor
>> @@ -2297,8 +2306,6 @@ bool GVN::runOnFunction(Function& F) {
>>     while (ShouldContinue) {
>>       DEBUG(dbgs() << "GVN iteration: " << Iteration << "\n");
>>       ShouldContinue = iterateOnFunction(F);
>> -    if (splitCriticalEdges())
>> -      ShouldContinue = true;
>>       Changed |= ShouldContinue;
>>       ++Iteration;
>>     }
>> @@ -2310,6 +2317,7 @@ bool GVN::runOnFunction(Function& F) {
>>         Changed |= PREChanged;
>>       }
>>     }
>> +
>>     // FIXME: Should perform GVN again after PRE does something.  PRE can move
>>     // computations into blocks where they become fully redundant.  Note that
>>     // we can't do this until PRE's critical edge splitting updates memdep.
>> @@ -2543,6 +2551,15 @@ bool GVN::performPRE(Function &F) {
>>     return Changed;
>>   }
>>
>> +/// Split the critical edge connecting the given two blocks, and return
>> +/// the block inserted to the critical edge.
>> +BasicBlock *GVN::splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ) {
>> +  BasicBlock *BB = SplitCriticalEdge(Pred, Succ, this);
>> +  if (MD)
>> +    MD->invalidateCachedPredecessors();
>> +  return BB;
>> +}
>> +
>>   /// splitCriticalEdges - Split critical edges found during the previous
>>   /// iteration that may enable further optimization.
>>   bool GVN::splitCriticalEdges() {
>> @@ -2569,9 +2586,18 @@ bool GVN::iterateOnFunction(Function &F)
>>          RE = RPOT.end(); RI != RE; ++RI)
>>       Changed |= processBlock(*RI);
>>   #else
>> +  // Save the blocks this function have before transformation begins. GVN may
>> +  // split critical edge, and hence may invalidate the RPO/DT iterator.
>> +  //
>> +  std::vector<BasicBlock *> BBVect;
>> +  BBVect.reserve(256);
>>     for (df_iterator<DomTreeNode*> DI = df_begin(DT->getRootNode()),
>>          DE = df_end(DT->getRootNode()); DI != DE; ++DI)
>> -    Changed |= processBlock(DI->getBlock());
>> +    BBVect.push_back(DI->getBlock());
>> +
>> +  for (std::vector<BasicBlock *>::iterator I = BBVect.begin(), E = BBVect.end();
>> +       I != E; I++)
>> +    Changed |= processBlock(*I);
>>   #endif
>>
>>     return Changed;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits