[LLVMdev] Question about GVN

[Bob Wilson]

On May 4, 2010, at 9:41 AM, Jakub Staszak wrote:

> Hello, I was investigating GVN.cpp file and I found suspicious part:
> 
> 1587   bool NeedToSplitEdges = false;
> 1588   for (pred_iterator PI = pred_begin(LoadBB), E = pred_end(LoadBB);
> 1589        PI != E; ++PI) {
> 1590     BasicBlock *Pred = *PI;
> 1591     if (IsValueFullyAvailableInBlock(Pred, FullyAvailableBlocks)) {
> 1592       continue;
> 1593     }
> 1594     PredLoads[Pred] = 0;
> 1595 
> 1596     if (Pred->getTerminator()->getNumSuccessors() != 1) {
> 1597       if (isa<IndirectBrInst>(Pred->getTerminator())) {
> 1598         DEBUG(dbgs() << "COULD NOT PRE LOAD BECAUSE OF INDBR CRITICAL EDGE '"
> 1599               << Pred->getName() << "': " << *LI << '\n');
> 1600         return false;
> 1601       }
> 1602       unsigned SuccNum = GetSuccessorNumber(Pred, LoadBB);
> 1603       toSplit.push_back(std::make_pair(Pred->getTerminator(), SuccNum));
> 1604       NeedToSplitEdges = true;
> 1605     }
> 1606   }
> 1607   if (NeedToSplitEdges)
> 1608     return false;
> 
> Because it iterates over predecessors it may happen that not the first one has IndirectBrInst terminator.
> However some "toSplit" edges can be already added. Is this OK?
> Maybe I just misunderstood something?

I suppose it might end up splitting some edges unnecessarily.  That isn't a huge problem but it would be good to fix.  The edges to be split could be saved on a  local list and added to "toSplit" only after looking at all the predecessors.  I'll fix that.