Please ignore this version too. I just sent out another patch to fix this problem better.
On Wed, Dec 18, 2013 at 02:47:59PM +0800, Zhigang Gong wrote: > 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 <[email protected]> > --- > 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 > > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
