[v2 PATCH vkd3d 1/4] tests: Fix shader tests crashes on WARP driver.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 tests/d3d12_test_utils.h    | 13 +++++++++++++
 tests/shader_runner_d3d12.c |  7 ++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/d3d12_test_utils.h b/tests/d3d12_test_utils.h
index 5d900f8..f2ce922 100644
--- a/tests/d3d12_test_utils.h
+++ b/tests/d3d12_test_utils.h
@@ -956,6 +956,8 @@ struct test_context
     ID3D12CommandQueue *queue;
     ID3D12CommandAllocator *allocator;
     ID3D12GraphicsCommandList *list;
+    ID3D12PipelineState **pso;
+    size_t pso_count, pso_capacity;
 
     D3D12_RESOURCE_DESC render_target_desc;
     ID3D12Resource *render_target;
@@ -1074,6 +1076,15 @@ static inline bool init_test_context_(unsigned int line, struct test_context *co
     return true;
 }
 
+static inline void destroy_pipeline_state_objects(struct test_context *context)
+{
+    size_t i;
+
+    for (i = 0; i < context->pso_count; ++i)
+        ID3D12PipelineState_Release(context->pso[i]);
+    context->pso_count = 0;
+}
+
 #define destroy_test_context(context) destroy_test_context_(__LINE__, context)
 static inline void destroy_test_context_(unsigned int line, struct test_context *context)
 {
@@ -1092,6 +1103,8 @@ static inline void destroy_test_context_(unsigned int line, struct test_context
     ID3D12CommandAllocator_Release(context->allocator);
     ID3D12CommandQueue_Release(context->queue);
     ID3D12GraphicsCommandList_Release(context->list);
+    destroy_pipeline_state_objects(context);
+    free(context->pso);
 
     refcount = ID3D12Device_Release(context->device);
     ok_(line)(!refcount, "ID3D12Device has %u references left.\n", (unsigned int)refcount);
diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c
index d404895..47f419b 100644
--- a/tests/shader_runner_d3d12.c
+++ b/tests/shader_runner_d3d12.c
@@ -419,10 +419,12 @@ static void parse_test_directive(struct shader_context *context, const char *lin
         hr = create_root_signature(context->c.device, &root_signature_desc, &context->c.root_signature);
         ok(hr == S_OK, "Failed to create root signature, hr %#x.\n", hr);
 
-        pso = create_pipeline_state(context->c.device, context->c.root_signature,
-                context->c.render_target_desc.Format, NULL, &ps, NULL);
+        pso = create_pipeline_state(context->c.device, context->c.root_signature, context->c.render_target_desc.Format,
+                NULL, &ps, NULL);
         if (!pso)
             return;
+        vkd3d_array_reserve((void **)&context->c.pso, &context->c.pso_capacity, context->c.pso_count + 1, sizeof(*context->c.pso));
+        context->c.pso[context->c.pso_count++] = pso;
 
         ID3D12GraphicsCommandList_SetGraphicsRootSignature(command_list, context->c.root_signature);
         if (context->uniform_count)
@@ -439,7 +441,6 @@ static void parse_test_directive(struct shader_context *context, const char *lin
         ID3D12GraphicsCommandList_ClearRenderTargetView(command_list, context->c.rtv, clear_color, 0, NULL);
         ID3D12GraphicsCommandList_SetPipelineState(command_list, pso);
         ID3D12GraphicsCommandList_DrawInstanced(command_list, 3, 1, 0, 0);
-        ID3D12PipelineState_Release(pso);
         transition_resource_state(command_list, context->c.render_target,
                 D3D12_RESOURCE_STATE_RENDER_TARGET, D3D12_RESOURCE_STATE_COPY_SOURCE);
     }
-- 
2.34.1

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.y    | 15 ++++++++++++++-
 tests/round.shader_test     | 13 +++++++++++++
 tests/shader_runner_d3d12.c | 13 +++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 636882c..61998e0 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1570,6 +1570,18 @@ static const struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *c
     return args.decl;
 }
 
+static struct hlsl_ir_node *intrinsic_float_convert_arg(struct hlsl_ctx *ctx,
+        const struct parse_initializer *params, struct hlsl_ir_node *arg, const struct vkd3d_shader_location *loc)
+{
+    struct hlsl_type *type = arg->data_type;
+
+    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type == HLSL_TYPE_HALF)
+        return arg;
+
+    type = hlsl_get_numeric_type(ctx, type->type, HLSL_TYPE_FLOAT, type->dimx, type->dimy);
+    return add_implicit_conversion(ctx, params->instrs, arg, type, loc);
+}
+
 static bool intrinsic_abs(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
@@ -1675,7 +1687,8 @@ static bool intrinsic_pow(struct hlsl_ctx *ctx,
 static bool intrinsic_round(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
-    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ROUND, params->args[0], loc);
+    struct hlsl_ir_node *arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc);
+    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ROUND, arg, loc);
 }
 
 static bool intrinsic_saturate(struct hlsl_ctx *ctx,
diff --git a/tests/round.shader_test b/tests/round.shader_test
index d8eb11b..cc5a697 100644
--- a/tests/round.shader_test
+++ b/tests/round.shader_test
@@ -24,3 +24,16 @@ float4 main(uniform float4 u) : sv_target
 uniform 0 float4 -0.5 6.5 7.5 3.4
 draw quad
 probe all rgba (6.0, 8.0, 0.0, 3.0) 4
+
+
+
+[pixel shader]
+float4 main(uniform int4 u) : sv_target
+{
+    return round(u);
+}
+
+[test]
+uniform 0 int4 -1 0 2 10
+draw quad
+probe all rgba (-1.0, 0.0, 2.0, 10.0) 4
diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c
index 47f419b..c5992aa 100644
--- a/tests/shader_runner_d3d12.c
+++ b/tests/shader_runner_d3d12.c
@@ -552,6 +552,19 @@ static void parse_test_directive(struct shader_context *context, const char *lin
             }
             memcpy(context->uniforms + offset, &i, sizeof(i));
         }
