Re: [Mesa-dev] [PATCH 3/3] R600: fix assumption in the CFG structurizers loop handling

2013-01-28 Thread Michel Dänzer
On Don, 2013-01-24 at 13:46 +0100, Christian König wrote: 
 From: Christian König christian.koe...@amd.com
 
 The loop handling in the CFG structurizer incorrectly assumed
 that only BasicBlock nodes can have a back edge, but that is
 also possible for the exit edges of subregions.
 
 Fixing 4 more piglit tests on radeonsi.

Thanks!

Tested-by: Michel Dänzer michel.daen...@amd.com


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast |  Debian, X and DRI developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/3] R600: fix assumption in the CFG structurizers loop handling

2013-01-24 Thread Christian König
From: Christian König christian.koe...@amd.com

The loop handling in the CFG structurizer incorrectly assumed
that only BasicBlock nodes can have a back edge, but that is
also possible for the exit edges of subregions.

Fixing 4 more piglit tests on radeonsi.

Signed-off-by: Christian König christian.koe...@amd.com
---
 lib/Target/R600/AMDGPUStructurizeCFG.cpp |  135 ++
 1 file changed, 81 insertions(+), 54 deletions(-)

diff --git a/lib/Target/R600/AMDGPUStructurizeCFG.cpp 
b/lib/Target/R600/AMDGPUStructurizeCFG.cpp
index 70622e7..9528fc2 100644
--- a/lib/Target/R600/AMDGPUStructurizeCFG.cpp
+++ b/lib/Target/R600/AMDGPUStructurizeCFG.cpp
@@ -120,9 +120,9 @@ class AMDGPUStructurizeCFG : public RegionPass {
   void buildPredicate(BranchInst *Term, unsigned Idx,
   BBPredicates Pred, bool Invert);
 
-  void analyzeBlock(BasicBlock *BB);
+  void analyzeNode(RegionNode *N);
 
-  void analyzeLoop(BasicBlock *BB, unsigned LoopIdx);
+  void analyzeLoop(RegionNode *N);
 
   void collectInfos();
 
@@ -227,72 +227,92 @@ void AMDGPUStructurizeCFG::buildPredicate(BranchInst 
*Term, unsigned Idx,
   Pred[Parent] = Cond;
 }
 
-/// \brief Analyze the successors of each block and build up predicates
-void AMDGPUStructurizeCFG::analyzeBlock(BasicBlock *BB) {
-  pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
+/// \brief Analyze the predecessors of each block and build up predicates
+void AMDGPUStructurizeCFG::analyzeNode(RegionNode *N) {
   RegionInfo *RI = ParentRegion-getRegionInfo();
+  BasicBlock *BB = N-getEntry();
   BBPredicates Pred = Predicates[BB];
 
-  for (; PI != PE; ++PI) {
+  for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
+   PI != PE; ++PI) {
+
+// Handle the case where multiple regions start at the same block
+Region *R = *PI != ParentRegion-getEntry() ?
+RI-getRegionFor(*PI) : ParentRegion;
 
-// Ignore self loops
-if (*PI == BB)
+// Edge from inside a subregion to its entry, ignore it
+if (R == N)
   continue;
 
-BranchInst *Term = castBranchInst((*PI)-getTerminator());
+if (R == ParentRegion) {
 
-for (unsigned i = 0, e = Term-getNumSuccessors(); i != e; ++i) {
-  BasicBlock *Succ = Term-getSuccessor(i);
-  if (Succ != BB)
-continue;
+  // It's a top level block in our region
+  BranchInst *Term = castBranchInst((*PI)-getTerminator());
+  for (unsigned i = 0, e = Term-getNumSuccessors(); i != e; ++i) {
+BasicBlock *Succ = Term-getSuccessor(i);
+if (Succ != BB)
+  continue;
+
+// Ignore self loops
+if (*PI != BB)
+  buildPredicate(Term, i, Pred, false);
+
+if (!Visited.count(*PI)) {
+  if (!LoopStart)
+LoopStart = BB;
 
-  // Handle the case where multiple regions start at the same block
-  Region *R = *PI != ParentRegion-getEntry() ?
-  RI-getRegionFor(*PI) : ParentRegion;
+  buildPredicate(Term, i, LoopPred, true);
+}
+  }
 
-  if (R == ParentRegion) {
-// It's a top level block in our region
-buildPredicate(Term, i, Pred, false);
+} else if (ParentRegion-contains(R)) {
 
-  } else if (ParentRegion-contains(R)) {
-// It's a block in a sub region
-while(R-getParent() != ParentRegion)
-  R = R-getParent();
+  // It's an exit from a sub region
+  while(R-getParent() != ParentRegion)
+R = R-getParent();
 
-Pred[R-getEntry()] = BoolTrue;
+  BasicBlock *Entry = R-getEntry();
+  Pred[Entry] = BoolTrue;
+  if (!Visited.count(Entry)) {
+if (!LoopStart)
+  LoopStart = BB;
 
-  } else {
-// It's a branch from outside into our parent region
-Pred[*PI] = BoolTrue;
+LoopPred[Entry] = BoolFalse;
   }
+
+} else {
+  // It's a branch from outside into our entry region
+  Pred[*PI] = BoolTrue;
 }
   }
 }
 
-/// \brief Analyze the conditions leading to loop to a previous block
-void AMDGPUStructurizeCFG::analyzeLoop(BasicBlock *BB, unsigned LoopIdx) {
-  BranchInst *Term = castBranchInst(BB-getTerminator());
+/// \brief Determine the end of the loop
+void AMDGPUStructurizeCFG::analyzeLoop(RegionNode *N) {
 
-  for (unsigned i = 0, e = Term-getNumSuccessors(); i != e; ++i) {
-BasicBlock *Succ = Term-getSuccessor(i);
+  if (N-isSubRegion()) {
+// Test for exit as back edge
+BasicBlock *Exit = N-getNodeAsRegion()-getExit();
+if (Visited.count(Exit))
+  LoopEnd = N-getEntry();
 
-// Ignore it if it's not a back edge
-if (!Visited.count(Succ))
-  continue;
+  } else {
+// Test for sucessors as back edge
+BasicBlock *BB = N-getNodeAsBasicBlock();
+BranchInst *Term = castBranchInst(BB-getTerminator());
 
-buildPredicate(Term, i, LoopPred, true);
+for (unsigned i = 0, e = Term-getNumSuccessors(); i != e; ++i) {
+  BasicBlock *Succ = Term-getSuccessor(i);