> -----Original Message----- > From: Beignet [mailto:[email protected]] On Behalf Of > Yang, Rong R > Sent: Tuesday, April 29, 2014 10:15 AM > To: Gong, Zhigang; [email protected] > Cc: Gong, Zhigang > Subject: Re: [Beignet] [PATCH 2/3] GBE: fix the large if/endif block issue. > > One comment. > > -----Original Message----- > From: Beignet [mailto:[email protected]] On Behalf Of > Zhigang Gong > Sent: Monday, April 28, 2014 9:01 AM > To: [email protected] > Cc: Gong, Zhigang > Subject: [Beignet] [PATCH 2/3] GBE: fix the large if/endif block issue. > > Some test cases have some very large block which contains more than 32768/2 > instructions which could fit into one if/endif block. > > This patch introduce a ifendif fix switch at the GenContext. > Once we encounter one of such error, we set the switch on and then recompile > the kernel. When the switch is on, we will insert extra endif/if pair to the block > to split one if/endif block to multiple ones to fix the large if/endif issue. > > Signed-off-by: Zhigang Gong <[email protected]> > --- > backend/src/backend/gen_context.cpp | 21 ++++++++++++++------- > backend/src/backend/gen_context.hpp | 16 +++++++++++++++- > backend/src/backend/gen_insn_selection.cpp | 19 ++++++++++++++++--- > backend/src/backend/gen_program.cpp | 8 ++++++++ > backend/src/backend/gen_reg_allocation.cpp | 9 ++++----- > 5 files changed, 57 insertions(+), 16 deletions(-) > > diff --git a/backend/src/backend/gen_context.cpp > b/backend/src/backend/gen_context.cpp > index 349349d..64b822e 100644 > --- a/backend/src/backend/gen_context.cpp > +++ b/backend/src/backend/gen_context.cpp > @@ -52,6 +52,7 @@ namespace gbe > this->p = NULL; > this->sel = NULL; > this->ra = NULL; > + this->ifEndifFix = false; > } > > GenContext::~GenContext(void) { > @@ -72,6 +73,7 @@ namespace gbe > this->ra = GBE_NEW(GenRegAllocator, *this); > this->branchPos2.clear(); > this->branchPos3.clear(); > + this->errCode = NO_ERROR; > } > > void GenContext::emitInstructionStream(void) { @@ -97,7 +99,7 @@ > namespace gbe > p->NOP(); > } > > - void GenContext::patchBranches(void) { > + bool GenContext::patchBranches(void) { > using namespace ir; > for (auto pair : branchPos2) { > const LabelIndex label = pair.first; @@ -108,14 +110,17 @@ > namespace gbe > for (auto pair : branchPos3) { > const LabelPair labelPair = pair.first; > const int32_t insnID = pair.second; > - // FIXME the 'labelPair' implementation must be fixed, as it is hard to > - // convert InstructionSelection offset to ASM offset since asm maybe > compacted > const int32_t jip = labelPos.find(labelPair.l0)->second; > const int32_t uip = labelPos.find(labelPair.l1)->second; > - assert((jip - insnID) < 32767 && (jip - insnID) > -32768); > - assert((uip - insnID) < 32767 && (uip - insnID) > -32768); > + if (((jip - insnID) > 32767 || (jip - insnID) < -32768) || > + ((uip - insnID) > 32768 || (uip - insnID) < -32768)) { > + // The only possible error instruction is if/endif here. > + errCode = OUT_OF_RANGE_IF_ENDIF; > + return false; > + } > p->patchJMPI(insnID, (((uip - insnID)) << 16) | ((jip - insnID))); > } > + return true; > } > > void GenContext::clearFlagRegister(void) { @@ -2013,7 +2018,8 @@ > namespace gbe > this->clearFlagRegister(); > this->emitStackPointer(); > this->emitInstructionStream(); > - this->patchBranches(); > + if (this->patchBranches() == false) > + return false; > genKernel->insnNum = p->store.size(); > genKernel->insns = GBE_NEW_ARRAY_NO_ARG(GenInstruction, > genKernel->insnNum); > std::memcpy(genKernel->insns, &p->store[0], genKernel->insnNum * > sizeof(GenInstruction)); @@ -2024,7 +2030,8 @@ namespace gbe > GenNativeInstruction insn; > std::cout << " L0:" << std::endl; > for (uint32_t insnID = 0; insnID < genKernel->insnNum; ) { > - if (labelPos.find((ir::LabelIndex)(curLabel + 1))->second == insnID) { > + if (labelPos.find((ir::LabelIndex)(curLabel + 1))->second == insnID && > + curLabel < this->getFunction().labelNum()) { > std::cout << " L" << curLabel + 1 << ":" << std::endl; > curLabel = (ir::LabelIndex)(curLabel + 1); > } > diff --git a/backend/src/backend/gen_context.hpp > b/backend/src/backend/gen_context.hpp > index dfddd28..3b59797 100644 > --- a/backend/src/backend/gen_context.hpp > +++ b/backend/src/backend/gen_context.hpp > @@ -42,6 +42,13 @@ namespace gbe > class SelectionInstruction; // Pre-RA Gen instruction > class SelectionReg; // Pre-RA Gen register > class GenRegister; > + typedef enum { > + NO_ERROR, > + REGISTER_ALLOCATION_FAIL, > + REGISTER_SPILL_EXCEED_THRESHOLD, > + REGISTER_SPILL_FAIL, > + OUT_OF_RANGE_IF_ENDIF, > + } CompileErrorCode; > > /*! Context is the helper structure to build the Gen ISA or simulation code > * from GenIR > @@ -73,7 +80,7 @@ namespace gbe > /*! Emit the instructions */ > void emitInstructionStream(void); > /*! Set the correct target values for the branches */ > - void patchBranches(void); > + bool patchBranches(void); > /*! Forward ir::Function isSpecialReg method */ > INLINE bool isSpecialReg(ir::Register reg) const { > return fn.isSpecialReg(reg); > @@ -177,12 +184,19 @@ namespace gbe > uint32_t reservedSpillRegs; > bool limitRegisterPressure; > bool relaxMath; > + const bool getIFENDIFFix(void) const { return ifEndifFix; } > + void setIFENDIFFix(bool fix) { ifEndifFix = fix; } > + const CompileErrorCode getErrCode() { return errCode; } > private: > + CompileErrorCode errCode; > + bool ifEndifFix; > /*! Build the curbe patch list for the given kernel */ > void buildPatchList(void); > /*! allocate a new curbe register and insert to curbe pool. */ > void allocCurbeReg(ir::Register reg, gbe_curbe_type value, uint32_t > subValue = 0); > > + friend GenRegAllocator; //!< need to access errCode > directly. > + > }; > > } /* namespace gbe */ > diff --git a/backend/src/backend/gen_insn_selection.cpp > b/backend/src/backend/gen_insn_selection.cpp > index 420737c..d484212 100644 > --- a/backend/src/backend/gen_insn_selection.cpp > +++ b/backend/src/backend/gen_insn_selection.cpp > @@ -196,7 +196,7 @@ namespace gbe > // SelectionBlock > /////////////////////////////////////////////////////////////////////////// > > - SelectionBlock::SelectionBlock(const ir::BasicBlock *bb) : bb(bb) {} > + SelectionBlock::SelectionBlock(const ir::BasicBlock *bb) : bb(bb), > + endifLabel( (ir::LabelIndex) 0){} > > void SelectionBlock::append(ir::Register reg) { tmp.push_back(reg); } > > @@ -975,7 +975,7 @@ namespace gbe > this->LABEL(this->block->endifLabel); > SelectionInstruction *insn = this->appendInsn(SEL_OP_ENDIF, 0, 1); > insn->src(0) = src; > - insn->index = uint16_t(jip); > + insn->index = uint16_t(this->block->endifLabel); > } > > void Selection::Opaque::CMP(uint32_t conditional, Reg src0, Reg src1, Reg > dst) { @@ -1476,13 +1476,26 @@ namespace gbe > // If there is no branch at the end of this block. > > // Try all the patterns from best to worst > - > do { > if ((*it)->emit(*this, dag)) > break; > ++it; > } while (it != end); > GBE_ASSERT(it != end); > + // 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. > + if (this->ctx.getIFENDIFFix() && > + this->block->insnList.size() != 0 && > + this->block->insnList.size() % 1000 == 0 && > + (uint16_t)this->block->endifLabel != 0) { > >>>>>>>if needEndif is false, also need this Endif? As you can see in the above condition check, This endif/if fix will be added only if "endifLabel != 0" which means there is already an endif generated.
The needEndif bool variable in this function is just for those block which doesn't have a branch instruction and has no effect on if/endif fix logic. _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
