[Beignet] [PATCH 3/3] GBE: fix the hacky usage of invalid register.
Zhigang Gong
zhigang.gong at linux.intel.com
Thu Feb 5 23:18:55 PST 2015
Right, I will keep it for some weeks, and if we don't find any user of it. I will choose to delete it then.
Thanks for the review comment.
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Yang, Rong R
> Sent: Friday, February 6, 2015 2:52 PM
> To: Gong, Zhigang; beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: Re: [Beignet] [PATCH 3/3] GBE: fix the hacky usage of invalid register.
>
> Is ir::ocl::invalid useless and could remove it?
>
> The other part of patchset LGTM.
>
> > -----Original Message-----
> > From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf
> > Of Zhigang Gong
> > Sent: Wednesday, February 4, 2015 12:02
> > To: beignet at lists.freedesktop.org
> > Cc: Gong, Zhigang
> > Subject: [Beignet] [PATCH 3/3] GBE: fix the hacky usage of invalid register.
> >
> > Orignally, the IR instruction only has 64 bits thus we don't have
> > enough room for coordinate number. Now the IR instructions has been
> > extent to 128 bits, we don't need to use the invalid register to
> > implicitly pass the coordinate number any more.
> >
> > Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> > ---
> > backend/src/backend/gen_insn_selection.cpp | 40
> > +++++++++++-------------
> > ------
> > backend/src/ir/instruction.cpp | 20 ++++++++-------
> > backend/src/ir/instruction.hpp | 4 +--
> > backend/src/llvm/llvm_gen_backend.cpp | 31 ++++++-----------------
> > 4 files changed, 36 insertions(+), 59 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_insn_selection.cpp
> > b/backend/src/backend/gen_insn_selection.cpp
> > index 81fa5f8..106ea82 100644
> > --- a/backend/src/backend/gen_insn_selection.cpp
> > +++ b/backend/src/backend/gen_insn_selection.cpp
> > @@ -4318,10 +4318,6 @@ namespace gbe
> > using namespace ir;
> > GBE_ASSERT(insn.getSrcType() != TYPE_FLOAT);
> > uint32_t srcNum = insn.getSrcNum();
> > - if (insn.getSrc(1) == ir::ocl::invalid) //not 3D
> > - srcNum = 1;
> > - else if (insn.getSrc(2) == ir::ocl::invalid)
> > - srcNum = 2;
> > msgPayloads[0] = sel.selReg(insn.getSrc(0), insn.getSrcType());
> > msgPayloads[1] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> > sel.MOV(msgPayloads[1], GenRegister::immud(0)); @@ -4364,12
> > +4360,6 @@ namespace gbe
> > for (valueID = 0; valueID < insn.getDstNum(); ++valueID)
> > dst[valueID] = sel.selReg(insn.getDst(valueID),
> > insn.getDstType());
> >
> > - GBE_ASSERT(srcNum == 3);
> > - if (insn.getSrc(1) == ir::ocl::invalid) //not 3D
> > - srcNum = 1;
> > - else if (insn.getSrc(2) == ir::ocl::invalid)
> > - srcNum = 2;
> > -
> > if (insn.getSamplerOffset() != 0) {
> > if(sel.getLdMsgOrder() < LD_MSG_ORDER_SKL)
> > this->emitLd_ivb(sel, insn, msgPayloads, msgLen); @@
> > -4405,7 +4395,7 @@ namespace gbe
> > const uint32_t simdWidth = sel.ctx.getSimdWidth();
> > GenRegister msgs[9]; // (header + U + V + R + LOD + 4)
> > const uint32_t msgNum = (8 / (simdWidth / 8)) + 1;
> > - const uint32_t coordNum = 3;
> > + const uint32_t dim = insn.getSrcNum() - 4;
> >
> > if (simdWidth == 16) {
> > for(uint32_t i = 0; i < msgNum; i++) @@ -4413,18 +4403,18 @@
> > namespace gbe
> > } else {
> > uint32_t valueID = 0;
> > msgs[0] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> > - for(uint32_t msgID = 1; msgID < 1 + coordNum; msgID++,
> valueID++)
> > + for(uint32_t msgID = 1; msgID < 1 + dim; msgID++, valueID++)
> > msgs[msgID] = sel.selReg(insn.getSrc(msgID - 1),
> > insn.getCoordType());
> >
> > - // fake u.
> > - if (insn.getSrc(1) == ir::ocl::invalid)
> > + // fake v.
> > + if (dim < 2)
> > msgs[2] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> > // fake w.
> > - if (insn.getSrc(2) == ir::ocl::invalid)
> > + if (dim < 3)
> > msgs[3] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> > // LOD.
> > msgs[4] = sel.selReg(sel.reg(FAMILY_DWORD), TYPE_U32);
> > - for(uint32_t msgID = 5; valueID < insn.getSrcNum(); msgID++,
> valueID++)
> > + for(uint32_t msgID = dim + 2; valueID < insn.getSrcNum();
> > + msgID++, valueID++)
> > msgs[msgID] = sel.selReg(insn.getSrc(valueID),
> insn.getSrcType());
> > }
> >
> > @@ -4441,14 +4431,14 @@ namespace gbe
> > sel.curr.execWidth = 8;
> > // Set zero LOD.
> > if (simdWidth == 8)
> > - sel.MOV(msgs[4], GenRegister::immud(0));
> > + sel.MOV(msgs[dim + 1], GenRegister::immud(0));
> > else
> > sel.MOV(GenRegister::Qn(msgs[2], 0), GenRegister::immud(0));
> > sel.pop();
> >
> > uint32_t bti = insn.getImageIndex();
> > if (simdWidth == 8)
> > - sel.TYPED_WRITE(msgs, msgNum, bti, insn.getSrc(2) !=
> ir::ocl::invalid);
> > + sel.TYPED_WRITE(msgs, msgNum, bti, dim == 3);
> > else {
> > sel.push();
> > sel.curr.execWidth = 8;
> > @@ -4464,16 +4454,16 @@ namespace gbe
> > sel.curr.quarterControl = (quarter == 0) ?
> GEN_COMPRESSION_Q1 :
> > GEN_COMPRESSION_Q2;
> > // Set U,V,W
> > QUARTER_MOV0(msgs, 1, sel.selReg(insn.getSrc(0),
> > insn.getCoordType()));
> > - if (insn.getSrc(1) != ir::ocl::invalid) //not 2D
> > + if (dim > 1)
> > QUARTER_MOV0(msgs, 2, sel.selReg(insn.getSrc(1),
> > insn.getCoordType()));
> > - if (insn.getSrc(2) != ir::ocl::invalid) //not 3D
> > + if (dim > 2)
> > QUARTER_MOV0(msgs, 3, sel.selReg(insn.getSrc(2),
> > insn.getCoordType()));
> > // Set R, G, B, A
> > - QUARTER_MOV1(msgs, 5, sel.selReg(insn.getSrc(3),
> > insn.getSrcType()));
> > - QUARTER_MOV1(msgs, 6, sel.selReg(insn.getSrc(4),
> > insn.getSrcType()));
> > - QUARTER_MOV1(msgs, 7, sel.selReg(insn.getSrc(5),
> > insn.getSrcType()));
> > - QUARTER_MOV1(msgs, 8, sel.selReg(insn.getSrc(6),
> > insn.getSrcType()));
> > - sel.TYPED_WRITE(msgs, msgNum, bti, insn.getSrc(2) !=
> ir::ocl::invalid);
> > + QUARTER_MOV1(msgs, 5, sel.selReg(insn.getSrc(dim),
> > insn.getSrcType()));
> > + QUARTER_MOV1(msgs, 6, sel.selReg(insn.getSrc(dim + 1),
> > insn.getSrcType()));
> > + QUARTER_MOV1(msgs, 7, sel.selReg(insn.getSrc(dim + 2),
> > insn.getSrcType()));
> > + QUARTER_MOV1(msgs, 8, sel.selReg(insn.getSrc(dim + 3),
> > insn.getSrcType()));
> > + sel.TYPED_WRITE(msgs, msgNum, bti, dim == 3);
> > #undef QUARTER_MOV0
> > #undef QUARTER_MOV1
> > }
> > diff --git a/backend/src/ir/instruction.cpp
> > b/backend/src/ir/instruction.cpp index 795ff07..928e365 100644
> > --- a/backend/src/ir/instruction.cpp
> > +++ b/backend/src/ir/instruction.cpp
> > @@ -506,10 +506,11 @@ namespace ir {
> > public TupleDstPolicy<SampleInstruction>
> > {
> > public:
> > - SampleInstruction(uint8_t imageIdx, Tuple dstTuple, Tuple srcTuple,
> bool
> > dstIsFloat, bool srcIsFloat, uint8_t sampler, uint8_t samplerOffset) {
> > + SampleInstruction(uint8_t imageIdx, Tuple dstTuple, Tuple
> > + srcTuple, uint8_t srcNum, bool dstIsFloat, bool srcIsFloat, uint8_t
> > + sampler, uint8_t samplerOffset) {
> > this->opcode = OP_SAMPLE;
> > this->dst = dstTuple;
> > this->src = srcTuple;
> > + this->srcNum = srcNum;
> > this->dstIsFloat = dstIsFloat;
> > this->srcIsFloat = srcIsFloat;
> > this->samplerIdx = sampler;
> > @@ -544,7 +545,7 @@ namespace ir {
> > uint8_t samplerIdx:4;
> > uint8_t samplerOffset:2;
> > uint8_t imageIdx;
> > - static const uint32_t srcNum = 3;
> > + uint8_t srcNum;
> > static const uint32_t dstNum = 4;
> > };
> >
> > @@ -555,9 +556,10 @@ namespace ir {
> > {
> > public:
> >
> > - INLINE TypedWriteInstruction(uint8_t imageIdx, Tuple srcTuple, Type
> > srcType, Type coordType) {
> > + INLINE TypedWriteInstruction(uint8_t imageIdx, Tuple srcTuple,
> > + uint8_t srcNum, Type srcType, Type coordType) {
> > this->opcode = OP_TYPED_WRITE;
> > this->src = srcTuple;
> > + this->srcNum = srcNum;
> > this->coordType = coordType;
> > this->srcType = srcType;
> > this->imageIdx = imageIdx;
> > @@ -580,12 +582,12 @@ namespace ir {
> > uint8_t srcType;
> > uint8_t coordType;
> > uint8_t imageIdx;
> > + // bti, u, [v], [w], 4 data elements
> > + uint8_t srcNum;
> >
> > INLINE uint8_t getImageIndex(void) const { return this->imageIdx; }
> > INLINE Type getSrcType(void) const { return (Type)this->srcType; }
> > INLINE Type getCoordType(void) const { return
> (Type)this->coordType; }
> > - // bti, u, v, w, 4 data elements
> > - static const uint32_t srcNum = 7;
> > Register dst[0]; //!< No dest register
> > };
> >
> > @@ -1774,12 +1776,12 @@ DECL_MEM_FN(GetImageInfoInstruction,
> > uint8_t, getImageIndex(void), getImageIndex
> > }
> >
> > // SAMPLE
> > - Instruction SAMPLE(uint8_t imageIndex, Tuple dst, Tuple src, bool
> > dstIsFloat, bool srcIsFloat, uint8_t sampler, uint8_t samplerOffset) {
> > - return internal::SampleInstruction(imageIndex, dst, src, dstIsFloat,
> > srcIsFloat, sampler, samplerOffset).convert();
> > + Instruction SAMPLE(uint8_t imageIndex, Tuple dst, Tuple src,
> > + uint8_t
> > srcNum, bool dstIsFloat, bool srcIsFloat, uint8_t sampler, uint8_t
> > samplerOffset) {
> > + return internal::SampleInstruction(imageIndex, dst, src, srcNum,
> > + dstIsFloat, srcIsFloat, sampler, samplerOffset).convert();
> > }
> >
> > - Instruction TYPED_WRITE(uint8_t imageIndex, Tuple src, Type
> > srcType, Type coordType) {
> > - return internal::TypedWriteInstruction(imageIndex, src, srcType,
> > coordType).convert();
> > + Instruction TYPED_WRITE(uint8_t imageIndex, Tuple src, uint8_t
> > + srcNum,
> > Type srcType, Type coordType) {
> > + return internal::TypedWriteInstruction(imageIndex, src, srcNum,
> > + srcType, coordType).convert();
> > }
> >
> > Instruction GET_IMAGE_INFO(int infoType, Register dst, uint8_t
> > imageIndex, Register infoReg) { diff --git
> > a/backend/src/ir/instruction.hpp b/backend/src/ir/instruction.hpp
> > index 484e7d1..6963111 100644
> > --- a/backend/src/ir/instruction.hpp
> > +++ b/backend/src/ir/instruction.hpp
> > @@ -706,9 +706,9 @@ namespace ir {
> > Instruction READ_ARF(Type type, Register dst, ARFRegister arf);
> > Instruction REGION(Register dst, Register src, uint32_t offset);
> > /*! typed write */
> > - Instruction TYPED_WRITE(uint8_t imageIndex, Tuple src, Type
> > srcType, Type coordType);
> > + Instruction TYPED_WRITE(uint8_t imageIndex, Tuple src, uint8_t
> > + srcNum, Type srcType, Type coordType);
> > /*! sample textures */
> > - Instruction SAMPLE(uint8_t imageIndex, Tuple dst, Tuple src, bool
> > dstIsFloat, bool srcIsFloat, uint8_t sampler, uint8_t samplerOffset);
> > + Instruction SAMPLE(uint8_t imageIndex, Tuple dst, Tuple src,
> > + uint8_t srcNum, bool dstIsFloat, bool srcIsFloat, uint8_t sampler,
> > + uint8_t samplerOffset);
> > /*! get image information , such as width/height/depth/... */
> > Instruction GET_IMAGE_INFO(int infoType, Register dst, uint8_t
> > imageIndex, Register infoReg);
> > /*! label labelIndex */
> > diff --git a/backend/src/llvm/llvm_gen_backend.cpp
> > b/backend/src/llvm/llvm_gen_backend.cpp
> > index 04f3092..3e22526 100644
> > --- a/backend/src/llvm/llvm_gen_backend.cpp
> > +++ b/backend/src/llvm/llvm_gen_backend.cpp
> > @@ -3460,16 +3460,8 @@ error:
> > GBE_ASSERT(isFloatCoord == requiredFloatCoord);
> >
> > vector<ir::Register> dstTupleData, srcTupleData;
> > - for (uint32_t elemID = 0; elemID < 3; elemID++) {
> > - ir::Register reg;
> > -
> > - if (elemID < imageDim)
> > - reg = this->getRegister(coordVal, elemID);
> > - else
> > - reg = ir::ocl::invalid;
> > -
> > - srcTupleData.push_back(reg);
> > - }
> > + for (uint32_t elemID = 0; elemID < imageDim; elemID++)
> > + srcTupleData.push_back(this->getRegister(coordVal,
> > + elemID));
> >
> > uint32_t elemNum;
> > ir::Type dstType = getVectorInfo(ctx, &I, elemNum); @@
> > -3480,9
> > +3472,9 @@ error:
> > dstTupleData.push_back(reg);
> > }
> > const ir::Tuple dstTuple = ctx.arrayTuple(&dstTupleData[0],
> elemNum);
> > - const ir::Tuple srcTuple = ctx.arrayTuple(&srcTupleData[0], 3);
> > + const ir::Tuple srcTuple =
> > + ctx.arrayTuple(&srcTupleData[0], imageDim);
> >
> > - ctx.SAMPLE(imageID, dstTuple, srcTuple, dstType ==
> ir::TYPE_FLOAT,
> > + ctx.SAMPLE(imageID, dstTuple, srcTuple, imageDim, dstType
> > + == ir::TYPE_FLOAT,
> > requiredFloatCoord, sampler, samplerOffset);
> > break;
> > }
> > @@ -3501,16 +3493,9 @@ error:
> > vector<ir::Register> srcTupleData;
> > GBE_ASSERT(imageDim >= 1 && imageDim <= 3);
> >
> > - for (uint32_t elemID = 0; elemID < 3; elemID++) {
> > - ir::Register reg;
> > + for (uint32_t elemID = 0; elemID < imageDim; elemID++)
> > + srcTupleData.push_back(this->getRegister(*AI, elemID));
> >
> > - if (elemID < imageDim)
> > - reg = this->getRegister(*AI, elemID);
> > - else
> > - reg = ir::ocl::invalid;
> > -
> > - srcTupleData.push_back(reg);
> > - }
> > ++AI; GBE_ASSERT(AI != AE);
> > uint32_t elemNum;
> > ir::Type srcType = getVectorInfo(ctx, *AI, elemNum); @@
> > -3520,8
> > +3505,8 @@ error:
> > const ir::Register reg = this->getRegister(*AI, elemID);
> > srcTupleData.push_back(reg);
> > }
> > - const ir::Tuple srcTuple = ctx.arrayTuple(&srcTupleData[0], 7);
> > - ctx.TYPED_WRITE(imageID, srcTuple, srcType, ir::TYPE_U32);
> > + const ir::Tuple srcTuple =
> > + ctx.arrayTuple(&srcTupleData[0], imageDim 4);
> > + ctx.TYPED_WRITE(imageID, srcTuple, imageDim + 4, srcType,
> > + ir::TYPE_U32);
> > break;
> > }
> > case GEN_OCL_MUL_HI_INT:
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > 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