[Beignet] [PATCH] GBE: Clear the value map when start a new scalarize pass.
Yang, Rong R
rong.r.yang at intel.com
Sun Jul 7 18:48:46 PDT 2013
It looks good to me.
Because I got a wrong impression that llvm passes's execute order is scalarize -> DeadInstElimination -> scalarize -> DeadInstElimination -> GenWrite -> GenWrite, so need to hold all function's value map.
Atfter double check, the execute order is scalarize -> DeadInstElimination -> GenWrite -> scalarize -> DeadInstElimination -> GenWrite, so this patch is ok.
-----Original Message-----
From: beignet-bounces+rong.r.yang=intel.com at lists.freedesktop.org [mailto:beignet-bounces+rong.r.yang=intel.com at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Thursday, July 04, 2013 7:22 PM
To: beignet at lists.freedesktop.org
Cc: Zhigang Gong
Subject: [Beignet] [PATCH] GBE: Clear the value map when start a new scalarize pass.
The scalarize pass is a function pass, and the valueMap should be a per-function data rather than a per-unit data. The reason we put it in the unit data structure is that the scalarize pass is before the GenWriter pass thus there is no ir::Function exists.
As there may be multiple kernel functions in one unit, if we don't clear the valueMap each time running a new scalarize pass, the previous data may cause some unexpected behaviour. For example, the previous instructions have been already erased, then latter a new instruction in this function may be created in the same position of the erased instruction, then it breaks this valueMap. That's the root cause why we run the unit test several times and may encounter an assertion sometime.
This commit also modify the ir::unit layer implementation to remove the dependency of llvm from that layer. In general, we should not add llvm related code to the ir layer.
Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
---
backend/src/ir/unit.cpp | 14 --------------
backend/src/ir/unit.hpp | 20 +++++++-------------
backend/src/llvm/llvm_gen_backend.cpp | 7 ++++---
backend/src/llvm/llvm_scalarize.cpp | 2 +-
4 files changed, 12 insertions(+), 31 deletions(-)
diff --git a/backend/src/ir/unit.cpp b/backend/src/ir/unit.cpp index 01e1eb1..4aeffe9 100644
--- a/backend/src/ir/unit.cpp
+++ b/backend/src/ir/unit.cpp
@@ -21,12 +21,6 @@
* \file unit.cpp
* \author Benjamin Segovia <benjamin.segovia at intel.com>
*/
-#include "llvm/Config/config.h"
-#if LLVM_VERSION_MINOR <= 2
-#include "llvm/Instructions.h"
-#else
-#include "llvm/IR/Instructions.h"
-#endif /* LLVM_VERSION_MINOR <= 2 */
#include "ir/unit.hpp"
#include "ir/function.hpp"
@@ -59,14 +53,6 @@ namespace ir {
constantSet.append(data, name, size, alignment);
}
- void Unit::removeDeadValues()
- {
- for(auto &it : valueMap) {
- llvm::Instruction* I = llvm::dyn_cast<llvm::Instruction>(it.first.first); //fake value
- if((I == NULL) || (I->getParent() == NULL))
- valueMap.erase(it.first);
- }
- }
std::ostream &operator<< (std::ostream &out, const Unit &unit) {
unit.apply([&out] (const Function &fn) { out << fn << std::endl; });
return out;
diff --git a/backend/src/ir/unit.hpp b/backend/src/ir/unit.hpp index 1017f5f..9e3d66a 100644
--- a/backend/src/ir/unit.hpp
+++ b/backend/src/ir/unit.hpp
@@ -24,13 +24,6 @@
#ifndef __GBE_IR_UNIT_HPP__
#define __GBE_IR_UNIT_HPP__
-#include "llvm/Config/config.h"
-#if LLVM_VERSION_MINOR <= 2
-#include "llvm/Value.h"
-#else
-#include "llvm/IR/Value.h"
-#endif /* LLVM_VERSION_MINOR <= 2 */
-
#include "ir/constant.hpp"
#include "ir/register.hpp"
#include "sys/hash_map.hpp"
@@ -49,7 +42,7 @@ namespace ir {
{
public:
typedef hash_map<std::string, Function*> FunctionSet;
- typedef std::pair<llvm::Value*, uint32_t> ValueIndex;
+ typedef std::pair<void*, uint32_t> ValueIndex;
/*! Create an empty unit */
Unit(PointerSize pointerSize = POINTER_32_BITS);
/*! Release everything (*including* the function pointers) */ @@ -84,8 +77,8 @@ namespace ir {
/*! Some values will not be allocated. For example a vector extract and
* a vector insertion when scalarize the vector load/store
*/
- void newValueProxy(llvm::Value *real,
- llvm::Value *fake,
+ void newValueProxy(void *real,
+ void *fake,
uint32_t realIndex = 0u,
uint32_t fakeIndex = 0u) {
const ValueIndex key(fake, fakeIndex); @@ -93,10 +86,11 @@ namespace ir {
GBE_ASSERT(valueMap.find(key) == valueMap.end()); // Do not insert twice
valueMap[key] = value;
}
- /* remove fake values that removed by other pass */
- void removeDeadValues(void);
+
+ void clearValueMap() { valueMap.clear(); }
+
/*! Return the value map */
- const map<ValueIndex, ValueIndex>& getValueMap(void) const { return valueMap; }
+ 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 diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
index 40f2667..36d8129 100644
--- a/backend/src/llvm/llvm_gen_backend.cpp
+++ b/backend/src/llvm/llvm_gen_backend.cpp
@@ -307,8 +307,10 @@ namespace gbe
}
/*! After scalarize pass, there are some valueMap in unit,
* use this function to copy from unit valueMap */
- void initValueMap(const map<ValueIndex, ValueIndex>& vMap) {
- valueMap.insert(vMap.begin(), vMap.end());
+ 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) { @@ -1207,7 +1209,6 @@ namespace gbe
}
ctx.startFunction(F.getName());
- unit.removeDeadValues();
this->regTranslator.clear();
this->regTranslator.initValueMap(unit.getValueMap());
this->labelMap.clear();
diff --git a/backend/src/llvm/llvm_scalarize.cpp b/backend/src/llvm/llvm_scalarize.cpp
index bab2236..41674b6 100644
--- a/backend/src/llvm/llvm_scalarize.cpp
+++ b/backend/src/llvm/llvm_scalarize.cpp
@@ -773,7 +773,7 @@ namespace gbe {
intTy = IntegerType::get(module->getContext(), 32);
floatTy = Type::getFloatTy(module->getContext());
builder = new IRBuilder<>(module->getContext());
- unit.removeDeadValues();
+ unit.clearValueMap();
scalarizeArgs(F);
typedef ReversePostOrderTraversal<Function*> RPOTType;
--
1.7.9.5
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list