[Beignet] [PATCH 4/7] fix bug that LOADI cannot inherits debug info

He Junyan junyan.he at inbox.com
Thu Oct 8 07:28:40 PDT 2015


On Fri, Sep 18, 2015 at 05:01:54PM +0800, Bai Yannan wrote:
> Date: Fri, 18 Sep 2015 17:01:54 +0800
> From: Bai Yannan <yannan.bai at intel.com>
> To: beignet at lists.freedesktop.org
> Cc: Bai Yannan <yannan.bai at intel.com>
> Subject: [Beignet] [PATCH 4/7] fix bug that LOADI cannot inherits debug info
> X-Mailer: git-send-email 1.9.1
> 
> Signed-off-by: Bai Yannan <yannan.bai at intel.com>
> ---
>  backend/src/backend/gen_context.cpp        |  4 +---
>  backend/src/backend/gen_insn_selection.cpp | 12 +++---------
>  backend/src/backend/program.cpp            |  4 +++-
>  backend/src/ir/function.cpp                |  7 +++++++
>  backend/src/ir/function.hpp                |  1 +
>  backend/src/llvm/llvm_gen_backend.cpp      | 31 ++++++++++++++++++++++++++++--
>  6 files changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/backend/src/backend/gen_context.cpp b/backend/src/backend/gen_context.cpp
> index 411336e..9264cd9 100644
> --- a/backend/src/backend/gen_context.cpp
> +++ b/backend/src/backend/gen_context.cpp
> @@ -254,6 +254,7 @@ namespace gbe
>    void GenContext::emitLabelInstruction(const SelectionInstruction &insn) {
>      const ir::LabelIndex label(insn.index);
>      this->labelPos.insert(std::make_pair(label, p->store.size()));
> +	SET_GENINSN_DBGINFO(insn);
>    }
>  
>    void GenContext::emitUnaryInstruction(const SelectionInstruction &insn) {
> @@ -631,9 +632,6 @@ namespace gbe
>      const GenRegister dst = ra->genReg(insn.dst(0));
>      const GenRegister src0 = ra->genReg(insn.src(0));
>      const GenRegister src1 = ra->genReg(insn.src(1));
> -	// debug
> -	if(insn.dbginfo.hasdbginfo)
> -		std::cout<<"    *** "<<insn.dbginfo.line<<std::endl;
>  	
>      switch (insn.opcode) {
>        case SEL_OP_SEL:  p->SEL(dst, src0, src1); break;
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index e861b7c..5ad665d 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -1104,26 +1104,19 @@ namespace gbe
>  		{
>  			SelectionInstruction &selinsn = *it;
>  			if(!selinsn.dbginfo.hasdbginfo)
> -				//SET_SELINSN_DBGINFO(selinsn)
> -			{	selinsn.dbginfo.line = line;		
> -				selinsn.dbginfo.col = col;			
> -				selinsn.dbginfo.hasdbginfo = true;}
> -			//else break;
> +				SET_SELINSN_DBGINFO(selinsn)
>  		}
>  	else
>  		for(auto it = this->blockList.rbegin(); it!= this->blockList.rend(); it++)
>  		{
>  			SelectionBlock &block = *it;
> -			for(auto jt = block.insnList.rbegin(); jt!= block.insnList.rend(); jt++)
> +			for(auto jt = block.insnList.begin(); jt!= block.insnList.end(); jt++)
>  			{
>  				SelectionInstruction &selinsn = *jt;
>  				if(!selinsn.dbginfo.hasdbginfo)
>  					SET_SELINSN_DBGINFO(selinsn)
> -				else goto OVER;
>  			}
>  		}
> -	OVER:
> -		;
>    }
>  #undef SET_SELINSN_DBGINFO
>  
> @@ -4768,6 +4761,7 @@ namespace gbe
>        const uint32_t simdWidth = sel.ctx.getSimdWidth();
>        GBE_ASSERTM(label < sel.ctx.getMaxLabel(), "We reached the maximum label number which is reserved for barrier handling");
>        sel.LABEL(label);
> +	  SET_SELOPAQUE_DBGINFO(insn);
>  
>        if(!insn.getParent()->needIf)
>          return true;
> diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp
> index af817de..e317230 100644
> --- a/backend/src/backend/program.cpp
> +++ b/backend/src/backend/program.cpp
> @@ -557,8 +557,10 @@ namespace gbe {
>  
>  	if(getenv("OCL_PROFILING")) {
>  		char * isProfiling =  getenv("OCL_PROFILING");
> -		if(*isProfiling == '1')
> +		if(*isProfiling == '1'){
> +			args.push_back("-o0");
>  			args.push_back("-g");
> +			}
>  	}
>  
>      // The compiler invocation needs a DiagnosticsEngine so it can report problems
> diff --git a/backend/src/ir/function.cpp b/backend/src/ir/function.cpp
> index f87f23a..439b34c 100644
> --- a/backend/src/ir/function.cpp
> +++ b/backend/src/ir/function.cpp
> @@ -389,6 +389,13 @@ namespace ir {
>      return const_cast<Instruction*>(&insn);
>    }
>  
> +  Instruction *BasicBlock::getSuccessorInstruction(Instruction *pos) {
> +	for(auto it = this->begin();it != this->end();it++)
> +		if(it == pos)	
> +			return &(*(++it));
> +	return &(*(--this->end()));
> +  }
> +  
>    LabelIndex BasicBlock::getLabelIndex(void) const {
>      const Instruction *first = this->getFirstInstruction();
>      const LabelInstruction *label = cast<LabelInstruction>(first);
> diff --git a/backend/src/ir/function.hpp b/backend/src/ir/function.hpp
> index 5d00cca..b435bbd 100644
> --- a/backend/src/ir/function.hpp
> +++ b/backend/src/ir/function.hpp
> @@ -71,6 +71,7 @@ namespace ir {
>      /*! Get successors and predecessors */
>      const BlockSet &getSuccessorSet(void) const { return successors; }
>      const BlockSet &getPredecessorSet(void) const { return predecessors; }
> +	Instruction *getSuccessorInstruction(Instruction *pos);
>      /*! Get the label index of this block */
>      LabelIndex getLabelIndex(void) const;
>      /*! Apply the given functor on all instructions */
> diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> index 1ce5e19..b75da22 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -203,9 +203,24 @@ if(OCL_PROFILING){					\
>  	llvm::DebugLoc dg; 				\
>  	GET_INSN_DBGLOC##FLAG(I); 		\
>  	gbe::ir::BasicBlock * bb = ctx.getBlock();				\
> -	gbe::ir::Instruction *curI = bb->getLastInstruction();	\
> -	curI->setDBGInfo(dg.getLine(), dg.getCol());}
> +	for(gbe::ir::Instruction *in = bb->getFirstInstruction(); in!=bb->getLastInstruction(); in=bb->getSuccessorInstruction(in) )\
> +		  if(!in->DBGInfo.hasDBGInfo) in->setDBGInfo(dg.getLine(), dg.getCol());\
> +	}
>  // end define SET_GENIR_DBGINFO
> +#define GET_PRE_LOADI_DBGINFO	\
> +if(OCL_PROFILING)	{	\
> +	gbe::ir::BasicBlock * tb = ctx.getBlock();\
> +	if(tb){gbe::ir::Instruction *curI = tb->getLastInstruction();	\
> +	line = curI->DBGInfo.line;	\
> +	col = curI->DBGInfo.col;}	\
> +}
> +// end define GET_PRE_LOADI_DBGINFO
> +#define SET_LOADI_DBGINFO	\
> +if(OCL_PROFILING)	{	\
> +	gbe::ir::BasicBlock * tb = ctx.getBlock();\
> +	gbe::ir::Instruction *curI = tb->getLastInstruction();	\
> +	curI->setDBGInfo(line, col);}
> +// end define SET_LOADI_DBGINFO	
I think this manner may have some problems.
In most of the cases, this LOADI will load in the imm for later
instruction which has not been generated. So this LOADI should
belong to that LLVM instruction and should share its DebugInfo,
rather than the last instruction.

>  
>  namespace gbe
>  {
> @@ -1779,14 +1794,20 @@ namespace gbe
>        } else {
>          immIndex = ctx.newDoubleImmediate((double)0.0);
>        }
> +	  uint line=0,col=0;
> +	  GET_PRE_LOADI_DBGINFO;
>        ctx.LOADI(dstType, reg, immIndex);
> +	  SET_LOADI_DBGINFO;
>        return reg;
>      }
>  
>      const ir::ImmediateIndex immIndex = this->newImmediate(c, elemID);
>      const ir::Immediate imm = ctx.getImmediate(immIndex);
>      const ir::Register reg = ctx.reg(getFamily(imm.getType()));
> +	uint line=0,col=0;
> +	GET_PRE_LOADI_DBGINFO;
>      ctx.LOADI(imm.getType(), reg, immIndex);
> +    SET_LOADI_DBGINFO;
>      return reg;
>    }
>  
> @@ -2510,14 +2531,20 @@ namespace gbe
>  
>          this->newRegister(const_cast<GlobalVariable*>(&v));
>          ir::Register reg = regTranslator.getScalar(const_cast<GlobalVariable*>(&v), 0);
> +		uint line=0,col=0;
> +	  GET_PRE_LOADI_DBGINFO;
>          ctx.LOADI(ir::TYPE_S32, reg, ctx.newIntegerImmediate(oldSlm + padding/8, ir::TYPE_S32));
> +		SET_LOADI_DBGINFO;		
>        } else if(addrSpace == ir::MEM_CONSTANT || v.isConstant()) {
>          GBE_ASSERT(v.hasInitializer());
>          this->newRegister(const_cast<GlobalVariable*>(&v));
>          ir::Register reg = regTranslator.getScalar(const_cast<GlobalVariable*>(&v), 0);
>          ir::Constant &con = unit.getConstantSet().getConstant(j ++);
>          GBE_ASSERT(con.getName() == v.getName());
> +		uint line=0,col=0;
> +	  GET_PRE_LOADI_DBGINFO;
>          ctx.LOADI(ir::TYPE_S32, reg, ctx.newIntegerImmediate(con.getOffset(), ir::TYPE_S32));
> +		SET_LOADI_DBGINFO;
>        } else {
>          if(v.getName().equals(StringRef("__gen_ocl_printf_buf"))) {
>            ctx.getFunction().getPrintfSet()->setBufBTI(BtiMap.find(const_cast<GlobalVariable*>(&v))->second);
> -- 
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet




More information about the Beignet mailing list