Now the root cause has been founded. The allocated surface size is not enough because it is dependent on global size. I Will fix it and resend patch set based on all previous review comments. Thanks.
Yan Wang > 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...@inbox.com> >> To: beignet@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.w...@linux.intel.com> >> > To: beignet@lists.freedesktop.org >> > Cc: Yan Wang <yan.w...@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...@linux.intel.com> >> > Signed-off-by: Yan Wang <yan.w...@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@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/beignet > > > _______________________________________________ > Beignet mailing list > Beignet@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet > _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet