On Tue, Sep 30, 2014 at 04:14:11AM +0800, [email protected] wrote:
> From: Luo Xionghu <[email protected]>
>
> filter the simple block out and replace the if/endif with global flag to
> control.
>
> Signed-off-by: Luo Xionghu <[email protected]>
> ---
> 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.
_______________________________________________
Beignet mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/beignet
