[Beignet] [PATCH v2] GBE: fix a corner case which refer to an undefined value.
Zhigang Gong
zhigang.gong at intel.com
Tue Dec 17 22:47:59 PST 2013
Clang/llvm may generate some code similar to the following IRs:
... (there is no definition of %7)
br label 2
label1:
%10 = add %7, %6
...
ret
label2:
%7 = ...
br label1
The value %7 is assigned after label2 but is referred at label1.
>From the control flow, the IRs is valid. As the reference will
be executed after the assignment.
But our current virtual register allocation assume that all the
values are assigned before any usage from the instructions' order
view which is incorrect for this case.
This patch choose a simple way to fix this issue. It allocate
register when it fails to get one. And latter when emit the assignment
instruction, it just ignore the duplicate allocation.
v2: handle the case when %7 is a proxyValue.
Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
---
backend/src/llvm/llvm_gen_backend.cpp | 77 ++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 12 deletions(-)
diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
index 0d4fb87..2104a36 100644
--- a/backend/src/llvm/llvm_gen_backend.cpp
+++ b/backend/src/llvm/llvm_gen_backend.cpp
@@ -302,6 +302,8 @@ namespace gbe
void clear(void) {
valueMap.clear();
scalarMap.clear();
+ incomplRegMap.clear();
+ needPatchIncomplReg = false;
}
/*! Some values will not be allocated. For example, a bit-cast destination
* like: %fake = bitcast %real or a vector insertion since we do not have
@@ -315,6 +317,9 @@ namespace gbe
const ValueIndex value(real, realIndex);
GBE_ASSERT(valueMap.find(key) == valueMap.end()); // Do not insert twice
valueMap[key] = value;
+
+ if (scalarMap.find(std::make_pair(fake, fakeIndex)) != scalarMap.end())
+ needPatchIncomplReg = true;
}
/*! Mostly used for the preallocated registers (lids, gids) */
void newScalarProxy(ir::Register reg, Value *value, uint32_t index = 0u) {
@@ -323,7 +328,7 @@ namespace gbe
scalarMap[key] = reg;
}
/*! Allocate a new scalar register */
- ir::Register newScalar(Value *value, Value *key = NULL, uint32_t index = 0u)
+ ir::Register newScalar(Value *value, Value *key = NULL, uint32_t index = 0u, bool incomplete = false)
{
// we don't allow normal constant, but GlobalValue is a special case,
// it needs a register to store its address
@@ -336,7 +341,7 @@ namespace gbe
case Type::DoubleTyID:
case Type::PointerTyID:
GBE_ASSERT(index == 0);
- return this->newScalar(value, key, type, index);
+ return this->newScalar(value, key, type, index, incomplete);
break;
case Type::VectorTyID:
{
@@ -347,7 +352,7 @@ namespace gbe
elementTypeID != Type::FloatTyID &&
elementTypeID != Type::DoubleTyID)
GBE_ASSERTM(false, "Vectors of elements are not supported");
- return this->newScalar(value, key, elementType, index);
+ return this->newScalar(value, key, elementType, index, incomplete);
break;
}
default: NOT_SUPPORTED;
@@ -401,15 +406,36 @@ namespace gbe
CPV = extractConstantElem(CPV, index);
return (CPV && (isa<UndefValue>(CPV)));
}
+
+ void patchIncomplReg(ir::Function &fn);
private:
+ /*! Map value pair to ir::register for the incomplete register patch usage */
+ /* These register are referred before their assignment physically, */
+ /* we may allocate the registers at the reference time, but latter, if the
+ value is just a proxy value, we have to patch all the previous allocated
+ registers to the real registers pointed by the proxy value. */
+ struct ValuePair {
+ ValuePair(Value *_v, uint32_t _id) : v(_v), elemID(_id) { };
+ Value *v;
+ uint32_t elemID;
+ };
+ map<ir::Register, struct ValuePair> incomplRegMap;
+ bool needPatchIncomplReg;
+
/*! This creates a scalar register for a Value (index is the vector index when
* the value is a vector of scalars)
*/
- ir::Register newScalar(Value *value, Value *key, Type *type, uint32_t index) {
+ ir::Register newScalar(Value *value, Value *key, Type *type, uint32_t index, bool incomplete) {
const ir::RegisterFamily family = getFamily(ctx, type);
- const ir::Register reg = ctx.reg(family);
key = key == NULL ? value : key;
+ if (scalarMap.find(std::make_pair(key, index)) != scalarMap.end())
+ return scalarMap[std::make_pair(key, index)];
+ const ir::Register reg = ctx.reg(family);
this->insertRegister(reg, key, index);
+ if (incomplete) {
+ struct ValuePair vp(value, index);
+ incomplRegMap.insert(std::make_pair(reg, vp));
+ }
return reg;
}
/*! Map value to ir::Register */
@@ -500,7 +526,7 @@ namespace gbe
/*! Each block end may require to emit MOVs for further PHIs */
void emitMovForPHI(BasicBlock *curr, BasicBlock *succ);
/*! Alocate one or several registers (if vector) for the value */
- INLINE void newRegister(Value *value, Value *key = NULL);
+ INLINE void newRegister(Value *value, Value *key = NULL, bool incomplete = false);
/*! get the register for a llvm::Constant */
ir::Register getConstantRegister(Constant *c, uint32_t index = 0);
/*! Return a valid register from an operand (can use LOADI to make one) */
@@ -853,7 +879,7 @@ namespace gbe
return processConstant<ir::ImmediateIndex>(CPV, NewImmediateFunctor(ctx), index);
}
- void GenWriter::newRegister(Value *value, Value *key) {
+ void GenWriter::newRegister(Value *value, Value *key, bool incomplete) {
auto type = value->getType();
auto typeID = type->getTypeID();
switch (typeID) {
@@ -861,14 +887,15 @@ namespace gbe
case Type::FloatTyID:
case Type::DoubleTyID:
case Type::PointerTyID:
- regTranslator.newScalar(value, key);
+ regTranslator.newScalar(value, key, 0, incomplete);
break;
case Type::VectorTyID:
{
auto vectorType = cast<VectorType>(type);
const uint32_t elemNum = vectorType->getNumElements();
- for (uint32_t elemID = 0; elemID < elemNum; ++elemID)
- regTranslator.newScalar(value, key, elemID);
+ for (uint32_t elemID = 0; elemID < elemNum; ++elemID) {
+ regTranslator.newScalar(value, key, elemID, incomplete);
+ }
break;
}
default: NOT_SUPPORTED;
@@ -971,8 +998,11 @@ namespace gbe
if(isa<Constant>(value)) {
Constant *c = dyn_cast<Constant>(value);
return getConstantRegister(c, elemID);
- } else
+ } else {
+ if (!(regTranslator.valueExists(value, elemID)))
+ this->newRegister(value, NULL, true);
return regTranslator.getScalar(value, elemID);
+ }
}
INLINE Value *GenWriter::getPHICopy(Value *PHI) {
@@ -1293,6 +1323,28 @@ namespace gbe
});
}
+ void RegisterTranslator::patchIncomplReg(ir::Function &fn)
+ {
+ // Traverse all blocks and patch the incomplete registers.
+ if (!needPatchIncomplReg) return;
+ fn.foreachBlock([&](ir::BasicBlock &bb)
+ {
+ // Top bottom traversal patch incomplete registers.
+ bb.foreach([&](ir::Instruction &insn)
+ {
+ const uint32_t srcNum = insn.getSrcNum();
+ for (uint32_t srcID = 0; srcID < srcNum; ++srcID) {
+ const ir::Register src = insn.getSrc(srcID);
+ const auto &vp = incomplRegMap.find(src);
+ if (vp == incomplRegMap.end())
+ continue;
+ insn.setSrc(srcID, this->getScalar(vp->second.v, vp->second.elemID));
+ }
+ });
+ });
+ }
+
+
void GenWriter::removeLOADIs(const ir::Liveness &liveness, ir::Function &fn)
{
// We store the last write and last read for each register
@@ -1465,7 +1517,8 @@ namespace gbe
// Liveness can be shared when we optimized the immediates and the MOVs
const ir::Liveness liveness(fn);
-
+ // Patch the incomplete registers.
+ regTranslator.patchIncomplReg(fn);
if (OCL_OPTIMIZE_LOADI) this->removeLOADIs(liveness, fn);
if (OCL_OPTIMIZE_PHI_MOVES) this->removeMOVs(liveness, fn);
}
--
1.7.9.5
More information about the Beignet
mailing list