[Mesa-dev] [RFC 1/4] nir: Add a new intrinsic 'load_image_stride'

[Eduardo Lima Mitev]

On 1/28/19 10:16 AM, Eduardo Lima Mitev wrote:
> On 1/28/19 9:56 AM, Bas Nieuwenhuizen wrote:
>> On Mon, Jan 28, 2019 at 9:38 AM Eduardo Lima Mitev <elima at igalia.com> wrote:
>>>
>>> On 1/26/19 5:34 PM, Jason Ekstrand wrote:
>>>> Mind suffixing it with _ir3 or something since it's a back-end-specific
>>>> intrinsic?  Incidentally, this looks a lot like load_image_param_intel.
>>>>
>>>
>>> Yes, I felt inclined to add the suffix but wasn't sure how good/bad a
>>> practice it is, so I deferred it to review :).
>>>
>>> Now, I did a quick experiment and it turns out I can reuse
>>> image_deref_load_param_intel as-is. The semantics are a bit different so
>>> I would have to fork the comment to explain ir3 too.
>>>
>>> So, what about I remove the '_intel' suffix to that one and use if for
>>> this ir3?
>>
>> Instructions that have different semantics for different drivers sound
>> like a lot of potential for confusion and latent bugs to me. Is it
>> really a problem to have both?
>>
> 
> It is not a problem at all, but I think it is preferable not to
> duplicate this intrinsic if it serves the same purpose on both backends,
> and only slightly differ in the specific set of params it holds:
> 
> On both backends the intrinsic is used to represent registers at compile
> time, that will be resolved to uniforms at shader run time.
> 
> On intel, the params are offset, tiling, stride and swizzling; on ir3
> they are bytes-per-pixel, y-stride and z-stride. That's what I mean by
> different semantics, and I think the difference is not enough to justify
> forking it.
> 

Actually, one can argue there is no semantic difference at all from
NIR's point of view. The intrinsic just load certain image params on
both backends, and it is only backend-specific what those params are.

> I'm ok either way, though.
> 
> Eduardo
> 
>>>
>>> Thanks for the feedback and the tip!
>>>
>>> Eduardo
>>>
>>>> On January 25, 2019 07:48:54 Eduardo Lima Mitev <elima at igalia.com> wrote:
>>>>
>>>>> This is an internal intrinsic intended to be injected by a
>>>>> (freedreno-specific) 'lower_sampler_io' pass that will be introduced
>>>>> later in this series; and consumed by ir3_compiler_nir.
>>>>>
>>>>> The intrinsic will load in SSA values for various constants
>>>>> for images (image_dims), namely the format's bytes-per-pixel,
>>>>> y-stride and z-stride; for which the backend compiler will emit
>>>>> the corresponding uniforms.
>>>>>
>>>>> const_index[0] is the image index, and const_index[1] is the index
>>>>> into image_dims' register file for a given image: 0 for bpp, 1 to
>>>>> y-stride and 2 for z-stride.
>>>>> ---
>>>>> src/compiler/nir/nir_intrinsics.py | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/src/compiler/nir/nir_intrinsics.py
>>>>> b/src/compiler/nir/nir_intrinsics.py
>>>>> index a5cc3f7401c..169c835d746 100644
>>>>> --- a/src/compiler/nir/nir_intrinsics.py
>>>>> +++ b/src/compiler/nir/nir_intrinsics.py
>>>>> @@ -590,6 +590,8 @@ load("shared", 1, [BASE, ALIGN_MUL, ALIGN_OFFSET],
>>>>> [CAN_ELIMINATE])
>>>>> load("push_constant", 1, [BASE, RANGE], [CAN_ELIMINATE, CAN_REORDER])
>>>>> # src[] = { offset }. const_index[] = { base, range }
>>>>> load("constant", 1, [BASE, RANGE], [CAN_ELIMINATE, CAN_REORDER])
>>>>> +# src[] = { offset }. const_index[] = { image_idx, dim_idx }
>>>>> +load("image_stride", 1, [], [CAN_REORDER])
>>>>>
>>>>> # Stores work the same way as loads, except now the first source is
>>>>> the value
>>>>> # to store and the second (and possibly third) source specify where to
>>>>> store
>>>>> --
>>>>> 2.20.1
>>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>>
>>>>
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>