[Beignet] [PATCH] GBE: fix a uniform analysis bug.

[Zhigang Gong]

From: Zhigang Gong <zhigang.gong at gmail.com>

If a value is defined in a loop and is used out-of the
loop. That value could not be a uniform(scalar) value.
The reason is that value may be assigned different
scalar value on different lanes when it reenters with
different lanes actived.
Thanks for yang rong reporting this bug.

Signed-off-by: Zhigang Gong <zhigang.gong at gmail.com>
---
 backend/src/ir/liveness.cpp | 63 +++++++++++++++++++++++++++++++--------------
 backend/src/ir/liveness.hpp |  3 ++-
 2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/backend/src/ir/liveness.cpp b/backend/src/ir/liveness.cpp
index c3d6fe4..afed476 100644
--- a/backend/src/ir/liveness.cpp
+++ b/backend/src/ir/liveness.cpp
@@ -43,13 +43,52 @@ namespace ir {
     // Now with iterative analysis, we compute liveout and livein sets
     this->computeLiveInOut();
     // extend register (def in loop, use out-of-loop) liveness to the whole loop
-    this->computeExtraLiveInOut();
+    set<Register> extentRegs;
+    this->computeExtraLiveInOut(extentRegs);
+    // analyze uniform values. The extentRegs contains all the values which is
+    // defined in a loop and use out-of-loop which could not be a uniform. The reason
+    // is that when it reenter the second time, it may active different lanes. So
+    // reenter many times may cause it has different values in different lanes.
+    this->analyzeUniform(&extentRegs);
   }
 
   Liveness::~Liveness(void) {
     for (auto &pair : liveness) GBE_SAFE_DELETE(pair.second);
   }
 
+  void Liveness::analyzeUniform(set<Register> *extentRegs) {
+    fn.foreachBlock([this, extentRegs](const BasicBlock &bb) {
+      const_cast<BasicBlock&>(bb).foreach([this, extentRegs](const Instruction &insn) {
+        const uint32_t srcNum = insn.getSrcNum();
+        const uint32_t dstNum = insn.getDstNum();
+        bool uniform = true;
+        for (uint32_t srcID = 0; srcID < srcNum; ++srcID) {
+          const Register reg = insn.getSrc(srcID);
+          if (!fn.isUniformRegister(reg))
+            uniform = false;
+        }
+        // A destination is a killed value
+        for (uint32_t dstID = 0; dstID < dstNum; ++dstID) {
+          const Register reg = insn.getDst(dstID);
+          int opCode = insn.getOpcode();
+          // FIXME, ADDSAT and uniform vector should be supported.
+          if (uniform &&
+              fn.getRegisterFamily(reg) != ir::FAMILY_QWORD &&
+              !insn.getParent()->definedPhiRegs.contains(reg) &&
+              opCode != ir::OP_ATOMIC &&
+              opCode != ir::OP_MUL_HI &&
+              opCode != ir::OP_HADD &&
+              opCode != ir::OP_RHADD &&
+              opCode != ir::OP_ADDSAT &&
+              (dstNum == 1 || insn.getOpcode() != ir::OP_LOAD) &&
+              !extentRegs->contains(reg)
+             )
+            fn.setRegisterUniform(reg, true);
+        }
+      });
+    });
+  }
+
   void Liveness::initBlock(const BasicBlock &bb) {
     GBE_ASSERT(liveness.contains(&bb) == false);
     BlockInfo *info = GBE_NEW(BlockInfo, bb);
@@ -64,11 +103,8 @@ namespace ir {
     const uint32_t srcNum = insn.getSrcNum();
     const uint32_t dstNum = insn.getDstNum();
     // First look for used before killed
-    bool uniform = true;
     for (uint32_t srcID = 0; srcID < srcNum; ++srcID) {
       const Register reg = insn.getSrc(srcID);
-      if (!fn.isUniformRegister(reg))
-        uniform = false;
       // Not killed -> it is really an upward use
       if (info.varKill.contains(reg) == false)
         info.upwardUsed.insert(reg);
@@ -76,19 +112,6 @@ namespace ir {
     // A destination is a killed value
     for (uint32_t dstID = 0; dstID < dstNum; ++dstID) {
       const Register reg = insn.getDst(dstID);
-      int opCode = insn.getOpcode();
-      // FIXME, ADDSAT and uniform vector should be supported.
-      if (uniform &&
-          fn.getRegisterFamily(reg) != ir::FAMILY_QWORD &&
-          !info.bb.definedPhiRegs.contains(reg) &&
-          opCode != ir::OP_ATOMIC &&
-          opCode != ir::OP_MUL_HI &&
-          opCode != ir::OP_HADD &&
-          opCode != ir::OP_RHADD &&
-          opCode != ir::OP_ADDSAT &&
-          (dstNum == 1 || insn.getOpcode() != ir::OP_LOAD)
-         )
-        fn.setRegisterUniform(reg, true);
       info.varKill.insert(reg);
     }
   }
@@ -144,8 +167,9 @@ namespace ir {
   killed period, and the instructions before kill point were re-executed with different prediction,
   the inactive lanes of vreg maybe over-written. Then the out-of-loop use will got wrong data.
 */
-  void Liveness::computeExtraLiveInOut(void) {
+  void Liveness::computeExtraLiveInOut(set<Register> &extentRegs) {
     const vector<Loop *> &loops = fn.getLoops();
+    extentRegs.clear();
     if(loops.size() == 0) return;
 
     for (auto l : loops) {
@@ -163,7 +187,8 @@ namespace ir {
           std::set_intersection(exiting->liveOut.begin(), exiting->liveOut.end(), exit->upwardUsed.begin(), exit->upwardUsed.end(), std::back_inserter(toExtend));
         }
         if (toExtend.size() == 0) continue;
-
+        for(auto r : toExtend)
+          extentRegs.insert(r);
         for (auto bb : l->bbs) {
           BlockInfo * bI = liveness[&fn.getBlock(bb)];
           for(auto r : toExtend) {
diff --git a/backend/src/ir/liveness.hpp b/backend/src/ir/liveness.hpp
index 11f1fdc..d55e00d 100644
--- a/backend/src/ir/liveness.hpp
+++ b/backend/src/ir/liveness.hpp
@@ -127,7 +127,8 @@ namespace ir {
     void initInstruction(BlockInfo &info, const Instruction &insn);
     /*! Now really compute LiveOut based on UEVar and VarKill */
     void computeLiveInOut(void);
-    void computeExtraLiveInOut(void);
+    void computeExtraLiveInOut(set<Register> &extentRegs);
+    void analyzeUniform(set<Register> *extentRegs);
     /*! Set of work list block which has exit(return) instruction */
     typedef set <struct BlockInfo*> WorkSet;
     WorkSet workSet;
-- 
1.8.3.2