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 <el...@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 <el...@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@lists.freedesktop.org >>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>>> >>>> >>>> >>>> >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev