[Beignet] [PATCH] GBE: Consolidate all read/write instruction's bti handling.
Zhigang Gong
zhigang.gong at intel.com
Wed May 28 00:04:52 PDT 2014
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 <zhigang.gong at intel.com>
---
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
--
1.8.3.2
More information about the Beignet
mailing list