This is really much cleaner than before. Thanks for the patch, will push it latter.
On Thu, Oct 17, 2013 at 01:36:55PM +0800, Yang Rong wrote: > If call newValueProxy in scalarize pass, the realValue maybe been deleted by > the following pass, cause assert. Move to genWriter pass, can fix this bug and > make code more clean. > > Signed-off-by: Yang Rong <[email protected]> > --- > backend/src/ir/unit.hpp | 20 ------------------- > backend/src/llvm/llvm_gen_backend.cpp | 36 > +++++++++++++++++++++++++---------- > backend/src/llvm/llvm_gen_backend.hpp | 2 +- > backend/src/llvm/llvm_scalarize.cpp | 18 +++--------------- > backend/src/llvm/llvm_to_gen.cpp | 2 +- > 5 files changed, 31 insertions(+), 47 deletions(-) > > diff --git a/backend/src/ir/unit.hpp b/backend/src/ir/unit.hpp > index 9e3d66a..d8eab79 100644 > --- a/backend/src/ir/unit.hpp > +++ b/backend/src/ir/unit.hpp > @@ -42,7 +42,6 @@ namespace ir { > { > public: > typedef hash_map<std::string, Function*> FunctionSet; > - typedef std::pair<void*, uint32_t> ValueIndex; > /*! Create an empty unit */ > Unit(PointerSize pointerSize = POINTER_32_BITS); > /*! Release everything (*including* the function pointers) */ > @@ -73,30 +72,11 @@ namespace ir { > ConstantSet& getConstantSet(void) { return constantSet; } > /*! Return the constant set */ > const ConstantSet& getConstantSet(void) const { return constantSet; } > - > - /*! Some values will not be allocated. For example a vector extract and > - * a vector insertion when scalarize the vector load/store > - */ > - void newValueProxy(void *real, > - void *fake, > - uint32_t realIndex = 0u, > - uint32_t fakeIndex = 0u) { > - const ValueIndex key(fake, fakeIndex); > - const ValueIndex value(real, realIndex); > - GBE_ASSERT(valueMap.find(key) == valueMap.end()); // Do not insert > twice > - valueMap[key] = value; > - } > - > - void clearValueMap() { valueMap.clear(); } > - > - /*! Return the value map */ > - const map<ValueIndex, ValueIndex> &getValueMap(void) const { return > valueMap; } > private: > friend class ContextInterface; //!< Can free modify the unit > hash_map<std::string, Function*> functions; //!< All the defined > functions > ConstantSet constantSet; //!< All the constants defined in the unit > PointerSize pointerSize; //!< Size shared by all pointers > - map<ValueIndex, ValueIndex> valueMap; //!< fake to real value map for > vector load/store > GBE_CLASS(Unit); > }; > > diff --git a/backend/src/llvm/llvm_gen_backend.cpp > b/backend/src/llvm/llvm_gen_backend.cpp > index 5fb4f49..d495f1a 100644 > --- a/backend/src/llvm/llvm_gen_backend.cpp > +++ b/backend/src/llvm/llvm_gen_backend.cpp > @@ -305,13 +305,6 @@ namespace gbe > GBE_ASSERT(valueMap.find(key) == valueMap.end()); // Do not insert > twice > valueMap[key] = value; > } > - /*! After scalarize pass, there are some valueMap in unit, > - * use this function to copy from unit valueMap */ > - void initValueMap(const map<ir::Unit::ValueIndex, ir::Unit::ValueIndex> > &vMap) { > - for(auto &it : vMap) > - newValueProxy((Value*)it.second.first, (Value*)it.first.first, > - it.second.second, it.first.second); > - } > /*! Mostly used for the preallocated registers (lids, gids) */ > void newScalarProxy(ir::Register reg, Value *value, uint32_t index = 0u) > { > const ValueIndex key(value, index); > @@ -1358,7 +1351,6 @@ namespace gbe > > ctx.startFunction(F.getName()); > this->regTranslator.clear(); > - this->regTranslator.initValueMap(unit.getValueMap()); > this->labelMap.clear(); > this->emitFunctionPrototype(F); > > @@ -1681,10 +1673,34 @@ namespace gbe > /*! Because there are still fake insert/extract instruction for > * load/store, so keep empty function here */ > void GenWriter::regAllocateInsertElement(InsertElementInst &I) {} > - void GenWriter::emitInsertElement(InsertElementInst &I) {} > + void GenWriter::emitInsertElement(InsertElementInst &I) { > + const VectorType *type = dyn_cast<VectorType>(I.getType()); > + GBE_ASSERT(type); > + const int elemNum = type->getNumElements(); > + > + Value *vec = I.getOperand(0); > + Value *value = I.getOperand(1); > + const Value *index = I.getOperand(2); > + const ConstantInt *c = dyn_cast<ConstantInt>(index); > + int i = c->getValue().getSExtValue(); > + > + for(int j=0; j<elemNum; j++) { > + if(i == j) > + regTranslator.newValueProxy(value, &I, 0, i); > + else > + regTranslator.newValueProxy(vec, &I, j, j); > + } > + } > > void GenWriter::regAllocateExtractElement(ExtractElementInst &I) {} > - void GenWriter::emitExtractElement(ExtractElementInst &I) {} > + void GenWriter::emitExtractElement(ExtractElementInst &I) { > + Value *vec = I.getVectorOperand(); > + const Value *index = I.getIndexOperand(); > + const ConstantInt *c = dyn_cast<ConstantInt>(index); > + GBE_ASSERT(c); > + int i = c->getValue().getSExtValue(); > + regTranslator.newValueProxy(vec, &I, i, 0); > + } > > void GenWriter::regAllocateShuffleVectorInst(ShuffleVectorInst &I) {} > void GenWriter::emitShuffleVectorInst(ShuffleVectorInst &I) {} > diff --git a/backend/src/llvm/llvm_gen_backend.hpp > b/backend/src/llvm/llvm_gen_backend.hpp > index 2ad879e..b8ad747 100644 > --- a/backend/src/llvm/llvm_gen_backend.hpp > +++ b/backend/src/llvm/llvm_gen_backend.hpp > @@ -81,7 +81,7 @@ namespace gbe > /*! Remove the GEP instructions */ > llvm::BasicBlockPass *createRemoveGEPPass(const ir::Unit &unit); > > - llvm::FunctionPass* createScalarizePass(ir::Unit &unit); > + llvm::FunctionPass* createScalarizePass(); > > } /* namespace gbe */ > > diff --git a/backend/src/llvm/llvm_scalarize.cpp > b/backend/src/llvm/llvm_scalarize.cpp > index 7a40616..74893bd 100644 > --- a/backend/src/llvm/llvm_scalarize.cpp > +++ b/backend/src/llvm/llvm_scalarize.cpp > @@ -92,7 +92,6 @@ > #include "llvm/Support/raw_ostream.h" > > #include "llvm/llvm_gen_backend.hpp" > -#include "ir/unit.hpp" > #include "sys/map.hpp" > > > @@ -126,7 +125,7 @@ namespace gbe { > // Standard pass stuff > static char ID; > > - Scalarize(ir::Unit& unit) : FunctionPass(ID), unit(unit) > + Scalarize() : FunctionPass(ID) > { > initializeLoopInfoPass(*PassRegistry::getPassRegistry()); > initializeDominatorTreePass(*PassRegistry::getPassRegistry()); > @@ -228,7 +227,6 @@ namespace gbe { > > Type* intTy; > Type* floatTy; > - ir::Unit &unit; > > std::vector<Instruction*> deadList; > > @@ -598,14 +596,11 @@ namespace gbe { > Value *cv = ConstantInt::get(intTy, i); > Value *EI = builder->CreateExtractElement(insn, cv); > vVals.setComponent(i, EI); > - //unit.fakeInsnMap[EI] = insn; > - unit.newValueProxy(insn, EI, i, 0); > } > } > > Value* Scalarize::InsertToVector(Value * insn, Value* vecValue) { > //VectorValues& vVals = vectorVals[writeValue]; > - //unit.vecValuesMap[call] = vectorVals[writeValue]; > > //add fake insert instructions to avoid removed > Value *II = NULL; > @@ -613,14 +608,8 @@ namespace gbe { > Value *vec = II ? II : UndefValue::get(vecValue->getType()); > Value *cv = ConstantInt::get(intTy, i); > II = builder->CreateInsertElement(vec, getComponent(i, vecValue), cv); > - //unit.vecValuesMap[insn].setComponent(i, getComponent(i, writeValue)); > - //unit.newValueProxy(getComponent(i, vecValue), vecValue, 0, i); > - //unit.fakeInsnMap[II] = insn; > } > > - for (int i = 0; i < GetComponentCount(vecValue); ++i) { > - unit.newValueProxy(getComponent(i, vecValue), II, 0, i); > - } > return II; > } > > @@ -772,7 +761,6 @@ namespace gbe { > intTy = IntegerType::get(module->getContext(), 32); > floatTy = Type::getFloatTy(module->getContext()); > builder = new IRBuilder<>(module->getContext()); > - unit.clearValueMap(); > > scalarizeArgs(F); > typedef ReversePostOrderTraversal<Function*> RPOTType; > @@ -844,9 +832,9 @@ namespace gbe { > { > return; > } > - FunctionPass* createScalarizePass(ir::Unit &unit) > + FunctionPass* createScalarizePass() > { > - return new Scalarize(unit); > + return new Scalarize(); > } > char Scalarize::ID = 0; > > diff --git a/backend/src/llvm/llvm_to_gen.cpp > b/backend/src/llvm/llvm_to_gen.cpp > index 788a3dd..111514f 100644 > --- a/backend/src/llvm/llvm_to_gen.cpp > +++ b/backend/src/llvm/llvm_to_gen.cpp > @@ -80,7 +80,7 @@ namespace gbe > // Print the code before further optimizations > if (OCL_OUTPUT_LLVM_BEFORE_EXTRA_PASS) > passes.add(createPrintModulePass(&*o)); > - passes.add(createScalarizePass(unit)); // Expand all vector ops > + passes.add(createScalarizePass()); // Expand all vector ops > passes.add(createScalarReplAggregatesPass()); // Break up allocas > passes.add(createRemoveGEPPass(unit)); > passes.add(createConstantPropagationPass()); > -- > 1.8.1.2 > > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
