[llvm] r175348 - R600/structurizer: improve loop handling

Christian Konig christian.koenig at amd.com
Sat Feb 16 03:27:45 PST 2013


Author: ckoenig
Date: Sat Feb 16 05:27:45 2013
New Revision: 175348

URL: http://llvm.org/viewvc/llvm-project?rev=175348&view=rev
Log:
R600/structurizer: improve loop handling

Generate more than one loop if it seems to make sense.

This is a candidate for the stable branch.

Signed-off-by: Christian König <christian.koenig at amd.com>
Reviewed-by: Tom Stellard <thomas.stellard at amd.com>

Modified:
    llvm/trunk/lib/Target/R600/AMDGPUStructurizeCFG.cpp

Modified: llvm/trunk/lib/Target/R600/AMDGPUStructurizeCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/AMDGPUStructurizeCFG.cpp?rev=175348&r1=175347&r2=175348&view=diff
==============================================================================
--- llvm/trunk/lib/Target/R600/AMDGPUStructurizeCFG.cpp (original)
+++ llvm/trunk/lib/Target/R600/AMDGPUStructurizeCFG.cpp Sat Feb 16 05:27:45 2013
@@ -43,6 +43,7 @@ typedef DenseMap<DomTreeNode *, unsigned
 typedef DenseMap<BasicBlock *, PhiMap> BBPhiMap;
 typedef DenseMap<BasicBlock *, Value *> BBPredicates;
 typedef DenseMap<BasicBlock *, BBPredicates> PredMap;
+typedef DenseMap<BasicBlock *, BasicBlock*> BB2BBMap;
 typedef DenseMap<BasicBlock *, BBVector> BB2BBVecMap;
 
 // The name for newly created blocks.
@@ -175,29 +176,30 @@ class AMDGPUStructurizeCFG : public Regi
 
   RNVector Order;
   BBSet Visited;
-  PredMap Predicates;
+
   BBPhiMap DeletedPhis;
   BB2BBVecMap AddedPhis;
+
+  PredMap Predicates;
   BranchVector Conditions;
 
-  BasicBlock *LoopStart;
-  BasicBlock *LoopEnd;
-  BBSet LoopTargets;
-  BBPredicates LoopPred;
+  BB2BBMap Loops;
+  PredMap LoopPreds;
+  BranchVector LoopConds;
 
-  void orderNodes();
+  RegionNode *PrevNode;
 
-  Value *buildCondition(BranchInst *Term, unsigned Idx, bool Invert);
+  void orderNodes();
 
-  bool analyzeLoopStart(BasicBlock *From, BasicBlock *To, Value *Condition);
+  void analyzeLoops(RegionNode *N);
 
-  void analyzeNode(RegionNode *N);
+  Value *buildCondition(BranchInst *Term, unsigned Idx, bool Invert);
 
-  void analyzeLoopEnd(RegionNode *N);
+  void gatherPredicates(RegionNode *N);
 
   void collectInfos();
 
-  void insertConditions();
+  void insertConditions(bool Loops);
 
   void delPhiValues(BasicBlock *From, BasicBlock *To);
 
@@ -212,17 +214,19 @@ class AMDGPUStructurizeCFG : public Regi
 
   BasicBlock *getNextFlow(BasicBlock *Dominator);
 
-  BasicBlock *needPrefix(RegionNode *&Prev, RegionNode *Node);
+  BasicBlock *needPrefix(bool NeedEmpty);
 
   BasicBlock *needPostfix(BasicBlock *Flow, bool ExitUseAllowed);
 
-  RegionNode *getNextPrev(BasicBlock *Next);
+  void setPrevNode(BasicBlock *BB);
 
   bool dominatesPredicates(BasicBlock *BB, RegionNode *Node);
 
-  bool isPredictableTrue(RegionNode *Who, RegionNode *Where);
+  bool isPredictableTrue(RegionNode *Node);
+
+  void wireFlow(bool ExitUseAllowed, BasicBlock *LoopEnd);
 
-  RegionNode *wireFlow(RegionNode *&Prev, bool ExitUseAllowed);
+  void handleLoops(bool ExitUseAllowed, BasicBlock *LoopEnd);
 
   void createFlow();
 
@@ -278,6 +282,29 @@ void AMDGPUStructurizeCFG::orderNodes()
   }
 }
 