+        if (match_string(line, "int4", &line))
+        {
+            int v[4];
+
+            if (sscanf(line, "%d %d %d %d", &v[0], &v[1], &v[2], &v[3]) < 4)
+                fatal_error("Malformed int4 constant '%s'.\n", line);
+            if (offset + 4 > context->uniform_count)
+            {
+                context->uniform_count = offset + 4;
+                context->uniforms = realloc(context->uniforms, context->uniform_count * sizeof(*context->uniforms));
+            }
+            memcpy(context->uniforms + offset, v, sizeof(v));
+        }
         else if (match_string(line, "uint", &line))
         {
             unsigned int u;
-- 
2.34.1

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Zebediah Figura (she/her)]

On 12/19/21 06:41, Nikolay Sivov wrote:
> +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct hlsl_ctx *ctx,
> +        const struct parse_initializer *params, struct hlsl_ir_node *arg, const struct vkd3d_shader_location *loc)
> +{
> +    struct hlsl_type *type = arg->data_type;
> +
> +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type == HLSL_TYPE_HALF)
> +        return arg;

add_implicit_conversion() already does this.
> +
> +    type = hlsl_get_numeric_type(ctx, type->type, HLSL_TYPE_FLOAT, type->dimx, type->dimy);
> +    return add_implicit_conversion(ctx, params->instrs, arg, type, loc);
> +}
> +

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Zebediah Figura (she/her)]

On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> On 12/19/21 06:41, Nikolay Sivov wrote:
> > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct 
> > hlsl_ctx *ctx,
> > +        const struct parse_initializer *params, struct hlsl_ir_node 
> > *arg, const struct vkd3d_shader_location *loc)
> > +{
> > +    struct hlsl_type *type = arg->data_type;
> > +
> > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type == 
> > HLSL_TYPE_HALF)
> > +        return arg;
>
> add_implicit_conversion() already does this.

Sorry, turns out I'm wrong; this preserves the input type for half, 
whereas add_implicit_conversion() alone would not.
> > +
> > +    type = hlsl_get_numeric_type(ctx, type->type, HLSL_TYPE_FLOAT, 
> > type->dimx, type->dimy);
> > +    return add_implicit_conversion(ctx, params->instrs, arg, type, loc);
> > +}
> > +

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Matteo Bruni]

On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
<zfigura(a)codeweavers.com> wrote:
> On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > hlsl_ctx *ctx,
> > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > *arg, const struct vkd3d_shader_location *loc)
> > > +{
> > > +    struct hlsl_type *type = arg->data_type;
> > > +
> > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > HLSL_TYPE_HALF)
> > > +        return arg;
> >
> > add_implicit_conversion() already does this.
>
> Sorry, turns out I'm wrong; this preserves the input type for half,
> whereas add_implicit_conversion() alone would not.

I wonder if that's what it should do in the first place?

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Nikolay Sivov]

On 1/19/22 15:03, Matteo Bruni wrote:
> On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> <zfigura(a)codeweavers.com> wrote:
> > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > hlsl_ctx *ctx,
> > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > *arg, const struct vkd3d_shader_location *loc)
> > > > +{
> > > > +    struct hlsl_type *type = arg->data_type;
> > > > +
> > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > HLSL_TYPE_HALF)
> > > > +        return arg;
> > > > add_implicit_conversion() already does this.
> > > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > > whereas add_implicit_conversion() alone would not.
> > > > I wonder if that's what it should do in the first place?

Does this actually block this series?

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Matteo Bruni]

On Wed, Jan 19, 2022 at 4:09 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
> On 1/19/22 15:03, Matteo Bruni wrote:
> > On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> > <zfigura(a)codeweavers.com> wrote:
> > > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > > hlsl_ctx *ctx,
> > > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > > *arg, const struct vkd3d_shader_location *loc)
> > > > > +{
> > > > > +    struct hlsl_type *type = arg->data_type;
> > > > > +
> > > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > > HLSL_TYPE_HALF)
> > > > > +        return arg;
> > > > > add_implicit_conversion() already does this.
> > > > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > > > whereas add_implicit_conversion() alone would not.
> > > > > I wonder if that's what it should do in the first place?
>
> Does this actually block this series?

No, I'll sign off on it in the next couple of days unless I notice
something else in the meantime.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Nikolay Sivov]

On 1/19/22 18:28, Matteo Bruni wrote:
> On Wed, Jan 19, 2022 at 4:09 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
> > On 1/19/22 15:03, Matteo Bruni wrote:
> > > On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> > > <zfigura(a)codeweavers.com> wrote:
> > > > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > > > hlsl_ctx *ctx,
> > > > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > > > *arg, const struct vkd3d_shader_location *loc)
> > > > > > +{
> > > > > > +    struct hlsl_type *type = arg->data_type;
> > > > > > +
> > > > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > > > HLSL_TYPE_HALF)
> > > > > > +        return arg;
> > > > > > add_implicit_conversion() already does this.
> > > > > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > > > > whereas add_implicit_conversion() alone would not.
> > > > > > I wonder if that's what it should do in the first place?
> >
> > Does this actually block this series?
>
> No, I'll sign off on it in the next couple of days unless I notice
> something else in the meantime.
> Ok, latest version on the list is v4, it has some error handling 
> changes, nothing functionally important.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Zebediah Figura (she/her)]

