From 68204669b80899944ced2c553282b5cdcc136ba9 Mon Sep 17 00:00:00 2001 From: Zhigang Gong Date: Wed, 28 May 2014 10:27:36 +0800 Subject: [PATCH] GBE: Consolidate all read/write instruction's bti handling. The previous bti handling for each read/write instruction is slightly different from each other. There are two major bugs, the OP_ATOMIC store the bti in different position, so the post scheduling for ATOMIC instruction is buggy. The second bug is the DWORD_GATHER instruction is not in the isRead list. That may cause potential bug. This patch fixes both of them. Signed-off-by: Zhigang Gong Reviewed-by: Yang Rong --- backend/src/backend/gen_context.cpp | 20 ++++++++--------- backend/src/backend/gen_insn_scheduling.cpp | 8 +++---- backend/src/backend/gen_insn_selection.cpp | 29 ++++++++++++------------ backend/src/backend/gen_insn_selection.hpp | 35 +++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 28 deletions(-) diff --git a/backend/src/backend/gen_context.cpp b/backend/src/backend/gen_context.cpp index f4c80e3..0f29468 100644 --- a/backend/src/backend/gen_context.cpp +++ b/backend/src/backend/gen_context.cpp @@ -1649,7 +1649,7 @@ namespace gbe const GenRegister src = ra->genReg(insn.src(0)); const GenRegister dst = ra->genReg(insn.dst(0)); const uint32_t function = insn.extra.function; - const uint32_t bti = insn.extra.elem; + const uint32_t bti = insn.getbti(); p->ATOMIC(dst, function, src, bti, insn.srcNum); } @@ -1780,14 +1780,14 @@ namespace gbe const GenRegister dst = ra->genReg(insn.dst(tmpRegSize)); const GenRegister tmp = ra->genReg(insn.dst(0)); const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); p->READ64(dst, tmp, tempAddr, src, bti, elemNum); } void GenContext::emitUntypedReadInstruction(const SelectionInstruction &insn) { const GenRegister dst = ra->genReg(insn.dst(0)); const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); const uint32_t elemNum = insn.extra.elem; p->UNTYPED_READ(dst, src, bti, elemNum); } @@ -1800,14 +1800,14 @@ namespace gbe const uint32_t elemNum = insn.extra.elem; const GenRegister addr = ra->genReg(insn.src(0)); //tmpRegSize + 1)); const GenRegister data = ra->genReg(insn.src(1)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); p->MOV(src, addr); p->WRITE64(src, data, bti, elemNum, sel->isScalarReg(data.reg())); } void GenContext::emitUntypedWriteInstruction(const SelectionInstruction &insn) { const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); const uint32_t elemNum = insn.extra.elem; p->UNTYPED_WRITE(src, bti, elemNum); } @@ -1815,14 +1815,14 @@ namespace gbe void GenContext::emitByteGatherInstruction(const SelectionInstruction &insn) { const GenRegister dst = ra->genReg(insn.dst(0)); const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); const uint32_t elemSize = insn.extra.elem; p->BYTE_GATHER(dst, src, bti, elemSize); } void GenContext::emitByteScatterInstruction(const SelectionInstruction &insn) { const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); const uint32_t elemSize = insn.extra.elem; p->BYTE_SCATTER(src, bti, elemSize); } @@ -1859,14 +1859,14 @@ namespace gbe void GenContext::emitDWordGatherInstruction(const SelectionInstruction &insn) { const GenRegister dst = ra->genReg(insn.dst(0)); const GenRegister src = ra->genReg(insn.src(0)); - const uint32_t bti = insn.extra.function; + const uint32_t bti = insn.getbti(); p->DWORD_GATHER(dst, src, bti); } void GenContext::emitSampleInstruction(const SelectionInstruction &insn) { const GenRegister dst = ra->genReg(insn.dst(0)); const GenRegister msgPayload = GenRegister::retype(ra->genReg(insn.src(0)), GEN_TYPE_F); - const unsigned char bti = insn.extra.rdbti; + const unsigned char bti = insn.getbti(); const unsigned char sampler = insn.extra.sampler; const unsigned int msgLen = insn.extra.rdmsglen; uint32_t simdWidth = p->curr.execWidth; @@ -1906,7 +1906,7 @@ namespace gbe void GenContext::emitTypedWriteInstruction(const SelectionInstruction &insn) { const GenRegister header = GenRegister::retype(ra->genReg(insn.src(0)), GEN_TYPE_UD); - const uint32_t bti = insn.extra.bti; + const uint32_t bti = insn.getbti(); p->TYPED_WRITE(header, true, bti); } diff --git a/backend/src/backend/gen_insn_scheduling.cpp b/backend/src/backend/gen_insn_scheduling.cpp index 6f37ed4..106d608 100644 --- a/backend/src/backend/gen_insn_scheduling.cpp +++ b/backend/src/backend/gen_insn_scheduling.cpp @@ -380,7 +380,7 @@ namespace gbe // Track writes in memory if (insn.isWrite()) { - const uint32_t index = this->getIndex(insn.extra.function); + const uint32_t index = this->getIndex(insn.getbti()); this->nodes[index] = node; } @@ -483,7 +483,7 @@ namespace gbe // read-after-write in memory if (insn.isRead()) { - const uint32_t index = tracker.getIndex(insn.extra.function); + const uint32_t index = tracker.getIndex(insn.getbti()); tracker.addDependency(node, index, READ_AFTER_WRITE); } //read-after-write of scratch memory @@ -516,7 +516,7 @@ namespace gbe // write-after-write in memory if (insn.isWrite()) { - const uint32_t index = tracker.getIndex(insn.extra.function); + const uint32_t index = tracker.getIndex(insn.getbti()); tracker.addDependency(node, index, WRITE_AFTER_WRITE); } @@ -546,7 +546,7 @@ namespace gbe // write-after-read in memory if (insn.isRead()) { - const uint32_t index = tracker.getIndex(insn.extra.function); + const uint32_t index = tracker.getIndex(insn.getbti()); tracker.addDependency(index, node, WRITE_AFTER_READ); } diff --git a/backend/src/backend/gen_insn_selection.cpp b/backend/src/backend/gen_insn_selection.cpp index 13412f7..cb86e34 100644 --- a/backend/src/backend/gen_insn_selection.cpp +++ b/backend/src/backend/gen_insn_selection.cpp @@ -167,7 +167,8 @@ namespace gbe this->opcode == SEL_OP_READ64 || this->opcode == SEL_OP_ATOMIC || this->opcode == SEL_OP_BYTE_GATHER || - this->opcode == SEL_OP_SAMPLE; + this->opcode == SEL_OP_SAMPLE || + this->opcode == SEL_OP_DWORD_GATHER; } bool SelectionInstruction::isWrite(void) const { @@ -843,9 +844,9 @@ namespace gbe if (poolOffset > ctx.reservedSpillRegs){ if (GBE_DEBUG) - std::cerr << "Instruction (#" << (uint32_t)insn.opcode - << ") dst too large pooloffset " - << (uint32_t)poolOffset << std::endl; + std::cerr << "Instruction (#" << (uint32_t)insn.opcode + << ") dst too large pooloffset " + << (uint32_t)poolOffset << std::endl; return false; } while(!regSet.empty()) { @@ -1058,7 +1059,7 @@ namespace gbe if(srcNum > 1) insn->src(1) = src1; if(srcNum > 2) insn->src(2) = src2; insn->extra.function = function; - insn->extra.elem = bti; + insn->setbti(bti); SelectionVector *vector = this->appendVector(); vector->regNum = srcNum; @@ -1089,7 +1090,7 @@ namespace gbe /* temporary addr register is to be modified, set it to dst registers.*/ insn->dst(elemNum) = tempAddr; insn->src(0) = addr; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = valueNum; // Only the temporary registers need contiguous allocation @@ -1117,7 +1118,7 @@ namespace gbe for (uint32_t elemID = 0; elemID < elemNum; ++elemID) insn->dst(elemID) = dst[elemID]; insn->src(0) = addr; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = elemNum; // Sends require contiguous allocation @@ -1148,7 +1149,7 @@ namespace gbe insn->src(elemID + 1) = src[elemID]; for (uint32_t elemID = 0; elemID < dstNum; ++elemID) insn->dst(elemID) = dst[elemID]; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = srcNum; // Only the addr + temporary registers need to be contiguous. @@ -1169,7 +1170,7 @@ namespace gbe insn->src(0) = addr; for (uint32_t elemID = 0; elemID < elemNum; ++elemID) insn->src(elemID+1) = src[elemID]; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = elemNum; // Sends require contiguous allocation for the sources @@ -1188,7 +1189,7 @@ namespace gbe // Instruction to encode insn->src(0) = addr; insn->dst(0) = dst; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = elemSize; // byte gather requires vector in the sense that scalar are not allowed @@ -1208,7 +1209,7 @@ namespace gbe // Instruction to encode insn->src(0) = addr; insn->src(1) = src; - insn->extra.function = bti; + insn->setbti(bti); insn->extra.elem = elemSize; // value and address are contiguous in the send @@ -1226,7 +1227,7 @@ namespace gbe insn->state.noMask = 1; insn->src(0) = addr; insn->dst(0) = dst; - insn->extra.function = bti; + insn->setbti(bti); vector->regNum = 1; vector->isSrc = 0; vector->reg = &insn->dst(0); @@ -1613,7 +1614,7 @@ namespace gbe msgVector->isSrc = 1; msgVector->reg = &insn->src(0); - insn->extra.rdbti = bti; + insn->setbti(bti); insn->extra.sampler = sampler; insn->extra.rdmsglen = msgNum; insn->extra.isLD = isLD; @@ -1642,7 +1643,7 @@ namespace gbe for( i = 0; i < msgNum; ++i, ++elemID) insn->src(elemID) = msgs[i]; - insn->extra.bti = bti; + insn->setbti(bti); insn->extra.msglen = msgNum; insn->extra.is3DWrite = is3D; // Sends require contiguous allocation diff --git a/backend/src/backend/gen_insn_selection.hpp b/backend/src/backend/gen_insn_selection.hpp index 6ce2249..0140a63 100644 --- a/backend/src/backend/gen_insn_selection.hpp +++ b/backend/src/backend/gen_insn_selection.hpp @@ -142,7 +142,42 @@ namespace gbe uint32_t ID; /*! Variable sized. Destinations and sources go here */ GenRegister regs[0]; + INLINE uint32_t getbti() const { + GBE_ASSERT(isRead() || isWrite()); + switch (opcode) { + case SEL_OP_ATOMIC: return extra.elem; + case SEL_OP_BYTE_SCATTER: + case SEL_OP_WRITE64: + case SEL_OP_DWORD_GATHER: + case SEL_OP_UNTYPED_WRITE: + case SEL_OP_UNTYPED_READ: + case SEL_OP_BYTE_GATHER: + case SEL_OP_READ64: return extra.function; + case SEL_OP_SAMPLE: return extra.rdbti; + case SEL_OP_TYPED_WRITE: return extra.bti; + default: + GBE_ASSERT(0); + } + return 0; + } private: + INLINE void setbti(uint32_t bti) { + GBE_ASSERT(isRead() || isWrite()); + switch (opcode) { + case SEL_OP_ATOMIC: extra.elem = bti; return; + case SEL_OP_BYTE_SCATTER: + case SEL_OP_WRITE64: + case SEL_OP_UNTYPED_WRITE: + case SEL_OP_DWORD_GATHER: + case SEL_OP_UNTYPED_READ: + case SEL_OP_BYTE_GATHER: + case SEL_OP_READ64: extra.function = bti; return; + case SEL_OP_SAMPLE: extra.rdbti = bti; return; + case SEL_OP_TYPED_WRITE: extra.bti = bti; return; + default: + GBE_ASSERT(0); + } + } /*! Just Selection class can create SelectionInstruction */ SelectionInstruction(SelectionOpcode, uint32_t dstNum, uint32_t srcNum); // Allocates (with a linear allocator) and owns SelectionInstruction -- 2.7.4