[Beignet] [PATCH] use global flag 0.0 to control unstructured simple block.
Zhigang Gong
zhigang.gong at linux.intel.com
Sun Oct 12 17:57:42 PDT 2014
On Tue, Sep 30, 2014 at 04:14:11AM +0800, xionghu.luo at intel.com wrote:
> From: Luo Xionghu <xionghu.luo at intel.com>
>
> filter the simple block out and replace the if/endif with global flag to
> control.
>
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
> backend/src/backend/gen_insn_selection.cpp | 50 ++++++++++++++++++++++++----
> backend/src/backend/gen_insn_selection.hpp | 1 +
> backend/src/backend/gen_reg_allocation.cpp | 3 +-
> 3 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index f284ae1..e3547c6 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -217,7 +217,7 @@ namespace gbe
> // SelectionBlock
> ///////////////////////////////////////////////////////////////////////////
>
> - SelectionBlock::SelectionBlock(const ir::BasicBlock *bb) : bb(bb), isLargeBlock(false), endifLabel( (ir::LabelIndex) 0){}
> + SelectionBlock::SelectionBlock(const ir::BasicBlock *bb) : bb(bb), isLargeBlock(false), endifLabel( (ir::LabelIndex) 0), removeSimpleIfEndif(false){}
>
> void SelectionBlock::append(ir::Register reg) { tmp.push_back(reg); }
>
> @@ -403,6 +403,8 @@ namespace gbe
> uint32_t buildBasicBlockDAG(const ir::BasicBlock &bb);
> /*! Perform the selection on the basic block */
> void matchBasicBlock(const ir::BasicBlock &bb, uint32_t insnNum);
> + /*! a simple block can use predication instead of if/endif*/
> + bool isSimpleBlock(const ir::BasicBlock &bb, uint32_t insnNum);
> /*! A root instruction needs to be generated */
> bool isRoot(const ir::Instruction &insn) const;
>
> @@ -1471,6 +1473,26 @@ namespace gbe
> return false;
> }
>
> + bool Selection::Opaque::isSimpleBlock(const ir::BasicBlock &bb, uint32_t insnNum) {
> + for (int32_t insnID = insnNum-1; insnID >= 0; --insnID) {
> + SelectionDAG &dag = *insnDAG[insnID];
> + const ir::Instruction& insn = dag.insn;
> + if(insn.isMemberOf<ir::CompareInstruction>() ||
> + insn.isMemberOf<ir::SelectInstruction>() ||
> + insn.getOpcode() == ir::OP_SIMD_ANY ||
> + insn.getOpcode() == ir::OP_SIMD_ALL ||
> + insn.getOpcode() == ir::OP_ELSE)
> + return false;
> + }
> +
> + if(!(insnDAG[insnNum-1]->insn.isMemberOf<ir::BranchInstruction>()) ||
> + insnDAG[insnNum-1]->insn.getOpcode() == ir::OP_ENDIF)
The above condtion check is not good. We need different condtion for structured and
unstructured block. For unstructured BB, this condition check should not be necessary.
For structured BB, We need to check whether this is an innermost if then or if then else.
L1:
...
IF (%1)
L2
...
ELSE
L3
...
ENDIF
The IF / ELSE / ENDIF could be removed in the above three BBs if the L2 and L3 are simple
BB and it is innermost if/then/else.
> + return false;
> +
> + return true;
> + }
> +
> +
> uint32_t Selection::Opaque::buildBasicBlockDAG(const ir::BasicBlock &bb)
> {
> using namespace ir;
> @@ -1551,7 +1573,9 @@ namespace gbe
> // Bottom up code generation
> bool needEndif = this->block->hasBranch == false && !this->block->hasBarrier;
> needEndif = needEndif && bb.needEndif;
> - if (needEndif) {
> + this->block->removeSimpleIfEndif = insnNum < 5 && isSimpleBlock(bb, insnNum);
could you tell why you choose 5 as a threshold here?
> + //this->block->removeSimpleIfEndif = false;//this->block->removeSimpleIfEndif && needEndif;
> + if (needEndif && !this->block->removeSimpleIfEndif) {
> if(!bb.needIf) // this basic block is the exit of a structure
> this->ENDIF(GenRegister::immd(0), bb.endifLabel, bb.endifLabel);
> else {
> @@ -1572,6 +1596,12 @@ namespace gbe
>
> // Start a new code fragment
> this->startBackwardGeneration();
> +
> + if(this->block->removeSimpleIfEndif){
> + this->curr.predicate = GEN_PREDICATE_NORMAL;
> + this->curr.flag = 0;
> + this->curr.subFlag = 0;
> + }
> // If there is no branch at the end of this block.
>
> // Try all the patterns from best to worst
> @@ -1581,6 +1611,12 @@ namespace gbe
> ++it;
> } while (it != end);
> GBE_ASSERT(it != end);
> +
> + if(this->block->removeSimpleIfEndif){
> + this->curr.predicate = GEN_PREDICATE_NONE;
> + this->curr.flag = 0;
> + this->curr.subFlag = 0;
> + }
The above method is not the correct way to handle context states. You should emit a this->push()
before you modify the states, then latter, use this->pop() to restore.
> // If we are in if/endif fix mode, and this block is
> // large enough, we need to insert endif/if pair to eliminate
> // the too long if/endif block.
> @@ -3808,7 +3844,8 @@ namespace gbe
> sel.JMPI(GenRegister::immd(0), jip, label);
> sel.pop();
> }
> - sel.push();
> + if(!sel.block->removeSimpleIfEndif){
> + sel.push();
> sel.curr.predicate = GEN_PREDICATE_NORMAL;
> if(!insn.getParent()->needEndif && insn.getParent()->needIf) {
> ir::LabelIndex label = insn.getParent()->endifLabel;
> @@ -3816,7 +3853,8 @@ namespace gbe
> }
> else
> sel.IF(GenRegister::immd(0), sel.block->endifLabel, sel.block->endifLabel);
> - sel.pop();
> + sel.pop();
Please keep the indent between a push()/pop() pair.
> + }
> }
>
> return true;
> @@ -4077,7 +4115,7 @@ namespace gbe
> sel.curr.predicate = GEN_PREDICATE_NORMAL;
> sel.MOV(ip, GenRegister::immuw(uint16_t(dst)));
> sel.curr.predicate = GEN_PREDICATE_NONE;
> - if (!sel.block->hasBarrier)
> + if (!sel.block->hasBarrier && !sel.block->removeSimpleIfEndif)
> sel.ENDIF(GenRegister::immd(0), nextLabel);
> sel.block->endifOffset = -1;
> sel.pop();
> @@ -4087,7 +4125,7 @@ namespace gbe
> if(insn.getParent()->needEndif)
> sel.MOV(ip, GenRegister::immuw(uint16_t(dst)));
>
> - if (!sel.block->hasBarrier) {
> + if (!sel.block->hasBarrier && !sel.block->removeSimpleIfEndif) {
> if(insn.getParent()->needEndif && !insn.getParent()->needIf)
> sel.ENDIF(GenRegister::immd(0), insn.getParent()->endifLabel, insn.getParent()->endifLabel);
> else if(insn.getParent()->needEndif)
> diff --git a/backend/src/backend/gen_insn_selection.hpp b/backend/src/backend/gen_insn_selection.hpp
> index 9bcce6f..974acf3 100644
> --- a/backend/src/backend/gen_insn_selection.hpp
> +++ b/backend/src/backend/gen_insn_selection.hpp
> @@ -233,6 +233,7 @@ namespace gbe
> int endifOffset;
> bool hasBarrier;
> bool hasBranch;
> + bool removeSimpleIfEndif;
> };
>
> /*! Owns the selection engine */
> diff --git a/backend/src/backend/gen_reg_allocation.cpp b/backend/src/backend/gen_reg_allocation.cpp
> index 067c9ce..3e18c3a 100644
> --- a/backend/src/backend/gen_reg_allocation.cpp
> +++ b/backend/src/backend/gen_reg_allocation.cpp
> @@ -1072,7 +1072,8 @@ namespace gbe
> insn.opcode == SEL_OP_JMPI ||
> insn.state.predicate == GEN_PREDICATE_NONE ||
> (block.hasBarrier && insn.opcode == SEL_OP_MOV) ||
> - (insn.state.flag == 0 && insn.state.subFlag == 1)));
> + (insn.state.flag == 0 && insn.state.subFlag == 1) ||
> + block.removeSimpleIfEndif));
You'd better use (block.removeSimpleIfEndif && insn.state.flag == 0 && insn.state.subFlag == 0).
Zhigang Gong,
Thanks.
More information about the Beignet
mailing list