[Beignet] [PATCH 1/2] Remove newValueProxy from scalarize pass to genWriter pass.
Zhigang Gong
zhigang.gong at linux.intel.com
Thu Oct 17 20:17:23 PDT 2013
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 <rong.r.yang at intel.com>
> ---
> 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
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list