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

[Eduardo Lima Mitev]

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.

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
>