[Beignet] [Printf][PATCH 06/11] Implement emision of printf instruction.
yan.wang at linux.intel.com
yan.wang at linux.intel.com
Sun Jan 31 19:25:10 PST 2016
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.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
>
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
>
More information about the Beignet
mailing list