On 1/19/22 06:03, Matteo Bruni wrote:
> On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> <zfigura(a)codeweavers.com> wrote:
> > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > hlsl_ctx *ctx,
> > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > *arg, const struct vkd3d_shader_location *loc)
> > > > +{
> > > > +    struct hlsl_type *type = arg->data_type;
> > > > +
> > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > HLSL_TYPE_HALF)
> > > > +        return arg;
> > >
> > > add_implicit_conversion() already does this.
> >
> > Sorry, turns out I'm wrong; this preserves the input type for half,
> > whereas add_implicit_conversion() alone would not.
>
> I wonder if that's what it should do in the first place?

Yes, witness the following shader:

---

uint func(float x) { return 1; }
uint func(half x) { return 2; }

float4 main(void) : sv_target
{
     return float4(func(float(0)), func(half(0)), func(int(0)), 
func(uint(0)));
}

---

Native reduces this to (1, 2, 1, 1).

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Matteo Bruni]

On Thu, Jan 20, 2022 at 12:42 AM Zebediah Figura (she/her)
<zfigura(a)codeweavers.com> wrote:
> On 1/19/22 06:03, Matteo Bruni wrote:
> > On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> > <zfigura(a)codeweavers.com> wrote:
> > > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > > hlsl_ctx *ctx,
> > > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > > *arg, const struct vkd3d_shader_location *loc)
> > > > > +{
> > > > > +    struct hlsl_type *type = arg->data_type;
> > > > > +
> > > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > > HLSL_TYPE_HALF)
> > > > > +        return arg;
> > > >
> > > > add_implicit_conversion() already does this.
> > >
> > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > whereas add_implicit_conversion() alone would not.
> >
> > I wonder if that's what it should do in the first place?
>
> Yes, witness the following shader:
>
> ---
>
> uint func(float x) { return 1; }
> uint func(half x) { return 2; }
>
> float4 main(void) : sv_target
> {
>      return float4(func(float(0)), func(half(0)), func(int(0)),
> func(uint(0)));
> }
>
> ---
>
> Native reduces this to (1, 2, 1, 1).

Fun... But at least it means we can fix add_implicit_conversion() and
avoid that kind of workarounds, if I understand correctly.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Zebediah Figura (she/her)]

On 1/20/22 06:12, Matteo Bruni wrote:
> On Thu, Jan 20, 2022 at 12:42 AM Zebediah Figura (she/her)
> <zfigura(a)codeweavers.com> wrote:
> > On 1/19/22 06:03, Matteo Bruni wrote:
> > > On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> > > <zfigura(a)codeweavers.com> wrote:
> > > > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > > > hlsl_ctx *ctx,
> > > > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > > > *arg, const struct vkd3d_shader_location *loc)
> > > > > > +{
> > > > > > +    struct hlsl_type *type = arg->data_type;
> > > > > > +
> > > > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > > > HLSL_TYPE_HALF)
> > > > > > +        return arg;
> > > > >
> > > > > add_implicit_conversion() already does this.
> > > >
> > > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > > whereas add_implicit_conversion() alone would not.
> > >
> > > I wonder if that's what it should do in the first place?
> >
> > Yes, witness the following shader:
> >
> > ---
> >
> > uint func(float x) { return 1; }
> > uint func(half x) { return 2; }
> >
> > float4 main(void) : sv_target
> > {
> >       return float4(func(float(0)), func(half(0)), func(int(0)),
> > func(uint(0)));
> > }
> >
> > ---
> >
> > Native reduces this to (1, 2, 1, 1).
>
> Fun... But at least it means we can fix add_implicit_conversion() and
> avoid that kind of workarounds, if I understand correctly.

I... guess, but it seems janky. The main point of 
add_implicit_conversion() is for stuff like assignment RHS or return 
values. If we had add_implicit_conversion() pass through half it'd 
generate IR like

2:  half | 1.0
3:       | x = @2
4: float | x


(i.e. "x" is a float variable). Which... works, I guess? Maybe it's not 
that bad, since we already do a somewhat similar thing for scalar vs vec1.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Matteo Bruni]

