[PATCH v2 0/1] MR234: vkd3d-shader/hlsl: Partially implement Load() for the Buffer object.

[Nikolay Sivov (＠nsivov)]

--
  v2: vkd3d-shader/hlsl: Partially implement Load() for the Buffer object.

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/234

[PATCH v2 1/1] vkd3d-shader/hlsl: Partially implement Load() for the Buffer object.

[Nikolay Sivov]

From: Nikolay Sivov

---
 libs/vkd3d-shader/hlsl.y | 45 +++++++++++++++++++++++++++++++++++++++-
 libs/vkd3d-shader/tpf.c  |  2 ++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 0e07fe57..f27d8748 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -3714,6 +3714,7 @@ static unsigned int hlsl_offset_dim_count(enum hlsl_sampler_dim dim)
             return 3;
         case HLSL_SAMPLER_DIM_CUBE:
         case HLSL_SAMPLER_DIM_CUBEARRAY:
+        case HLSL_SAMPLER_DIM_BUFFER:
             /* Offset parameters not supported for these types. */
             return 0;
         default:
@@ -3733,6 +3734,38 @@ static bool raise_invalid_method_object_type(struct hlsl_ctx *ctx, const struct
     return false;
 }

+static bool add_buffer_load_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object,
+        const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
+{
+    struct hlsl_resource_load_params load_params = {.type = HLSL_RESOURCE_LOAD};
+    const struct hlsl_type *object_type = object->data_type;
+    struct hlsl_ir_node *load;
+
+    if (params->args_count != 1 && params->args_count != 2)
+    {
+        hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
+                "Wrong number of arguments to method 'Load': expected 1 or 2, but got %u.", params->args_count);
+        return false;
+    }
+
+    if (params->args_count > 1)
+    {
+        hlsl_fixme(ctx, loc, "Tiled resource status argument.");
+    }
+
+    if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[0],
+            hlsl_get_scalar_type(ctx, HLSL_TYPE_INT), loc)))
+        return false;
+
+    load_params.format = object_type->e.resource_format;
+    load_params.resource = object;
+
+    if (!(load = hlsl_new_resource_load(ctx, &load_params, loc)))
+        return false;
+    list_add_tail(instrs, &load->entry);
+    return true;
+}
+
 static bool add_load_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object,
         const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
@@ -3743,6 +3776,12 @@ static bool add_load_method_call(struct hlsl_ctx *ctx, struct list *instrs, stru
     struct hlsl_ir_node *load;
     bool multisampled;

+    if (object_type->sampler_dim == HLSL_SAMPLER_DIM_BUFFER
+            || object_type->sampler_dim == HLSL_SAMPLER_DIM_STRUCTURED_BUFFER)
+    {
+        return add_buffer_load_method_call(ctx, instrs, object, name, params, loc);
+    }
+
     if (object_type->sampler_dim == HLSL_SAMPLER_DIM_CUBE
             || object_type->sampler_dim == HLSL_SAMPLER_DIM_CUBEARRAY)
     {
@@ -5098,7 +5137,11 @@ parameter:
         }

texture_type:
-      KW_TEXTURE1D
+      KW_BUFFER
+        {
+            $$ = HLSL_SAMPLER_DIM_BUFFER;
+        }
+    | KW_TEXTURE1D
         {
             $$ = HLSL_SAMPLER_DIM_1D;
         }
diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c
index d066b13e..5a7d218d 100644
--- a/libs/vkd3d-shader/tpf.c
+++ b/libs/vkd3d-shader/tpf.c
@@ -2817,6 +2817,8 @@ static D3D_SHADER_VARIABLE_TYPE sm4_base_type(const struct hlsl_type *type)
                     return D3D_SVT_TEXTURECUBE;
                 case HLSL_SAMPLER_DIM_GENERIC:
                     return D3D_SVT_TEXTURE;
+                case HLSL_SAMPLER_DIM_BUFFER:
+                    return D3D_SVT_BUFFER;
                 default:
                     vkd3d_unreachable();
             }

[Zebediah Figura (＠zfigura)]

On Tue Jun 13 11:34:09 2023 +0000, Giovanni Mascellani wrote:
> Of course I don't want this issue to become too predominant, given that
> it's just a matter of taste. I mentioned it mostly because at some point
> I thought my suggestions was somewhat aligned with Zeb's tastes too (but
> I could be wrong).
> My point, though, is not really about the `else` (which I still prefer
> to have, if the two branches are somewhat symmetrical), but rather the
> fact of "hand-inlining" the default helper. I.e., I consider this a
> little suboptimal:
> if (condition)
>     return f();
> return g();
>
> but I consider this significantly worse:
> if (condition)
>     return f();
> // inline g here
>
> (though, in the end, acceptable)

I don't think I can bring myself to care about this particular issue.

[Giovanni Mascellani (＠giomasce)]

On Thu Jun 15 18:48:40 2023 +0000, Zebediah Figura wrote:
> I don't think I can bring myself to care about this particular issue.

Ok, it seems I am in minority here. Let's ignore this, then.

[Francisco Casas (＠fcasas)]

Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
>          || object_type->sampler_dim == HLSL_SAMPLER_DIM_2DMSARRAY)
>  {
>      return raise_invalid_method_object_type(ctx, object_type, name, loc);
>  }

We are not excluding buffers here. Same of the other sample methods.

[Zebediah Figura (＠zfigura)]

On Wed Jun 28 22:58:05 2023 +0000, Francisco Casas wrote:
> We are not excluding buffers here. Same of the other sample methods.

This comment still needs to be addressed, I believe.