+/// \brief Determine the end of the loops
+void AMDGPUStructurizeCFG::analyzeLoops(RegionNode *N) {
+
+  if (N->isSubRegion()) {
+    // Test for exit as back edge
+    BasicBlock *Exit = N->getNodeAs<Region>()->getExit();
+    if (Visited.count(Exit))
+      Loops[Exit] = N->getEntry();
+
+  } else {
+    // Test for sucessors as back edge
+    BasicBlock *BB = N->getNodeAs<BasicBlock>();
+    BranchInst *Term = cast<BranchInst>(BB->getTerminator());
+
+    for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
+      BasicBlock *Succ = Term->getSuccessor(i);
+
+      if (Visited.count(Succ))
+        Loops[Succ] = BB;
+    }
+  }
+}
+
 /// \brief Build the condition for one edge
 Value *AMDGPUStructurizeCFG::buildCondition(BranchInst *Term, unsigned Idx,
                                             bool Invert) {
@@ -291,54 +318,20 @@ Value *AMDGPUStructurizeCFG::buildCondit
   return Cond;
 }
 
-/// \brief Analyze the start of a loop and insert predicates as necessary
-bool AMDGPUStructurizeCFG::analyzeLoopStart(BasicBlock *From, BasicBlock *To,
-                                            Value *Condition) {
-  LoopPred[From] = Condition;
-  LoopTargets.insert(To);
-  if (!LoopStart) {
-    LoopStart = To;
-    return true;
-
-  } else if (LoopStart == To)
-    return true;
-
-  // We need to handle the case of intersecting loops, e. g.
-  //
-  //    /----<-----
-  //    |         |
-  // -> A -> B -> C -> D
-  //         |         |
-  //         -----<----/
-
-  RNVector::reverse_iterator OI = Order.rbegin(), OE = Order.rend();
-
-  for (;OI != OE; ++OI)
-    if ((*OI)->getEntry() == LoopStart)
-      break;
-
-  for (;OI != OE && (*OI)->getEntry() != To; ++OI) {
-    BBPredicates &Pred = Predicates[(*OI)->getEntry()];
-    if (!Pred.count(From))
-      Pred[From] = Condition;
-  }
-  return false;
-}
-
 /// \brief Analyze the predecessors of each block and build up predicates