On Thu, Jan 20, 2022 at 6:51 PM Zebediah Figura (she/her)
<zfigura(a)codeweavers.com> wrote:
> On 1/20/22 06:12, Matteo Bruni wrote:
> > On Thu, Jan 20, 2022 at 12:42 AM Zebediah Figura (she/her)
> > <zfigura(a)codeweavers.com> wrote:
> > > On 1/19/22 06:03, Matteo Bruni wrote:
> > > > On Mon, Dec 20, 2021 at 10:44 PM Zebediah Figura (she/her)
> > > > <zfigura(a)codeweavers.com> wrote:
> > > > > On 12/20/21 12:15, Zebediah Figura (she/her) wrote:
> > > > > > On 12/19/21 06:41, Nikolay Sivov wrote:
> > > > > > > +static struct hlsl_ir_node *intrinsic_float_convert_arg(struct
> > > > > > > hlsl_ctx *ctx,
> > > > > > > +        const struct parse_initializer *params, struct hlsl_ir_node
> > > > > > > *arg, const struct vkd3d_shader_location *loc)
> > > > > > > +{
> > > > > > > +    struct hlsl_type *type = arg->data_type;
> > > > > > > +
> > > > > > > +    if (type->base_type == HLSL_TYPE_FLOAT || type->base_type ==
> > > > > > > HLSL_TYPE_HALF)
> > > > > > > +        return arg;
> > > > > >
> > > > > > add_implicit_conversion() already does this.
> > > > >
> > > > > Sorry, turns out I'm wrong; this preserves the input type for half,
> > > > > whereas add_implicit_conversion() alone would not.
> > > >
> > > > I wonder if that's what it should do in the first place?
> > >
> > > Yes, witness the following shader:
> > >
> > > ---
> > >
> > > uint func(float x) { return 1; }
> > > uint func(half x) { return 2; }
> > >
> > > float4 main(void) : sv_target
> > > {
> > >       return float4(func(float(0)), func(half(0)), func(int(0)),
> > > func(uint(0)));
> > > }
> > >
> > > ---
> > >
> > > Native reduces this to (1, 2, 1, 1).
> >
> > Fun... But at least it means we can fix add_implicit_conversion() and
> > avoid that kind of workarounds, if I understand correctly.
>
> I... guess, but it seems janky. The main point of
> add_implicit_conversion() is for stuff like assignment RHS or return
> values. If we had add_implicit_conversion() pass through half it'd
> generate IR like
>
> 2:  half | 1.0
> 3:       | x = @2
> 4: float | x
>
> (i.e. "x" is a float variable). Which... works, I guess? Maybe it's not
> that bad, since we already do a somewhat similar thing for scalar vs vec1.

I think that's an improvement to the current state of things, right
now we're effectively losing information from the get-go. We can
always add a half-to-float lowering pass to simplify the IR.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Giovanni Mascellani]

Hi,

Il 20/01/22 19:20, Matteo Bruni ha scritto:
> > > > > I wonder if that's what it should do in the first place?
> > > > > Yes, witness the following shader:
> > > >
> > > > ---
> > > >
> > > > uint func(float x) { return 1; }
> > > > uint func(half x) { return 2; }
> > > >
> > > > float4 main(void) : sv_target
> > > > {
> > > >        return float4(func(float(0)), func(half(0)), func(int(0)),
> > > > func(uint(0)));
> > > > }
> > > >
> > > > ---
> > > >
> > > > Native reduces this to (1, 2, 1, 1).
> > > > Fun... But at least it means we can fix add_implicit_conversion() and
> > > > avoid that kind of workarounds, if I understand correctly.
> > > > I... guess, but it seems janky. The main point of
> > > > add_implicit_conversion() is for stuff like assignment RHS or return
> > > > values. If we had add_implicit_conversion() pass through half it'd
> > > > generate IR like
> >
> > 2:  half | 1.0
> > 3:       | x = @2
> > 4: float | x
> >
> > (i.e. "x" is a float variable). Which... works, I guess? Maybe it's not
> > that bad, since we already do a somewhat similar thing for scalar vs vec1.
> > I think that's an improvement to the current state of things, right
> > now we're effectively losing information from the get-go. We can
> > always add a half-to-float lowering pass to simplify the IR.

