[Beignet] [Printf][PATCH 06/11] Implement emision of printf instruction.

He Junyan junyan.he at inbox.com
Wed Jan 27 20:38:58 PST 2016


After applied the printf patch set, I find the last test still
failed, please help to check.

On Thu, Jan 28, 2016 at 12:33:05PM +0800, He Junyan wrote:
> Date: Thu, 28 Jan 2016 12:33:05 +0800
> From: He Junyan <junyan.he at inbox.com>
> To: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [Printf][PATCH 06/11] Implement emision of printf
>  instruction.
> 
> On Thu, Jan 21, 2016 at 11:30:21AM +0800, Yan Wang wrote:
> > Date: Thu, 21 Jan 2016 11:30:21 +0800
> > From: Yan Wang <yan.wang at linux.intel.com>
> > To: beignet at lists.freedesktop.org
> > Cc: Yan Wang <yan.wang at linux.intel.com>
> > Subject: [Beignet] [Printf][PATCH 06/11] Implement emision of printf
> >  instruction.
> > X-Mailer: git-send-email 2.5.0
> > 
> > Contributor: Junyan He <junyan.he at linux.intel.com>
> > Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
> > ---
> >  backend/src/ir/context.hpp            |  5 ++
> >  backend/src/llvm/llvm_gen_backend.cpp | 89 ++++++++++++++++++++++++++++-------
> >  2 files changed, 78 insertions(+), 16 deletions(-)
> > 
> I think it is better to write another patch to type TUPLE logic
> > diff --git a/backend/src/ir/context.hpp b/backend/src/ir/context.hpp
> > index b95741f..877d639 100644
> > --- a/backend/src/ir/context.hpp
> > +++ b/backend/src/ir/context.hpp
> > @@ -149,6 +149,11 @@ namespace ir {
> >        GBE_ASSERTM(fn != NULL, "No function currently defined");
> >        return fn->file.appendArrayTuple(reg, regNum);
> >      }
> > +    /*! Make a tuple from an array of types */
> > +    INLINE Tuple arrayTypeTuple(const ir::Type *type, uint32_t num) {
> > +      GBE_ASSERTM(fn != NULL, "No function currently defined");
> > +      return fn->file.appendArrayTypeTuple((uint8_t*)type, num);
> > +    }
> >      /*! We just use variadic templates to forward instruction functions */
> >  #define DECL_INSN(NAME, FAMILY) \
> >      template <typename... Args> INLINE void NAME(Args...args);
> > diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> > index dba9dba..cc736d7 100644
> > --- a/backend/src/llvm/llvm_gen_backend.cpp
> > +++ b/backend/src/llvm/llvm_gen_backend.cpp
> > @@ -486,6 +486,9 @@ namespace gbe
> >      typedef map<Value *, SmallVector<Value *, 4>>::iterator PtrOrigMapIter;
> >      // map pointer source to bti
> >      map<Value *, unsigned> BtiMap;
> > +    // map printf pointer source to bti
> > +    int printfBti;
> > +    uint32_t printfNum;
> >      // map ptr to its bti register
> >      map<Value *, Value *> BtiValueMap;
> >      // map ptr to it's base
> > @@ -520,6 +523,8 @@ namespace gbe
> >          unit(unit),
> >          ctx(unit),
> >          regTranslator(ctx),
> > +        printfBti(-1),
> Also need to reset printfBti for each runOnFunction.
> 
> > +        printfNum(0),
> >          LI(0),
> >          TheModule(0),
> >          btiBase(BTI_RESERVED_NUM),
> > @@ -594,7 +599,7 @@ namespace gbe
> >      /*! For all possible pointers, GlobalVariable, function pointer argument,
> >          alloca instruction, find their pointer escape points */
> >      void analyzePointerOrigin(Function &F);
> > -    unsigned getNewBti(Value *origin, bool isImage);
> > +    unsigned getNewBti(Value *origin, bool force);
> >      void assignBti(Function &F);
> >      bool isSingleBti(Value *Val);
> >      Value *getBtiRegister(Value *v);
> > @@ -717,12 +722,7 @@ namespace gbe
> >      // handle load of dword/qword with unaligned address
> >      void emitUnalignedDQLoadStore(ir::Register ptr, Value *llvmValues, ir::AddressSpace addrSpace, ir::Register bti, bool isLoad, bool dwAligned, bool fixedBTI);
> >      void visitInstruction(Instruction &I) {NOT_SUPPORTED;}
> > -    void* getPrintfInfo(CallInst* inst)
> > -    {
> > -      if (&unit.printfs[inst])
> > -        return (void*)&unit.printfs[inst];
> > -      return NULL;
> > -    }
> > +    ir::PrintfSet::PrintfFmt* getPrintfInfo(CallInst* inst) { return &unit.printfs[inst]; }
> 
> I think
>     ir::PrintfSet::PrintfFmt* getPrintfInfo(CallInst* inst) {
> 	      if (unit.printfs.find(inst) == unit.printfs.end())
> 	        return NULL;
> 	
> 	     return &unit.printfs[inst];
> 	 }
> 
> would be better
> 
> >      private:
> >        void setDebugInfo_CTX(llvm::Instruction * insn); // store the debug infomation in context for subsequently passing to Gen insn
> >        ir::ImmediateIndex processConstantImmIndexImpl(Constant *CPV, int32_t index = 0u);
> > @@ -1127,21 +1127,15 @@ namespace gbe
> >      }
> >    }
> >  
> > -  unsigned GenWriter::getNewBti(Value *origin, bool isImage) {
> > +  unsigned GenWriter::getNewBti(Value *origin, bool force) {
> >      unsigned new_bti = 0;
> > -    if (isImage) {
> > +    if (force) {
> >        new_bti = btiBase;
> >        incBtiBase();
> >        return new_bti;
> >      }
> >  
> > -    if(origin->getName().equals(StringRef("__gen_ocl_printf_buf"))) {
> > -      new_bti = btiBase;
> > -      incBtiBase();
> > -    } else if (origin->getName().equals(StringRef("__gen_ocl_printf_index_buf"))) {
> > -      new_bti = btiBase;
> > -      incBtiBase();
> > -    } else if (origin->getName().equals(StringRef("__gen_ocl_profiling_buf"))) {
> > +    if (origin->getName().equals(StringRef("__gen_ocl_profiling_buf"))) {
> >        new_bti = btiBase;
> >        incBtiBase();
> >      }
> > @@ -3716,6 +3710,16 @@ namespace gbe
> >          this->newRegister(&I);
> >          break;
> >        case GEN_OCL_PRINTF:
> > +        this->newRegister(&I);  // fall through
> > +      case GEN_OCL_PUTS:
> > +      {
> > +         // We need a new BTI as printf output.
> > +         if (printfBti < 0) {
> > +           printfBti = this->getNewBti(&I, true);
> > +           ctx.getFunction().getPrintfSet()->setBufBTI(printfBti);
> > +         }
> > +         break;
> > +      }
> >        case GEN_OCL_CALC_TIMESTAMP:
> >        case GEN_OCL_STORE_PROFILING:
> >        case GEN_OCL_DEBUGWAIT:
> > @@ -4527,6 +4531,59 @@ namespace gbe
> >  
> >            case GEN_OCL_PRINTF:
> >            {
> > +            ir::PrintfSet::PrintfFmt* fmt = getPrintfInfo(&I);
> I think add a check here.
> 
>             if (fmt == NULL) {
> 	             break; //Maybe printf(""), NULL string, just ignore.
> 	          }
> 
> > +            ctx.getFunction().getPrintfSet()->append(printfNum, fmt);
> > +
> > +            vector<ir::Register> tupleData;
> > +            vector<ir::Type> tupleTypeData;
> > +            int argNum = static_cast<int>(I.getNumOperands());
> > +            argNum -= 2; // no fmt and last NULL.
> > +            int realArgNum = argNum;
> > +
> > +            for (int n = 0; n < argNum; n++) {
> > +              /* First, ignore %s, the strings are recorded and not passed to GPU. */
> > +              llvm::Constant* args = dyn_cast<llvm::ConstantExpr>(I.getOperand(n + 1));
> > +              llvm::Constant* args_ptr = NULL;
> > +              if (args)
> > +                args_ptr = dyn_cast<llvm::Constant>(args->getOperand(0));
> > +
> > +              if (args_ptr) {
> > +                ConstantDataSequential* fmt_arg = dyn_cast<ConstantDataSequential>(args_ptr->getOperand(0));
> > +                if (fmt_arg && fmt_arg->isCString()) {
> > +                  realArgNum--;
> > +                  continue;
> > +                }
> > +              }
> > +
> > +              Type * type = I.getOperand(n + 1)->getType();
> > +              if (type->isVectorTy()) {
> > +                uint32_t srcElemNum = 0;
> > +                Value *srcValue = I.getOperand(n + 1);
> > +                ir::Type srcType = getVectorInfo(ctx, srcValue, srcElemNum);
> > +                GBE_ASSERT(!(srcType == ir::TYPE_S64 || srcType == ir::TYPE_DOUBLE));
> > +
> > +                uint32_t elemID = 0;
> > +                for (elemID = 0; elemID < srcElemNum; ++elemID) {
> > +                  ir::Register reg = getRegister(srcValue, elemID);
> > +                  tupleData.push_back(reg);
> > +                  tupleTypeData.push_back(srcType);
> > +                }
> > +                realArgNum += srcElemNum - 1;
> > +              } else {
> > +                ir::Register reg = getRegister(I.getOperand(n + 1));
> > +                tupleData.push_back(reg);
> > +                tupleTypeData.push_back(getType(ctx, I.getOperand(n + 1)->getType()));
> > +              }
> > +            }
> > +
> > +            ir::Tuple tuple;
> > +            ir::Tuple typeTuple;
> > +            if (realArgNum > 0) {
> > +              tuple = ctx.arrayTuple(&tupleData[0], realArgNum);
> > +              typeTuple = ctx.arrayTypeTuple(&tupleTypeData[0], realArgNum);
> > +            }
> > +            ctx.PRINTF(getRegister(&I), tuple, typeTuple, realArgNum, printfBti, printfNum);
> > +            printfNum++;
> >              break;
> >            }
> >            case GEN_OCL_CALC_TIMESTAMP:
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet




More information about the Beignet mailing list