[Beignet] [PATCH v3] use global flag 0.0 to control unstructured simple block.

Zhigang Gong zhigang.gong at linux.intel.com
Sun Oct 19 23:29:21 PDT 2014


On Fri, Oct 17, 2014 at 11:33:23AM +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.
> 
> v2: fix the luxmark sala performance degression due to extern flag in a
> BRA instruction.
> 
> v3: fix compiler_switch regression, LOAD/STORE instruction could
> call replaceSrc/replaceDst to generate 2 extra MOV instruction; exclude
> the scalar instructions since they don't have prediction.
No need to exclude scalar instruction. We just need to make sure don't set
a predication flag for those instructions.

I will put a FIXME tag there to track it and should be fixed in the future.

Thanks,
Zhigang Gong.

> 
> this patch is somewhat dangerous to change the instruction structure of block,
> will add sanity check after emitInstructionStream to assert if illegally
> modified.
> 
> Signed-off-by: Luo Xionghu <xionghu.luo at intel.com>
> ---
>  backend/src/backend/gen_insn_selection.cpp |  105 ++++++++++++++++++++++------
>  backend/src/backend/gen_insn_selection.hpp |    1 +
>  backend/src/backend/gen_reg_allocation.cpp |    3 +-
>  3 files changed, 86 insertions(+), 23 deletions(-)
> 
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index b2df76f..ee9f587 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); }
>  
> @@ -405,6 +405,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;
>  
> @@ -926,6 +928,11 @@ namespace gbe
>        SelectionInstruction *mov = this->create(SEL_OP_MOV, 1, 1);
>        mov->src(0) = GenRegister::retype(insn->src(regID), gr.type);
>        mov->state = GenInstructionState(simdWidth);
> +      if(this->block->removeSimpleIfEndif){
> +        mov->state.predicate = GEN_PREDICATE_NORMAL;
> +        mov->state.flag = 0;
> +        mov->state.subFlag = 0;
> +      }
>        if (this->isScalarReg(insn->src(regID).reg()))
>          mov->state.noMask = 1;
>        mov->dst(0) = gr;
> @@ -955,6 +962,11 @@ namespace gbe
>        SelectionInstruction *mov = this->create(SEL_OP_MOV, 1, 1);
>        mov->dst(0) = GenRegister::retype(insn->dst(regID), gr.type);
>        mov->state = GenInstructionState(simdWidth);
> +      if(this->block->removeSimpleIfEndif){
> +        mov->state.predicate = GEN_PREDICATE_NORMAL;
> +        mov->state.flag = 0;
> +        mov->state.subFlag = 0;
> +      }
>        if (simdWidth == 1) {
>          mov->state.noMask = 1;
>          mov->src(0) = GenRegister::retype(GenRegister::vec1(GEN_GENERAL_REGISTER_FILE, gr.reg()), gr.type);
> @@ -1483,6 +1495,38 @@ namespace gbe
>      return false;
>    }
>  
> +  bool Selection::Opaque::isSimpleBlock(const ir::BasicBlock &bb, uint32_t insnNum) {
> +
> +    if(bb.belongToStructure)
> +      return false;
> +
> +    for (int32_t insnID = insnNum-1; insnID >= 0; --insnID) {
> +      SelectionDAG &dag = *insnDAG[insnID];
> +      const ir::Instruction& insn = dag.insn;
> +      if ( (insn.getDstNum() && this->isScalarReg(insn.getDst(0)) == true) ||
> +         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;
> +    }
> +
> +    // there would generate a extra CMP instruction for predicated BRA with extern flag,
> +    // should retrun false to keep the if/endif.
> +    if((insnDAG[insnNum-1]->insn.isMemberOf<ir::BranchInstruction>())){
> +      if (insnDAG[insnNum-1]->insn.getOpcode() == ir::OP_BRA) {
> +        const ir::BranchInstruction &insn = ir::cast<ir::BranchInstruction>(insnDAG[insnNum-1]->insn);
> +        if(insn.isPredicated() && insnDAG[insnNum-1]->child[0] == NULL){
> +          return false;
> +        }
> +      }
> +    }
> +
> +    return true;
> +  }
> +
> +
>    uint32_t Selection::Opaque::buildBasicBlockDAG(const ir::BasicBlock &bb)
>    {
>      using namespace ir;
> @@ -1563,7 +1607,8 @@ 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 < 10 && isSimpleBlock(bb, insnNum);
> +    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 {
> @@ -1584,6 +1629,13 @@ namespace gbe
>  
>          // Start a new code fragment
>          this->startBackwardGeneration();
> +
> +        if(this->block->removeSimpleIfEndif){
> +          this->push();
> +            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
> @@ -1593,6 +1645,13 @@ 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;
> +          this->pop();
> +        }
>          // 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.
> @@ -3836,15 +3895,17 @@ namespace gbe
>              sel.JMPI(GenRegister::immd(0), jip, label);
>            sel.pop();
>          }
> -        sel.push();
> -          sel.curr.predicate = GEN_PREDICATE_NORMAL;
> -          if(!insn.getParent()->needEndif && insn.getParent()->needIf) {
> -            ir::LabelIndex label = insn.getParent()->endifLabel;
> -            sel.IF(GenRegister::immd(0), label, label);
> -          }
> -          else
> -            sel.IF(GenRegister::immd(0), sel.block->endifLabel, sel.block->endifLabel);
> -        sel.pop();
> +        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;
> +              sel.IF(GenRegister::immd(0), label, label);
> +            }
> +            else
> +              sel.IF(GenRegister::immd(0), sel.block->endifLabel, sel.block->endifLabel);
> +          sel.pop();
> +        }
>        }
>  
>        return true;
> @@ -4105,7 +4166,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();
> @@ -4115,7 +4176,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)
> @@ -4163,7 +4224,7 @@ namespace gbe
>            sel.MOV(ip, GenRegister::immuw(uint16_t(dst)));
>            sel.block->endifOffset = -1;
>            sel.curr.predicate = GEN_PREDICATE_NONE;
> -          if (!sel.block->hasBarrier)
> +          if (!sel.block->hasBarrier && !sel.block->removeSimpleIfEndif)
>              sel.ENDIF(GenRegister::immd(0), next);
>            sel.curr.execWidth = 1;
>            if (simdWidth == 16)
> @@ -4179,7 +4240,7 @@ namespace gbe
>          if(insn.getParent()->needEndif)
>            sel.MOV(ip, GenRegister::immuw(uint16_t(dst)));
>          sel.block->endifOffset = -1;
> -        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)
> @@ -4249,13 +4310,13 @@ namespace gbe
>          const Register pred = insn.getPredicateIndex();
>          const LabelIndex jip = insn.getLabelIndex();
>          sel.push();
> -        sel.curr.physicalFlag = 0;
> -        sel.curr.flagIndex = (uint64_t)pred;
> -        sel.curr.externFlag = 1;
> -        sel.curr.inversePredicate = insn.getInversePredicated();
> -        sel.curr.predicate = GEN_PREDICATE_NORMAL;
> -        sel.WHILE(GenRegister::immd(0), jip);
> -        sel.curr.inversePredicate = 0;
> +          sel.curr.physicalFlag = 0;
> +          sel.curr.flagIndex = (uint64_t)pred;
> +          sel.curr.externFlag = 1;
> +          sel.curr.inversePredicate = insn.getInversePredicated();
> +          sel.curr.predicate = GEN_PREDICATE_NORMAL;
> +          sel.WHILE(GenRegister::immd(0), jip);
> +          sel.curr.inversePredicate = 0;
>          sel.pop();
>        } else
>          NOT_IMPLEMENTED;
> diff --git a/backend/src/backend/gen_insn_selection.hpp b/backend/src/backend/gen_insn_selection.hpp
> index e39aa6e..761c5c1 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..a57edb3 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 && insn.state.flag == 0 && insn.state.subFlag == 0) ));
>          }
>          lastID = insnID;
>          insnID++;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list