I don't really understand what Zeb's proposed fragment of code have to 
do with implicit conversions (that seems mostly a test about overload 
resolution), and I don't understand why and how implicit conversions 
should treat half in a special way.

Also, I don't think we should mix types in the IR without casts. If 
we're doing the same for vec1 vs scalar, I think we should rather fix 
that than introduce other mistypings.

Giovanni.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Zebediah Figura (she/her)]

On 1/21/22 05:04, Giovanni Mascellani wrote:
> Hi,
>
> Il 20/01/22 19:20, Matteo Bruni ha scritto:
> > > > > > I wonder if that's what it should do in the first place?
> > > > > > Yes, witness the following shader:
> > > > >
> > > > > ---
> > > > >
> > > > > uint func(float x) { return 1; }
> > > > > uint func(half x) { return 2; }
> > > > >
> > > > > float4 main(void) : sv_target
> > > > > {
> > > > >        return float4(func(float(0)), func(half(0)), func(int(0)),
> > > > > func(uint(0)));
> > > > > }
> > > > >
> > > > > ---
> > > > >
> > > > > Native reduces this to (1, 2, 1, 1).
> > > > > Fun... But at least it means we can fix add_implicit_conversion() and
> > > > > avoid that kind of workarounds, if I understand correctly.
> > > > > I... guess, but it seems janky. The main point of
> > > > > add_implicit_conversion() is for stuff like assignment RHS or return
> > > > > values. If we had add_implicit_conversion() pass through half it'd
> > > > > generate IR like
> > >
> > > 2:  half | 1.0
> > > 3:       | x = @2
> > > 4: float | x
> > >
> > > (i.e. "x" is a float variable). Which... works, I guess? Maybe it's not
> > > that bad, since we already do a somewhat similar thing for scalar vs 
> > > vec1.
> > > I think that's an improvement to the current state of things, right
> > > now we're effectively losing information from the get-go. We can
> > > always add a half-to-float lowering pass to simplify the IR.
>
> I don't really understand what Zeb's proposed fragment of code have to 
> do with implicit conversions (that seems mostly a test about overload 
> resolution), and I don't understand why and how implicit conversions 
> should treat half in a special way.

Overload resolution and implicit conversion are essentially two 
strategies for dealing with what that code fragment demonstrates.
> Also, I don't think we should mix types in the IR without casts. If 
> we're doing the same for vec1 vs scalar, I think we should rather fix 
> that than introduce other mistypings.

I don't think this is a desirable invariant to keep. It makes a lot of 
transformations harder for no reason (e.g. you can't "just" peephole two 
subsequent instructions anymore; you now have to deal with a possible 
identity conversion between them). By the time we get out of the parser 
we really shouldn't be doing type validation anyway.

[v2 PATCH vkd3d 2/4] vkd3d-shader/hlsl: Cast round() input to float.

[Giovanni Mascellani]

Hi,

On 19/12/21 13:41, Nikolay Sivov wrote:
> @@ -1675,7 +1687,8 @@ static bool intrinsic_pow(struct hlsl_ctx *ctx,
>   static bool intrinsic_round(struct hlsl_ctx *ctx,
>           const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
>   {
> -    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ROUND, params->args[0], loc);
> +    struct hlsl_ir_node *arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc);
> +    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_ROUND, arg, loc);
>   }

add_unary_arithmetic_expr() assume that the nodes it receives are not 
NULL, therefore you should check before passing.

Same thing for the following patches.

Giovanni.

