[Beignet] *** SPAM LEVEL 5.929 *** beignet: libEGL ABI abuse (getting at symbols not intended to be public)
He Junyan
junyan.he at inbox.com
Fri Jul 5 01:37:54 PDT 2013
Hi zhigang:
This patch seems have some problem to me.
The unit test will crash every time,
Program received signal SIGSEGV, Segmentation fault.
cl_gbm_set_image_extension (gbm=0x618640, display=0x0) at
/home/robinhe/CL/beignet/src/x11/gbm_dri2_x11_platform.c:99
99 struct _hack_dri2_egl_display *dri2_dpy = (struct
_hack_dri2_egl_display*)egl_dpy->DriverData;
(gdb) bt
#0 cl_gbm_set_image_extension (gbm=0x618640, display=0x0) at
/home/robinhe/CL/beignet/src/x11/gbm_dri2_x11_platform.c:99
#1 0x00007ffff6e34689 in intel_driver_open (intel=0x615e50,
props=0x7fffffffd0d0) at
/home/robinhe/CL/beignet/src/intel/intel_driver.c:216
#2 0x00007ffff6e34879 in cl_intel_driver_new (props=0x7fffffffd0d0) at
/home/robinhe/CL/beignet/src/intel/intel_driver.c:378
#3 0x00007ffff6e30cb0 in cl_context_new (props=0x7fffffffd0d0) at
/home/robinhe/CL/beignet/src/cl_context.c:145
#4 0x00007ffff6e30e92 in cl_create_context (properties=0x618600,
num_devices=<optimized out>, devices=0x7ffff7dd9d80,
pfn_notify=<optimized out>, user_data=<optimized out>,
errcode_ret=0x7fffffffd14c) at
/home/robinhe/CL/beignet/src/cl_context.c:116
#5 0x00007ffff6e299f1 in clCreateContext (properties=0x618600,
num_devices=1, devices=0x7ffff7dd9d80, pfn_notify=0,
user_data=<optimized out>, errcode_ret=0x7fffffffdab4)
at /home/robinhe/CL/beignet/src/cl_api.c:203
#6 0x00007ffff7bbd811 in cl_ocl_init () at
/home/robinhe/CL/beignet/utests/utest_helper.cpp:358
#7 0x0000000000401172 in main (argc=1, argv=0x7fffffffe438) at
/home/robinhe/CL/beignet/utests/utest_run.cpp:33
(gdb)
It seems the display struct may be changed between our mesa version.
On 07/05/2013 04:16 PM, Zhigang Gong wrote:
> 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
>
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20130705/b6fc145e/attachment.html>
More information about the Beignet
mailing list