[Mesa-dev] [PATCH 02/13] nir: add load_param

[Jason Ekstrand]

On Wed, Feb 28, 2018 at 1:25 PM, Rob Clark <robdclark at gmail.com> wrote:

> On Wed, Feb 28, 2018 at 4:16 PM, Eric Anholt <eric at anholt.net> wrote:
> > Rob Clark <robdclark at gmail.com> writes:
> >
> >> From: Karol Herbst <kherbst at redhat.com>
> >>
> >> OpenCL kernels have parameters (see pipe_grid_info::input), and so we
> >> need a way to access them.
> >>
> >> Signed-off-by: Rob Clark <robdclark at gmail.com>
> >>
> >> ---
> >>  src/compiler/nir/nir_intrinsics.h |  2 ++
> >>  src/compiler/nir/nir_lower_io.c   | 13 ++++++++++---
> >>  2 files changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/compiler/nir/nir_intrinsics.h b/src/compiler/nir/nir_
> intrinsics.h
> >> index ede29277876..0915c5e809f 100644
> >> --- a/src/compiler/nir/nir_intrinsics.h
> >> +++ b/src/compiler/nir/nir_intrinsics.h
> >> @@ -435,6 +435,8 @@ LOAD(ubo, 2, 0, xx, xx, xx,
> NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REOR
> >>  LOAD(input, 1, 2, BASE, COMPONENT, xx, NIR_INTRINSIC_CAN_ELIMINATE |
> NIR_INTRINSIC_CAN_REORDER)
> >>  /* src[] = { vertex, offset }. const_index[] = { base, component } */
> >>  LOAD(per_vertex_input, 2, 2, BASE, COMPONENT, xx,
> NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
> >> +/* src[] = { }. const_index[] = { base } */
> >> +LOAD(param, 0, 1, BASE, xx, xx, NIR_INTRINSIC_CAN_ELIMINATE |
> NIR_INTRINSIC_CAN_REORDER)
> >
> > I know this is a new request compared to the existing pattern, but could
> > you put a comment describing what load_param does in the code as well as
> > the commit message?  Especially what the meaning of the base is.  We've
> > been bad at this in NIR, and it makes learning how to write a new NIR
> > backend more challenging than it should be.
> >
> > Also, what makes these params different from UBOs or default uniforms?
>
> yeah, I guess makes sense to describe better in code.. but for now
> base is just the parameter number (ie. first param base=0, second
> param base=1, etc)
>
> For ir3, I end up uploading these as uniforms.. I'm not sure how nv
> does kernel params offhand.  Maybe if they just end up uniforms for
> everyone we can change clover to use ctx->set_constant_buffer() and
> use a pass to lower load_param to load_uniform.  At least that would
> solve one awkward thing about the pipe driver and clover agreeing on
> the layout of pipe_grid_info::input.
>

Disclaimer: I won't claim to be an OpenCL expert.  I had a chat with Curro
this morning and I think I have a decent handle on how it works now but I
may be missing something.

That said, I suspect that load/store_param is the wrong approach.  My
understanding is that each type of parameter: pointer, image, constant,
etc. has a corresponding binding type in GL that's a fairly close match:
SSBO, image, UBO, etc.  If this is true, then I think the better thing to
do would be to translate parameters in the main function into vtn_variables
of the appropriate type.  OpenCL can use a very simple binding model where
you index by the order they show up in the parameter list.  For globals,
I'm not sure how OpenCL gives you access to them but I would guess some
fairly straightforward binding concept can be used there as well.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180301/9ecf66e0/attachment-0001.html>