[Beignet] [PATCH 1/3] GBE: support const private array initialization.
Zhigang Gong
zhigang.gong at linux.intel.com
Wed Dec 3 22:52:58 PST 2014
The patchset LGTM， will push latter, thanks.
On Thu, Dec 04, 2014 at 10:52:09AM +0800, Ruiling Song wrote:
> Developers are allowed to declare initialized private array like below:
>
> void func() {
> const arr[]={1, 2, 3, 4};
> }
>
> The implementation is simply put them into __constant memory space.
>
> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> ---
> backend/src/backend/gen_insn_selection.cpp | 43 +++++++++++-----------
> backend/src/llvm/llvm_gen_backend.cpp | 54 +++++++++++++++-------------
> 2 files changed, 53 insertions(+), 44 deletions(-)
>
> diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp
> index 96970c7..7ea2498 100644
> --- a/backend/src/backend/gen_insn_selection.cpp
> +++ b/backend/src/backend/gen_insn_selection.cpp
> @@ -2844,7 +2844,6 @@ namespace gbe
> vector<GenRegister> &dst2,
> GenRegister addr,
> uint32_t valueNum,
> - ir::AddressSpace space,
> ir::BTI bti) const
> {
> for (uint32_t x = 0; x < bti.count; x++) {
> @@ -2852,7 +2851,7 @@ namespace gbe
> for (uint32_t dstID = 0; dstID < valueNum; ++dstID)
> dst2[dstID] = sel.selReg(sel.reg(ir::FAMILY_DWORD), ir::TYPE_U32);
>
> - GenRegister temp = getRelativeAddress(sel, addr, space, bti.bti[x]);
> + GenRegister temp = getRelativeAddress(sel, addr, bti.bti[x]);
> sel.UNTYPED_READ(temp, dst2.data(), valueNum, bti.bti[x]);
> if(x > 0) {
> sel.push();
> @@ -2878,7 +2877,7 @@ namespace gbe
> vector<GenRegister> dst2(valueNum);
> for (uint32_t dstID = 0; dstID < valueNum; ++dstID)
> dst2[dstID] = dst[dstID] = sel.selReg(insn.getValue(dstID), TYPE_U32);
> - readDWord(sel, dst, dst2, addr, valueNum, insn.getAddressSpace(), bti);
> + readDWord(sel, dst, dst2, addr, valueNum, bti);
> }
>
> void emitDWordGather(Selection::Opaque &sel,
> @@ -2925,7 +2924,7 @@ namespace gbe
> GBE_ASSERT(valueNum == 1);
> GBE_ASSERT(bti.count == 1);
> vector<GenRegister> dst(valueNum);
> - GenRegister tmpAddr = getRelativeAddress(sel, addr, insn.getAddressSpace(), bti.bti[0]);
> + GenRegister tmpAddr = getRelativeAddress(sel, addr, bti.bti[0]);
> for ( uint32_t dstID = 0; dstID < valueNum; ++dstID)
> dst[dstID] = sel.selReg(insn.getValue(dstID), ir::TYPE_U64);
> sel.READ64(tmpAddr, dst.data(), valueNum, bti.bti[0]);
> @@ -2996,7 +2995,7 @@ namespace gbe
> tmp2[i] = tmp[i] = GenRegister::udxgrf(simdWidth, tmpReg[i]);
> }
>
> - readDWord(sel, tmp, tmp2, address, tmpRegNum, insn.getAddressSpace(), bti);
> + readDWord(sel, tmp, tmp2, address, tmpRegNum, bti);
>
> for(uint32_t i = 0; i < tmpRegNum; i++) {
> unsigned int elemNum = (valueNum - i * (4 / typeSize)) > 4/typeSize ?
> @@ -3099,7 +3098,7 @@ namespace gbe
> sel.ADD(alignedAddr, alignedAddr, GenRegister::immud(pos * 4));
> sel.pop();
> }
> - readDWord(sel, t1, t2, alignedAddr, width, insn.getAddressSpace(), bti);
> + readDWord(sel, t1, t2, alignedAddr, width, bti);
> remainedReg -= width;
> pos += width;
> } while(remainedReg);
> @@ -3124,7 +3123,7 @@ namespace gbe
> if (x > 0)
> tmp = sel.selReg(sel.reg(family, simdWidth == 1), insn.getValueType());
>
> - GenRegister addr = getRelativeAddress(sel, address, insn.getAddressSpace(), bti.bti[x]);
> + GenRegister addr = getRelativeAddress(sel, address, bti.bti[x]);
> readByteAsDWord(sel, elemSize, addr, tmp, simdWidth, bti.bti[x]);
> if (x > 0) {
> sel.push();
> @@ -3151,8 +3150,8 @@ namespace gbe
> sel.INDIRECT_MOVE(dst, src);
> }
>
> - INLINE GenRegister getRelativeAddress(Selection::Opaque &sel, GenRegister address, ir::AddressSpace space, uint8_t bti) const {
> - if(space == ir::MEM_LOCAL || space == ir::MEM_CONSTANT)
> + INLINE GenRegister getRelativeAddress(Selection::Opaque &sel, GenRegister address, uint8_t bti) const {
> + if (bti == 0xfe || bti == BTI_CONSTANT)
> return address;
>
> sel.push();
> @@ -3162,6 +3161,14 @@ namespace gbe
> sel.pop();
> return temp;
> }
> + // check whether all binded table index point to constant memory
> + INLINE bool isAllConstant(const ir::BTI &bti) const {
> + for (int x = 0; x < bti.count; x++) {
> + if (bti.bti[x] != BTI_CONSTANT)
> + return false;
> + }
> + return true;
> + }
>
> INLINE bool emitOne(Selection::Opaque &sel, const ir::LoadInstruction &insn, bool &markChildren) const {
> using namespace ir;
> @@ -3179,14 +3186,10 @@ namespace gbe
> sel.ADD(temp, address, sel.selReg(ocl::slmoffset, ir::TYPE_U32));
> address = temp;
> }
> - BTI bti;
> - if (space == MEM_CONSTANT || space == MEM_LOCAL) {
> - bti.bti[0] = space == MEM_CONSTANT ? BTI_CONSTANT : 0xfe;
> - bti.count = 1;
> - } else {
> - bti = insn.getBTI();
> - }
> - if (space == MEM_CONSTANT) {
> + const BTI &bti = insn.getBTI();
> + bool allConstant = isAllConstant(bti);
> +
> + if (allConstant) {
> // XXX TODO read 64bit constant through constant cache
> // Per HW Spec, constant cache messages can read at least DWORD data.
> // So, byte/short data type, we have to read through data cache.
> @@ -3291,8 +3294,8 @@ namespace gbe
> }
> }
>
> - INLINE GenRegister getRelativeAddress(Selection::Opaque &sel, GenRegister address, ir::AddressSpace space, uint8_t bti) const {
> - if(space == ir::MEM_LOCAL || space == ir::MEM_CONSTANT)
> + INLINE GenRegister getRelativeAddress(Selection::Opaque &sel, GenRegister address, uint8_t bti) const {
> + if(bti == 0xfe)
> return address;
>
> sel.push();
> @@ -3318,7 +3321,7 @@ namespace gbe
>
> BTI bti = insn.getBTI();
> for (int x = 0; x < bti.count; x++) {
> - GenRegister temp = getRelativeAddress(sel, address, space, bti.bti[x]);
> + GenRegister temp = getRelativeAddress(sel, address, bti.bti[x]);
> if (insn.isAligned() == true && elemSize == GEN_BYTE_SCATTER_QWORD)
> this->emitWrite64(sel, insn, temp, bti.bti[x]);
> else if (insn.isAligned() == true && elemSize == GEN_BYTE_SCATTER_DWORD)
> diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> index b6cb4c7..5bff92b 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -848,7 +848,7 @@ namespace gbe
> if(!v.isConstantUsed()) continue;
> const char *name = v.getName().data();
> unsigned addrSpace = v.getType()->getAddressSpace();
> - if(addrSpace == ir::AddressSpace::MEM_CONSTANT) {
> + if(addrSpace == ir::AddressSpace::MEM_CONSTANT || v.isConstant()) {
> GBE_ASSERT(v.hasInitializer());
> const Constant *c = v.getInitializer();
> Type * type = c->getType();
> @@ -1924,7 +1924,7 @@ namespace gbe
> this->newRegister(const_cast<GlobalVariable*>(&v));
> ir::Register reg = regTranslator.getScalar(const_cast<GlobalVariable*>(&v), 0);
> ctx.LOADI(ir::TYPE_S32, reg, ctx.newIntegerImmediate(oldSlm + padding/8, ir::TYPE_S32));
> - } else if(addrSpace == ir::MEM_CONSTANT) {
> + } 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);
> @@ -1942,7 +1942,7 @@ namespace gbe
> ctx.getFunction().getPrintfSet()->setIndexBufBTI(btiBase);
> globalPointer.insert(std::make_pair(&v, incBtiBase()));
> regTranslator.newScalarProxy(ir::ocl::printfiptr, const_cast<GlobalVariable*>(&v));
> - } else if(v.getName().str().substr(0, 4) == ".str") {
> + } else if(v.getName().str().substr(0, 4) == ".str") {
> /* When there are multi printf statements in multi kernel fucntions within the same
> translate unit, if they have the same sting parameter, such as
> kernel_func1 () {
> @@ -1955,7 +1955,7 @@ namespace gbe
> So when translating the kernel_func1, we can not unref that global var, so we will
> get here. Just ignore it to avoid assert. */
> } else {
> - GBE_ASSERT(0);
> + GBE_ASSERT(0 && "Unsupported private memory access pattern");
> }
> }
> }
> @@ -3873,27 +3873,33 @@ handle_write_image:
> for (unsigned i = 0; i < origins.size(); i++) {
> uint8_t new_bti = 0;
> Value *origin = origins[i];
> - unsigned space = origin->getType()->getPointerAddressSpace();
> - switch (space) {
> - case 0:
> - new_bti = BTI_PRIVATE;
> - break;
> - case 1:
> - {
> - GlobalPtrIter iter = globalPointer.find(origin);
> - GBE_ASSERT(iter != globalPointer.end());
> - new_bti = iter->second;
> - break;
> + // all constant put into constant cache, including __constant & const __private
> + if (isa<GlobalVariable>(origin)
> + && dyn_cast<GlobalVariable>(origin)->isConstant()) {
> + new_bti = BTI_CONSTANT;
> + } else {
> + unsigned space = origin->getType()->getPointerAddressSpace();
> + switch (space) {
> + case 0:
> + new_bti = BTI_PRIVATE;
> + break;
> + case 1:
> + {
> + GlobalPtrIter iter = globalPointer.find(origin);
> + GBE_ASSERT(iter != globalPointer.end());
> + new_bti = iter->second;
> + break;
> + }
> + case 2:
> + new_bti = BTI_CONSTANT;
> + break;
> + case 3:
> + new_bti = 0xfe;
> + break;
> + default:
> + GBE_ASSERT(0 && "address space not unhandled in gatherBTI()\n");
> + break;
> }
> - case 2:
> - new_bti = BTI_CONSTANT;
> - break;
> - case 3:
> - new_bti = 0xfe;
> - break;
> - default:
> - GBE_ASSERT(0 && "address space not unhandled in gatherBTI()\n");
> - break;
> }
>
> // avoid duplicate
> --
> 1.7.10.4
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list