[Beignet] [PATCH V2 5/5] GBE: structurized loop exit need an extra branching instruction when do reordering.
Luo, Xionghu
xionghu.luo at intel.com
Thu Sep 25 01:45:45 PDT 2014
This patchset LGTM. Thanks.
Will investigate the performance regression and opencv regression later to enable it.
Luo Xionghu
Best Regards
-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Tuesday, September 23, 2014 3:03 PM
To: beignet at lists.freedesktop.org
Cc: Gong, Zhigang
Subject: [Beignet] [PATCH V2 5/5] GBE: structurized loop exit need an extra branching instruction when do reordering.
When we want to reorder the BBs and move the unstructured BB out-of the structured block, we need to add a BRA to the block. If the exit of the structured block is a loop, we need to append a unconditional BRA right after the predicated BRA. Otherwise, we may lost the correct successor if an unstructured BB is moved next to this BB.
After this patch, with loop optimization enabled, there is no regression on both utests and piglit. But there are still a few regressions in opencv test suite:
[----------] Global test environment tear-down [==========] 8 tests from 2 test cases ran. (40041 ms total) [ PASSED ] 2 tests.
[ FAILED ] 6 tests, listed below:
[ FAILED ] OCL_Photo/FastNlMeansDenoising.Mat/2, where GetParam() = (Channels(2), false) [ FAILED ] OCL_Photo/FastNlMeansDenoising.Mat/3, where GetParam() = (Channels(2), true) [ FAILED ] OCL_Photo/FastNlMeansDenoisingColored.Mat/0, where GetParam() = (Channels(3), false) [ FAILED ] OCL_Photo/FastNlMeansDenoisingColored.Mat/1, where GetParam() = (Channels(3), true) [ FAILED ] OCL_Photo/FastNlMeansDenoisingColored.Mat/2, where GetParam() = (Channels(4), false) [ FAILED ] OCL_Photo/FastNlMeansDenoisingColored.Mat/3, where GetParam() = (Channels(4), true)
So let's turn this optimizaion disabled. Will enable it when I fixed all the known issues.
Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
---
backend/src/ir/function.cpp | 19 +++++++++++++++----
backend/src/ir/function.hpp | 5 +++++
backend/src/ir/structural_analysis.cpp | 28 ++++++++++++++++++----------
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/backend/src/ir/function.cpp b/backend/src/ir/function.cpp index deb65ef..3767d9c 100644
--- a/backend/src/ir/function.cpp
+++ b/backend/src/ir/function.cpp
@@ -260,14 +260,23 @@ namespace ir {
jumpToNext = &bb;
return;
}
- const BranchInstruction &insn = cast<BranchInstruction>(*last);
- if (insn.getOpcode() == OP_BRA) {
+ ir::BasicBlock::iterator it = --bb.end();
+ uint32_t handledInsns = 0;
+ while ((handledInsns < 2 && it != bb.end()) &&
+ static_cast<ir::BranchInstruction *>(&*it)->getOpcode() == OP_BRA) {
+ const BranchInstruction &insn = cast<BranchInstruction>(*it);
+ if (insn.getOpcode() != OP_BRA)
+ break;
const LabelIndex label = insn.getLabelIndex();
BasicBlock *target = this->blocks[label];
GBE_ASSERT(target != NULL);
target->predecessors.insert(&bb);
bb.successors.insert(target);
- if ( insn.isPredicated() == true) jumpToNext = &bb;
+ if (insn.isPredicated() == true) jumpToNext = &bb;
+ // If we are going to handle the second bra, this bra must be a predicated bra
+ GBE_ASSERT(handledInsns == 0 || insn.isPredicated() == true);
+ --it;
+ ++handledInsns;
}
});
}
@@ -324,7 +333,9 @@ namespace ir {
BasicBlock::BasicBlock(Function &fn) : needEndif(true), needIf(true), endifLabel(0),
matchingEndifLabel(0), matchingElseLabel(0),
thisElseLabel(0), belongToStructure(false),
- isStructureExit(false), matchingStructureEntry(NULL),
+ isStructureExit(false), isLoopExit(false),
+ hasExtraBra(false),
+ matchingStructureEntry(NULL),
fn(fn) {
this->nextBlock = this->prevBlock = NULL;
}
diff --git a/backend/src/ir/function.hpp b/backend/src/ir/function.hpp index d0b595e..0f145c5 100644
--- a/backend/src/ir/function.hpp
+++ b/backend/src/ir/function.hpp
@@ -122,6 +122,11 @@ namespace ir {
* identified structure. so if isStructureExit is false then matchingStructureEntry
* is meaningless. */
bool isStructureExit;
+ /* This block is an exit point of a loop block. It may not be exit point of
+ the large structure block. */
+ bool isLoopExit;
+ /* This block has an extra branch in the end of the block. */
+ bool hasExtraBra;
BasicBlock *matchingStructureEntry;
/* variable liveout is for if-else structure liveness analysis. eg. we have an sequence of
* bbs of 0, 1, 2, 3, 4 and the CFG is as below:
diff --git a/backend/src/ir/structural_analysis.cpp b/backend/src/ir/structural_analysis.cpp
index 040a7e0..1e98629 100644
--- a/backend/src/ir/structural_analysis.cpp
+++ b/backend/src/ir/structural_analysis.cpp
@@ -59,20 +59,24 @@ namespace analysis
}
void ControlTree::handleSelfLoopNode(Node *loopnode, ir::LabelIndex& whileLabel)
{
+ //NodeList::iterator child_iter =
+ (*it)->children.begin();
ir::BasicBlock *pbb = loopnode->getExit();
- ir::BranchInstruction* pinsn = static_cast<ir::BranchInstruction *>(pbb->getLastInstruction());
- ir::Register reg = pinsn->getPredicateIndex();
+ GBE_ASSERT(pbb->isLoopExit);
ir::BasicBlock::iterator it = pbb->end();
it--;
+ if (pbb->hasExtraBra)
+ it--;
+ ir::BranchInstruction* pinsn = static_cast<ir::BranchInstruction *>(&*it);
+ ir::Register reg = pinsn->getPredicateIndex();
/* since this node is an while node, so we remove the BRA instruction at the bottom of the exit BB of 'node',
* and insert WHILE instead
*/
- pbb->erase(it);
whileLabel = pinsn->getLabelIndex();
ir::Instruction insn = ir::WHILE(whileLabel, reg);
ir::Instruction* p_new_insn = pbb->getParent().newInstruction(insn);
- pbb->append(*p_new_insn);
+ pbb->insertAt(it, *p_new_insn);
pbb->whileLabel = whileLabel;
+ pbb->erase(it);
}
/* recursive mark the bbs' variable needEndif, the bbs all belong to node.*/ @@ -257,11 +261,15 @@ namespace analysis
for(size_t i = 0; i < blocks.size(); ++i)
{
bbs[i] = blocks[i];
- if(bbs[i]->getLastInstruction()->getOpcode() != ir::OP_BRA && i != blocks.size() - 1)
+ if(i != blocks.size() -1 &&
+ (bbs[i]->getLastInstruction()->getOpcode() != ir::OP_BRA ||
+ (bbs[i]->isStructureExit && bbs[i]->isLoopExit)))
{
ir::Instruction insn = ir::BRA(bbs[i]->getNextBlock()->getLabelIndex());
ir::Instruction* pNewInsn = bbs[i]->getParent().newInstruction(insn);
bbs[i]->append(*pNewInsn);
+ if (bbs[i]->isStructureExit && bbs[i]->isLoopExit)
+ bbs[i]->hasExtraBra = true;
}
}
@@ -337,6 +345,9 @@ namespace analysis
it--;
bbs[i]->erase(it);
+
+ if (bbs[i]->hasExtraBra)
+ bbs[i]->hasExtraBra = false;
}
}
}
@@ -346,7 +357,6 @@ namespace analysis
fn->sortLabels();
fn->computeCFG();
-#if 1
it = begin;
while(it != end)
{
@@ -384,9 +394,8 @@ namespace analysis
case SelfLoop:
{
- NodeList::iterator child_iter = (*it)->children.begin();
ir::LabelIndex whilelabel;
- handleSelfLoopNode(*child_iter, whilelabel);
+ handleSelfLoopNode(*it, whilelabel);
}
break;
@@ -397,7 +406,6 @@ namespace analysis
it++;
}
-#endif
}
@@ -868,7 +876,7 @@ namespace analysis
Node* p = new SelfLoopNode(node);
p->canBeHandled = true;
-
+ node->getExit()->isLoopExit = true;
return insertNode(p);
}
else
--
1.8.3.2
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list