[Beignet] * SPAM LEVEL 5.929 * beignet: libEGL ABI abuse (getting at symbols not intended to be public)

Fri Jul 5 01:16:43 PDT 2013

Sorry, I just found I sent the wrong patch in my last email. Please
ignore that patch and use this one:
0001-CLGL-Refine-the-hack-of-gbm-extension-initialization.patch.

On Thu, Jul 04, 2013 at 08:26:09PM +0800, Zhigang Gong wrote:
> On Wed, Jul 03, 2013 at 01:35:08PM +0200, Simon Richter wrote:
> > Hi,
> > 
> > On 03.07.2013 12:01, Zhigang Gong wrote:
> > 
> > > The gbm device doesn't init lookup_image method and the user data. The lookup_image is only
> > > initialized when use the gbm device to create an egl display and then initialize the egl drm
> > > platform which is not our use model.
> > 
> > My plan for Beignet in Debian is to upload the releases to the regular
> > track where they can move into the next release, and direct git
> > snapshots at the experimental track (I've made an exception with 0.1,
> > because that version breaks installed OpenCL software with the error
> > returns from the query APIs), so ideally I'd like to have a solution
> > before that.
> > 
> > Can this be delegated to the Mesa project by means of a change request
> > in their BTS?
> 
> I will do that when I have time, may be within this month. Before that,
> I have a workaround patch to get the gl texture sharing work. Could you
> help to test it? It's still very hacky but avoid reference to the mesa's
> internal symbol directly.
> 
> > 
> > Current state:
> > 
> >  - Debian#712880 (autogenerated dependencies too weak) blocks Mesa 9
> > from propagating along the regular package track (unstable -> testing ->
> > stable). I expect that to be fixed soon.
> > 
> >  - Debian#712903 (dependency on a non-public symbol) blocks Beignet from
> > propagating within Debian
> > 
> >  - Debian#630344 (support for private symbols in package dependency
> > calculations) is looming above our heads. When implemented, this feature
> > will make builds of packages using private symbols from other packages fail.
> > 
> > What I can do is drop EGL support in the Debian packages, at least for
> > the regular track, which would allow us to be part of the release, but
> > at reduced functionality.
> > 
> >    Simon
> > 
> > Bug references:
> > 
> > http://bugs.debian.org/712880
> > http://bugs.debian.org/712903
> > http://buge.debian.org/630344
> > 
> 
> 
> 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> 

> From cacf883a0fed9e56beb943edb1f9e43ed87c928b Mon Sep 17 00:00:00 2001
> From: Zhigang Gong <zhigang.gong at linux.intel.com>
> Date: Thu, 4 Jul 2013 19:12:52 +0800
> Subject: [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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-CLGL-Refine-the-hack-of-gbm-extension-initialization.patch
Type: text/x-diff
Size: 5303 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20130705/5fc82511/attachment.patch>

[Beignet] *** SPAM LEVEL 5.929 *** beignet: libEGL ABI abuse (getting at symbols not intended to be public)

[Beignet] * SPAM LEVEL 5.929 * beignet: libEGL ABI abuse (getting at symbols not intended to be public)