[PATCH v5 vkd3d 12/13] vkd3d-shader/hlsl: Implement texture gather methods.

[Matteo Bruni]

On Wed, Jan 26, 2022 at 11:02 PM Matteo Bruni <matteo.mystral at gmail.com> wrote:
>
> On Wed, Jan 26, 2022 at 11:00 PM Zebediah Figura (she/her)
> <zfigura at codeweavers.com> wrote:
> >
> > On 1/26/22 15:48, Matteo Bruni wrote:
> > > On Wed, Jan 26, 2022 at 10:33 PM Zebediah Figura (she/her)
> > > <zfigura at codeweavers.com> wrote:
> > >>
> > >> On 1/26/22 08:52, Matteo Bruni wrote:
> > >>> On Wed, Jan 19, 2022 at 1:06 PM Matteo Bruni <matteo.mystral at gmail.com> wrote:
> > >>>>
> > >>>> On Wed, Dec 22, 2021 at 12:23 AM Zebediah Figura
> > >>>> <zfigura at codeweavers.com> wrote:
> > >>>>>
> > >>>>> From: Francisco Casas <fcasas at codeweavers.com>
> > >>>>>
> > >>>>> Signed-off-by: Francisco Casas <fcasas at codeweavers.com>
> > >>>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> > >>>>> ---
> > >>>>> v5: Strip newlines from hlsl_fixme(), get rid of the as-yet-unused
> > >>>>> status_out_arg variable, expand the srcs[] array to have size 4, use sm4
> > >>>>> register helpers, minor stylistic tweaks.
> > >>>>>
> > >>>>>    libs/vkd3d-shader/hlsl.c     |  4 ++
> > >>>>>    libs/vkd3d-shader/hlsl.h     |  4 ++
> > >>>>>    libs/vkd3d-shader/hlsl.y     | 99 ++++++++++++++++++++++++++++++++++++
> > >>>>>    libs/vkd3d-shader/hlsl_sm4.c | 57 ++++++++++++++++++++-
> > >>>>>    4 files changed, 163 insertions(+), 1 deletion(-)
> > >>>>>
> > >>>>> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> > >>>>> index 26175008d..856fb0f9d 100644
> > >>>>> --- a/libs/vkd3d-shader/hlsl.c
> > >>>>> +++ b/libs/vkd3d-shader/hlsl.c
> > >>>>> @@ -1280,6 +1280,10 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru
> > >>>>>        {
> > >>>>>            [HLSL_RESOURCE_LOAD] = "load_resource",
> > >>>>>            [HLSL_RESOURCE_SAMPLE] = "sample",
> > >>>>> +        [HLSL_RESOURCE_GATHER_RED] = "gather_red",
> > >>>>> +        [HLSL_RESOURCE_GATHER_GREEN] = "gather_green",
> > >>>>> +        [HLSL_RESOURCE_GATHER_BLUE] = "gather_blue",
> > >>>>> +        [HLSL_RESOURCE_GATHER_ALPHA] = "gather_alpha",
> > >>>>>        };
> > >>>>>
> > >>>>>        vkd3d_string_buffer_printf(buffer, "%s(resource = ", type_names[load->load_type]);
> > >>>>> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
> > >>>>> index 2396adb40..49fa8d9d3 100644
> > >>>>> --- a/libs/vkd3d-shader/hlsl.h
> > >>>>> +++ b/libs/vkd3d-shader/hlsl.h
> > >>>>> @@ -378,6 +378,10 @@ enum hlsl_resource_load_type
> > >>>>>    {
> > >>>>>        HLSL_RESOURCE_LOAD,
> > >>>>>        HLSL_RESOURCE_SAMPLE,
> > >>>>> +    HLSL_RESOURCE_GATHER_RED,
> > >>>>> +    HLSL_RESOURCE_GATHER_GREEN,
> > >>>>> +    HLSL_RESOURCE_GATHER_BLUE,
> > >>>>> +    HLSL_RESOURCE_GATHER_ALPHA,
> > >>>>>    };
> > >>>>>
> > >>>>>    struct hlsl_ir_resource_load
> > >>>>> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> > >>>>> index 9b1e5c071..460ba44bb 100644
> > >>>>> --- a/libs/vkd3d-shader/hlsl.y
> > >>>>> +++ b/libs/vkd3d-shader/hlsl.y
> > >>>>> @@ -1930,6 +1930,105 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl
> > >>>>>            list_add_tail(instrs, &load->node.entry);
> > >>>>>            return true;
> > >>>>>        }
> > >>>>> +    else if (!strcmp(name, "Gather") || !strcmp(name, "GatherRed") || !strcmp(name, "GatherBlue")
> > >>>>> +            || !strcmp(name, "GatherGreen") || !strcmp(name, "GatherAlpha"))
> > >>>>> +    {
> > >>>>> +        const unsigned int sampler_dim = sampler_dim_count(object_type->sampler_dim);
> > >>>>> +        enum hlsl_resource_load_type load_type;
> > >>>>> +        const struct hlsl_type *sampler_type;
> > >>>>> +        struct hlsl_ir_resource_load *load;
> > >>>>> +        struct hlsl_ir_node *offset = NULL;
> > >>>>> +        struct hlsl_ir_load *sampler_load;
> > >>>>> +        struct hlsl_type *result_type;
> > >>>>> +        struct hlsl_ir_node *coords;
> > >>>>> +        unsigned int read_channel;
> > >>>>> +
> > >>>>> +        if (!strcmp(name, "GatherGreen"))
> > >>>>> +        {
> > >>>
> > >>> At some point we probably want to introduce an enum for intrinsics and
> > >>> e.g. table-driven intrinsic_from_{function,method}() helpers to find
> > >>> out which intrinsic a call is supposed to match, if any. Repeatedly
> > >>> string-matching the function name like this is a bit ugly and it's
> > >>> only going to get worse as we introduce more intrinsics.
> > >>> The intrinsics currently in hlsl.y, intrinsic_functions[] sidestep the
> > >>> problem but that's only going to apply to those that can be inlined
> > >>> straight away.
> > >>
> > >> I agree that a table would help here. I'm not sure I understand the
> > >> concern about intrinsics that can't be inlined, though; can you please
> > >> elaborate?
> > >
> > > I'm referring to those intrinsics that shouldn't become expressions
> > > but stay as function calls, to make sure they aren't reordered or
> > > otherwise optimized (e.g. barriers). As we discussed earlier, they
> > > could also be represented by a separate instruction type, in which
> > > case they are sort of inlined anyway and, I guess, the existing
> > > intrinsic_functions[] table could be a good place for the
> > > string-to-enum conversion I'm proposing.
> >
> > That seems like an orthogonal problem; there's nothing about the way
> > intrinsic functions are currently handled that is tied to EXPR types.
> > I.e. there's no reason an handler from the intrinsic_functions table has
> > to emit an HLSL_IR_EXPR; it could just as easily emit HLSL_IR_INTRINSIC
> > or HLSL_IR_BARRIER or whatever it'd end up being called.
>
> Yeah. Just saying that that's what I meant with "intrinsics that can't
> be inlined".

To be clear, in my original comment I really don't care about the
inlining or not aspect aka representing "atomic" intrinsics as a
non-inlined call or in some other way. I guess I should have left the
final sentence out, it only added to the confusion.