On Wed, Jan 26, 2022 at 11:02 PM Matteo Bruni matteo.mystral@gmail.com wrote:
On Wed, Jan 26, 2022 at 11:00 PM Zebediah Figura (she/her) zfigura@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@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@gmail.com wrote:
On Wed, Dec 22, 2021 at 12:23 AM Zebediah Figura zfigura@codeweavers.com wrote: > > From: Francisco Casas fcasas@codeweavers.com > > Signed-off-by: Francisco Casas fcasas@codeweavers.com > Signed-off-by: Zebediah Figura zfigura@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.