[v2 PATCH vkd3d 3/4] vkd3d-shader/hlsl: Cast saturate() input to float.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.y   |  3 ++-
 tests/saturate.shader_test | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 61998e0..3d0f585 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1694,7 +1694,8 @@ static bool intrinsic_round(struct hlsl_ctx *ctx,
 static bool intrinsic_saturate(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
-    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_SAT, params->args[0], loc);
+    struct hlsl_ir_node *arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc);
+    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_SAT, arg, loc);
 }
 
 static const struct intrinsic_function
diff --git a/tests/saturate.shader_test b/tests/saturate.shader_test
index 0a954e7..e99df5b 100644
--- a/tests/saturate.shader_test
+++ b/tests/saturate.shader_test
@@ -8,3 +8,14 @@ float4 main(uniform float2 u) : sv_target
 uniform 0 float4 0.7 -0.1 0.0 0.0
 draw quad
 probe all rgba (0.7, 0.0, 1.0, 0.0)
+
+[pixel shader]
+float4 main(uniform int4 u) : sv_target
+{
+    return saturate(u);
+}
+
+[test]
+uniform 0 int4 -2 0 2 -1
+draw quad
+probe all rgba (0.0, 0.0, 1.0, 0.0)
-- 
2.34.1

[v2 PATCH vkd3d 4/4] vkd3d-shader/hlsl: Implement floor().

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 Makefile.am                  | 1 +
 libs/vkd3d-shader/hlsl.h     | 1 +
 libs/vkd3d-shader/hlsl.y     | 8 ++++++++
 libs/vkd3d-shader/hlsl_sm4.c | 4 ++++
 4 files changed, 14 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 20fee06..9c5a772 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -60,6 +60,7 @@ vkd3d_shader_tests = \
 	tests/cast-to-int.shader_test \
 	tests/cast-to-uint.shader_test \
 	tests/conditional.shader_test \
+	tests/floor.shader_test \
 	tests/hlsl-array-dimension.shader_test \
 	tests/hlsl-bool-cast.shader_test \
 	tests/hlsl-clamp.shader_test \
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 57acf3a..c935426 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -293,6 +293,7 @@ enum hlsl_ir_expr_op
     HLSL_OP1_DSX,
     HLSL_OP1_DSY,
     HLSL_OP1_EXP2,
+    HLSL_OP1_FLOOR,
     HLSL_OP1_FRACT,
     HLSL_OP1_LOG2,
     HLSL_OP1_LOGIC_NOT,
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 3d0f585..410399f 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1653,6 +1653,13 @@ static bool intrinsic_cross(struct hlsl_ctx *ctx,
     return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, &mul2->node, mul1_neg, loc);
 }
 
+static bool intrinsic_floor(struct hlsl_ctx *ctx,
+        const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
+{
+    struct hlsl_ir_node *arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc);
+    return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_FLOOR, arg, loc);
+}
+
 static bool intrinsic_max(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
@@ -1712,6 +1719,7 @@ intrinsic_functions[] =
     {"abs",                                 1, true,  intrinsic_abs},
     {"clamp",                               3, true,  intrinsic_clamp},
     {"cross",                               2, true,  intrinsic_cross},
+    {"floor",                               1, true,  intrinsic_floor},
     {"max",                                 2, true,  intrinsic_max},
     {"min",                                 2, true,  intrinsic_min},
     {"pow",                                 2, true,  intrinsic_pow},
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 2458018..2defc1f 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -1336,6 +1336,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
                     write_sm4_unary_op(buffer, VKD3D_SM4_OP_EXP, &expr->node, arg1, 0);
                     break;
 
+                case HLSL_OP1_FLOOR:
+                    write_sm4_unary_op(buffer, VKD3D_SM4_OP_ROUND_NI, &expr->node, arg1, 0);
+                    break;
+
                 case HLSL_OP1_LOG2:
                     write_sm4_unary_op(buffer, VKD3D_SM4_OP_LOG, &expr->node, arg1, 0);
                     break;
-- 
2.34.1

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>