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

[Zhigang Gong]

On Fri, Oct 31, 2014 at 02:19:54PM +0800, Zhenyu Wang wrote:
> 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.

With ppgtt's presence, I agree with you. Could you simply put a FIXME tag to
indicate that we have a potential NULL pointer issue?

Thanks,
Zhigang Gong.

> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827



> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet