[Beignet] [PATCH 3/5] Remove intel_gpgpu_check_binded_buf_address()

[Zhenyu Wang]

On 2014.10.27 17:53:51 +0800, Zhenyu Wang wrote:
> On 2014.10.27 12:57:59 +0800, Zhenyu Wang wrote:
> > On 2014.10.24 18:10:36 +0800, Zhigang Gong wrote:
> > > This assertion is just to make sure we will not get a NULL pointer
> > > for a normal buffer. The OpenCL spec doesn't give a very specific
> > > statement about a NULL buffer object. But it does allow to pass
> > > a NULL to a buffer object. Thus some one may implement the following
> > > kernel:
> > > 
> > > __kernel foo( global uint * input, global uint *output1, global uint *output2)
> > > {
> > >   ...
> > >   if (output1)
> > >     output1[get_global_id(0)] = result0;
> > >   if (output2)
> > >     output2[get_global_id(1)] = result1;
> > > }
> > > 
> > > If we pass in a NULL output1 which should be a normal allocated buffer,
> > > it breaks the above code.
> > 
> > Looks "runtime_null_kernel_arg" is a good one to test this.
> > 
> > > 
> > > Without PPGTT, this assertion works fine till now. If with PPGTT,
> > > we may hit this assertion, but we can't just simply remove this assertion.
> > 
> > Actually with aliasing PPGTT (which is default on for a long time),
> > that case can still possibly fail. You might hit it more easy when
> > boot into console mode without X.  The issue is that buffer offset is
> > zero doesn't mean buffer pointer is NULL.
> > 
> > > 
> > > Do you have good suggestion to always avoid allocate 0 offset for a
> > > valid buffer object?
> > > 
> > 
> > I'm not sure if kernel provides such capability or if it should, or cl implement
> > should take special value for NULL.
> > 
> 
> Current kernel doesn't expose any flag for exec buffers with bias
> offset, although it only cares for batch object internally with biased
> offset when binding. If you really think we need such flag and bias
> offset for buffer execution, we might ask kernel developer about that
> idea. Generally I think this should be handled by compiler with
> runtime check.
> 

Any idea or plan on how to fix this? I think the original patch itself
is right and should be checked in, otherwise it just makes possible random
failures for normal CL apps whose kernel doesn't check NULL ptrs at all.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20141031/a4c1c6cd/attachment.sig>