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

[Zebediah Figura (she/her)]

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?