[Beignet] [PATCH] GBE: fix a uniform analysis bug.
Yang, Rong R
rong.r.yang at intel.com
Thu May 22 22:59:30 PDT 2014
LGTM, thanks.
-----Original Message-----
From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Friday, May 23, 2014 9:40 AM
To: beignet at lists.freedesktop.org
Cc: Zhigang Gong
Subject: [Beignet] [PATCH] GBE: fix a uniform analysis bug.
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
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list