-void AMDGPUStructurizeCFG::analyzeNode(RegionNode *N) {
+void AMDGPUStructurizeCFG::gatherPredicates(RegionNode *N) {
+
   RegionInfo *RI = ParentRegion->getRegionInfo();
   BasicBlock *BB = N->getEntry();
   BBPredicates &Pred = Predicates[BB];
+  BBPredicates &LPred = LoopPreds[BB];
 
   for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
        PI != PE; ++PI) {
 
-    if (!ParentRegion->contains(*PI)) {
-      // It's a branch from outside into our region entry
-      Pred[*PI] = BoolTrue;
+    // Ignore it if it's a branch from outside into our region entry
+    if (!ParentRegion->contains(*PI))
       continue;
-    }
 
     Region *R = RI->getRegionFor(*PI);
     if (R == ParentRegion) {
@@ -355,7 +348,7 @@ void AMDGPUStructurizeCFG::analyzeNode(R
           if (Term->isConditional()) {
             // Try to treat it like an ELSE block
             BasicBlock *Other = Term->getSuccessor(!i);
-            if (Visited.count(Other) && !LoopTargets.count(Other) &&
+            if (Visited.count(Other) && !Loops.count(Other) &&
                 !Pred.count(Other) && !Pred.count(*PI)) {
 
               Pred[Other] = BoolFalse;
@@ -363,13 +356,12 @@ void AMDGPUStructurizeCFG::analyzeNode(R
               continue;
             }
           }
-
+          Pred[*PI] = buildCondition(Term, i, false);
+ 
         } else {
           // Back edge
-          if (analyzeLoopStart(*PI, BB, buildCondition(Term, i, true)))
-            continue;
+          LPred[*PI] = buildCondition(Term, i, true);
         }
-        Pred[*PI] = buildCondition(Term, i, false);
       }
 
     } else {
@@ -383,34 +375,10 @@ void AMDGPUStructurizeCFG::analyzeNode(R
         continue;
 
       BasicBlock *Entry = R->getEntry();
-      if (!Visited.count(Entry))
-        if (analyzeLoopStart(Entry, BB, BoolFalse))
-          continue;
-
-      Pred[Entry] = BoolTrue;
-    }
-  }
-}
-
-/// \brief Determine the end of the loop
-void AMDGPUStructurizeCFG::analyzeLoopEnd(RegionNode *N) {
-
-  if (N->isSubRegion()) {
-    // Test for exit as back edge
-    BasicBlock *Exit = N->getNodeAs<Region>()->getExit();
-    if (Visited.count(Exit))
-      LoopEnd = N->getEntry();
-
-  } else {
-    // Test for sucessors as back edge
-    BasicBlock *BB = N->getNodeAs<BasicBlock>();
-    BranchInst *Term = cast<BranchInst>(BB->getTerminator());
-
-    for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
-      BasicBlock *Succ = Term->getSuccessor(i);
-
-      if (Visited.count(Succ))
-        LoopEnd = BB;
+      if (Visited.count(Entry))
+        Pred[Entry] = BoolTrue;
+      else
+        LPred[Entry] = BoolFalse;
     }
   }
 }
@@ -422,9 +390,8 @@ void AMDGPUStructurizeCFG::collectInfos(
   Predicates.clear();
 
   // and loop infos
-  LoopStart = LoopEnd = 0;
-  LoopTargets.clear();
-  LoopPred.clear();
+  Loops.clear();
+  LoopPreds.clear();
 
   // Reset the visited nodes
   Visited.clear();
@@ -433,42 +400,37 @@ void AMDGPUStructurizeCFG::collectInfos(
        OI != OE; ++OI) {
 
     // Analyze all the conditions leading to a node
-    analyzeNode(*OI);
+    gatherPredicates(*OI);
 
     // Remember that we've seen this node
     Visited.insert((*OI)->getEntry());
 
-    // Find the last back edge
-    analyzeLoopEnd(*OI);
+    // Find the last back edges
+    analyzeLoops(*OI);
   }
-
-  // Both or neither must be set
-  assert(!LoopStart == !LoopEnd);
 }
 
 /// \brief Insert the missing branch conditions
-void AMDGPUStructurizeCFG::insertConditions() {
+void AMDGPUStructurizeCFG::insertConditions(bool Loops) {
+  BranchVector &Conds = Loops ? LoopConds : Conditions;
+  Value *Default = Loops ? BoolTrue : BoolFalse;
   SSAUpdater PhiInserter;
 
-  for (BranchVector::iterator I = Conditions.begin(),
-       E = Conditions.end(); I != E; ++I) {
+  for (BranchVector::iterator I = Conds.begin(),
+       E = Conds.end(); I != E; ++I) {
 
     BranchInst *Term = *I;
-    BasicBlock *Parent = Term->getParent();
-
     assert(Term->isConditional());
 
-    Value *Default = (Parent == LoopEnd) ? BoolTrue : BoolFalse;
+    BasicBlock *Parent = Term->getParent();
+    BasicBlock *SuccTrue = Term->getSuccessor(0);
+    BasicBlock *SuccFalse = Term->getSuccessor(1);
 
     PhiInserter.Initialize(Boolean, "");
     PhiInserter.AddAvailableValue(&Func->getEntryBlock(), Default);
-    if (Parent == LoopEnd)
-      PhiInserter.AddAvailableValue(LoopStart, BoolTrue);
-    else
-      PhiInserter.AddAvailableValue(Parent, BoolFalse);
+    PhiInserter.AddAvailableValue(Loops ? SuccFalse : Parent, Default);
 
-    BasicBlock *Succ = Term->getSuccessor(0);
-    BBPredicates &Preds = (Parent == LoopEnd) ? LoopPred : Predicates[Succ];
+    BBPredicates &Preds = Loops ? LoopPreds[SuccFalse] : Predicates[SuccTrue];
 
     NearestCommonDominator Dominator(DT);
     Dominator.addBlock(Parent, false);
@@ -648,54 +610,24 @@ BasicBlock *AMDGPUStructurizeCFG::getNex
 }
 
 /// \brief Create a new or reuse the previous node as flow node
-BasicBlock *AMDGPUStructurizeCFG::needPrefix(RegionNode *&Prev,
-                                             RegionNode *Node) {
-
-  if (!Prev || Prev->isSubRegion() ||
-      (Node && Node->getEntry() == LoopStart)) {
+BasicBlock *AMDGPUStructurizeCFG::needPrefix(bool NeedEmpty) {
 
-    // We need to insert a flow node, first figure out the dominator
-    DomTreeNode *Dominator = Prev ? DT->getNode(Prev->getEntry()) : 0;
-    if (!Dominator)
-      Dominator = DT->getNode(Node->getEntry())->getIDom();
-    assert(Dominator && "Illegal loop to function entry");
-
-    // then create the flow node
-    BasicBlock *Flow = getNextFlow(Dominator->getBlock());
-
-    // wire up the new flow
-    if (Prev) {
-      changeExit(Prev, Flow, true);
-    } else {
-      // Parent regions entry needs predicates, create a new region entry
-      BasicBlock *Entry = Node->getEntry();
-      for (pred_iterator I = pred_begin(Entry), E = pred_end(Entry);
-           I != E;) {
+  BasicBlock *Entry = PrevNode->getEntry();
 
-        BasicBlock *BB = *(I++);
-        if (ParentRegion->contains(BB))
-          continue;
+  if (!PrevNode->isSubRegion()) {
+    killTerminator(Entry);
+    if (!NeedEmpty || Entry->getFirstInsertionPt() == Entry->end())
+      return Entry;
 
-        // Remove PHY values from outside to our entry node
-        delPhiValues(BB, Entry);
+  } 
 
-        // Update the branch instructions
-        BB->getTerminator()->replaceUsesOfWith(Entry, Flow);
-      }
+  // create a new flow node
+  BasicBlock *Flow = getNextFlow(Entry);
 
-      // Populate the region tree with the new entry
-      for (Region *R = ParentRegion; R && R->getEntry() == Entry;
-           R = R->getParent()) {
-        R->replaceEntry(Flow);
-      }
-    }
-    Prev = ParentRegion->getBBNode(Flow);
-
-  } else {
-    killTerminator(Prev->getEntry());
-  }
-
-  return Prev->getEntry();
+  // and wire it up
+  changeExit(PrevNode, Flow, true);
+  PrevNode = ParentRegion->getBBNode(Flow);
+  return Flow;
 }
 
 /// \brief Returns the region exit if possible, otherwise just a new flow node
@@ -711,9 +643,9 @@ BasicBlock *AMDGPUStructurizeCFG::needPo
   return getNextFlow(Flow);
 }
 
-/// \brief Returns the region node for Netx, or null if Next is the exit
-RegionNode *AMDGPUStructurizeCFG::getNextPrev(BasicBlock *Next) {
-  return ParentRegion->contains(Next) ? ParentRegion->getBBNode(Next) : 0;
+/// \brief Set the previous node
+void AMDGPUStructurizeCFG::setPrevNode(BasicBlock *BB) {
+  PrevNode =  ParentRegion->contains(BB) ? ParentRegion->getBBNode(BB) : 0;
 }
 
 /// \brief Does BB dominate all the predicates of Node ?
@@ -729,11 +661,14 @@ bool AMDGPUStructurizeCFG::dominatesPred
 }
 
 /// \brief Can we predict that this node will always be called?
-bool AMDGPUStructurizeCFG::isPredictableTrue(RegionNode *Who,
-                                             RegionNode *Where) {
+bool AMDGPUStructurizeCFG::isPredictableTrue(RegionNode *Node) {
 
-  BBPredicates &Preds = Predicates[Who->getEntry()];
-  bool Dominated = Where == 0;
+  BBPredicates &Preds = Predicates[Node->getEntry()];
+  bool Dominated = false;
+
+  // Regionentry is always true
+  if (PrevNode == 0)
+    return true;
 
   for (BBPredicates::iterator I = Preds.begin(), E = Preds.end();
        I != E; ++I) {
@@ -741,7 +676,7 @@ bool AMDGPUStructurizeCFG::isPredictable
     if (I->second != BoolTrue)
       return false;
 
-    if (!Dominated && DT->dominates(I->first, Where->getEntry()))
+    if (!Dominated && DT->dominates(I->first, PrevNode->getEntry()))
       Dominated = true;
   }
 
@@ -750,45 +685,69 @@ bool AMDGPUStructurizeCFG::isPredictable
 }
 
 /// Take one node from the order vector and wire it up
-RegionNode *AMDGPUStructurizeCFG::wireFlow(RegionNode *&Prev,
-                                           bool ExitUseAllowed) {
+void AMDGPUStructurizeCFG::wireFlow(bool ExitUseAllowed,
+                                    BasicBlock *LoopEnd) {
 
   RegionNode *Node = Order.pop_back_val();
+  Visited.insert(Node->getEntry());
 
-  if (isPredictableTrue(Node, Prev)) {
+  if (isPredictableTrue(Node)) {
     // Just a linear flow
-    if (Prev) {
-      changeExit(Prev, Node->getEntry(), true);
+    if (PrevNode) {
+      changeExit(PrevNode, Node->getEntry(), true);
     }
-    Prev = Node;
+    PrevNode = Node;
 
   } else {
     // Insert extra prefix node (or reuse last one)
-    BasicBlock *Flow = needPrefix(Prev, Node);
-    if (Node->getEntry() == LoopStart)
-      LoopStart = Flow;
+    BasicBlock *Flow = needPrefix(false);
 
     // Insert extra postfix node (or use exit instead)
     BasicBlock *Entry = Node->getEntry();
-    BasicBlock *Next = needPostfix(Flow, ExitUseAllowed && Entry != LoopEnd);
+    BasicBlock *Next = needPostfix(Flow, ExitUseAllowed);
 
     // let it point to entry and next block
     Conditions.push_back(BranchInst::Create(Entry, Next, BoolUndef, Flow));
     addPhiValues(Flow, Entry);
     DT->changeImmediateDominator(Entry, Flow);
 
-    Prev = Node;
-    while (!Order.empty() && Node->getEntry() != LoopEnd &&
-           !LoopTargets.count(Order.back()->getEntry()) &&
+    PrevNode = Node;
+    while (!Order.empty() && !Visited.count(LoopEnd) &&
            dominatesPredicates(Entry, Order.back())) {
-      Node = wireFlow(Prev, false);
+      handleLoops(false, LoopEnd);
     }
 
-    changeExit(Prev, Next, false);
-    Prev = getNextPrev(Next);
+    changeExit(PrevNode, Next, false);
+    setPrevNode(Next);
   }
+}
+
+void AMDGPUStructurizeCFG::handleLoops(bool ExitUseAllowed,
+                                       BasicBlock *LoopEnd) {
+  RegionNode *Node = Order.back();
+  BasicBlock *LoopStart = Node->getEntry();
 
-  return Node;
+  if (!Loops.count(LoopStart)) {
+    wireFlow(ExitUseAllowed, LoopEnd);
+    return;
+  }
+
+  if (!isPredictableTrue(Node))
+    LoopStart = needPrefix(true);
+
+  LoopEnd = Loops[Node->getEntry()];
+  wireFlow(false, LoopEnd);
+  while (!Visited.count(LoopEnd)) {
+    handleLoops(false, LoopEnd);
+  }
+
+  // Create an extra loop end node
+  LoopEnd = needPrefix(false);
+  BasicBlock *Next = needPostfix(LoopEnd, ExitUseAllowed);
+  LoopConds.push_back(BranchInst::Create(Next, LoopStart,
+                                         BoolUndef, LoopEnd));
+  addPhiValues(LoopEnd, LoopStart);
+  setPrevNode(Next);
 }
 
 /// After this function control flow looks like it should be, but
@@ -801,26 +760,17 @@ void AMDGPUStructurizeCFG::createFlow()
   DeletedPhis.clear();
   AddedPhis.clear();
   Conditions.clear();
+  LoopConds.clear();
 
-  RegionNode *Prev = 0;
-  while (!Order.empty()) {
-
-    RegionNode *Node = wireFlow(Prev, EntryDominatesExit);
-
-    // Create an extra loop end node
-    if (Node->getEntry() == LoopEnd) {
-      LoopEnd = needPrefix(Prev, 0);
-      BasicBlock *Next = needPostfix(LoopEnd, EntryDominatesExit);
+  PrevNode = 0;
+  Visited.clear();
 
-      Conditions.push_back(BranchInst::Create(Next, LoopStart,
-                                              BoolUndef, LoopEnd));
-      addPhiValues(LoopEnd, LoopStart);
-      Prev = getNextPrev(Next);
-    }
+  while (!Order.empty()) {
+    handleLoops(EntryDominatesExit, 0);
   }
 
-  if (Prev)
-    changeExit(Prev, Exit, EntryDominatesExit);
+  if (PrevNode)
+    changeExit(PrevNode, Exit, EntryDominatesExit);
   else
     assert(EntryDominatesExit);
 }
@@ -880,19 +830,21 @@ bool AMDGPUStructurizeCFG::runOnRegion(R
   orderNodes();
   collectInfos();
   createFlow();
-  insertConditions();
+  insertConditions(false);
+  insertConditions(true);
   setPhiValues();
   rebuildSSA();
 
   // Cleanup
   Order.clear();
   Visited.clear();
-  Predicates.clear();
   DeletedPhis.clear();
   AddedPhis.clear();
+  Predicates.clear();
   Conditions.clear();
-  LoopTargets.clear();
-  LoopPred.clear();
+  Loops.clear();
+  LoopPreds.clear();
+  LoopConds.clear();
 
   return true;
 }





More information about the llvm-commits mailing list