[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mesa3d-dev
Subject:    Re: [Mesa-dev] [PATCH 02/13] nir: add load_param
From:       Rob Clark <robdclark () gmail ! com>
Date:       2018-02-28 21:25:43
Message-ID: CAF6AEGu==rxTENSimj9d7T1O8GPDCKiWGS0DsNdA-TcNSQ-MLA () mail ! gmail ! com
[Download RAW message or body]

On Wed, Feb 28, 2018 at 4:16 PM, Eric Anholt <eric@anholt.net> wrote:
> Rob Clark <robdclark@gmail.com> writes:
> 
> > From: Karol Herbst <kherbst@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@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.

BR,
-R
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[prev in list] [next in list] [prev in thread] [next in thread]