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 <zhigang.gong@intel.com>
Reviewed-by: Yang Rong <rong.r.yang@intel.com>

4 files changed, 64 insertions, 28 deletions

diff --git a/backend/src/backend/gen_context.cpp b/backend/src/backend/gen_context.cpp
index f4c80e34..0f294686 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 6f37ed4c..106d608e 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 13412f73..cb86e340 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 6ce22497..0140a631 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