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