[PATCH 0/4] MR152: vkd3d-shader/hlsl: Fixes for sm1 semantics.

[PATCH 1/4] vkd3d-shader/hlsl: Sort only uniforms by name in the sm1 backend.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl_sm1.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index be32c8db..2cfdb1dc 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -302,7 +302,10 @@ static void sm1_sort_externs(struct hlsl_ctx *ctx)
     struct hlsl_ir_var *var, *next;

LIST_FOR_EACH_ENTRY_SAFE(var, next, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
-        sm1_sort_extern(&sorted, var);
+    {
+        if (var->is_uniform)
+            sm1_sort_extern(&sorted, var);
+    }
     list_move_tail(&ctx->extern_vars, &sorted);
 }

[PATCH 2/4] vkd3d-shader/hlsl: Map ps_1_* color and texcoord semantics to registers.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl_sm1.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index 2cfdb1dc..bd0d5b68 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -37,6 +37,9 @@ bool hlsl_sm1_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_sem
     }
     register_table[] =
     {
+        {"color",       false, VKD3D_SHADER_TYPE_PIXEL, 1, D3DSPR_INPUT},
+        {"texcoord",    false, VKD3D_SHADER_TYPE_PIXEL, 1, D3DSPR_TEXTURE},
+
         {"color",       true,  VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_COLOROUT},
         {"depth",       true,  VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_DEPTHOUT},
         {"sv_depth",    true,  VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_DEPTHOUT},

[PATCH 3/4] vkd3d-shader/hlsl: Do not write DCL instructions for ps_1_* shaders.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl_sm1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index bd0d5b68..4b4452fa 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -645,7 +645,7 @@ static void write_sm1_semantic_dcls(struct hlsl_ctx *ctx, struct vkd3d_bytecode_
     bool write_in = false, write_out = false;
     struct hlsl_ir_var *var;

-    if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL)
+    if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL && ctx->profile->major_version >= 2)
         write_in = true;
     else if (ctx->profile->type == VKD3D_SHADER_TYPE_VERTEX && ctx->profile->major_version == 3)
         write_in = write_out = true;

[PATCH 4/4] vkd3d-shader/hlsl: Allow VPOS and VFACE to be specified in sm3 pixel shaders.

[Zebediah Figura]

From: Zebediah Figura

We were previously (accidentally) rejecting them because they didn't have a
usage.
---
 libs/vkd3d-shader/hlsl_codegen.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 5d7e426f..7f79a0fd 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -2837,7 +2837,8 @@ static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var
         D3DDECLUSAGE usage;
         uint32_t usage_idx;

-        if (!hlsl_sm1_usage_from_semantic(&var->semantic, &usage, &usage_idx))
+        builtin = hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, &reg);
+        if (!builtin && !hlsl_sm1_usage_from_semantic(&var->semantic, &usage, &usage_idx))
         {
             hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC,
                     "Invalid semantic '%s'.", var->semantic.name);
@@ -2846,8 +2847,6 @@ static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var

if ((!output && !var->last_read) || (output && !var->first_write))
             return;
-
-        builtin = hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, &reg);
     }
     else
     {

[Giovanni Mascellani (＠giomasce)]

Approving this because nothing screams of being wrong and I'm not uncomfortable with anything in this MR, but my knowledge of the low level bits of SM1 is pretty weak, so I can't really give an independent confirmation that we want these changes. But I have a general trust that Zeb knows what she's doing! :-)

[PATCH 0/4] MR152: vkd3d-shader/hlsl: Fixes for sm1 semantics. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.

[Zebediah Figura (＠zfigura)]

> Approving this because nothing screams of being wrong and I'm not uncomfortable with anything in this MR, but my knowledge of the low level bits of SM1 is pretty weak, so I can't really give an independent confirmation that we want these changes. But I have a general trust that Zeb knows what she's doing! :-)

Sanity-checking the code changes is always appreciated even if you have to trust that the functional change is correct :-)

1/4 and 4/4 were both things I came across while trying to get sm1 tests to work, so in a sense we already have tests for those, we just can't quite run them yet.

2/4 and 3/4 are things related to ps_1_*, which I think we originally dismissed as not worth implementing, on the basis that native doesn't support emitting them and implicitly promotes them to ps_2_*. But native d3dx9_31 does support emitting ps_1_*, and perhaps more importantly, we want to be able to test that our sm1 -> glsl/spirv translator can correctly handle those versions.

[PATCH 0/4] MR152: vkd3d-shader/hlsl: Fixes for sm1 semantics. - approved

[Matteo Bruni (＠Mystral)]

This merge request was approved by Matteo Bruni.

[Matteo Bruni (＠Mystral)]

On Fri Apr 14 19:23:49 2023 +0000, Zebediah Figura wrote:
> > Approving this because nothing screams of being wrong and I'm not
>
> uncomfortable with anything in this MR, but my knowledge of the low
> level bits of SM1 is pretty weak, so I can't really give an independent
> confirmation that we want these changes. But I have a general trust that
> Zeb knows what she's doing! :-)
> Sanity-checking the code changes is always appreciated even if you have
> to trust that the functional change is correct :-)
> 1/4 and 4/4 were both things I came across while trying to get sm1 tests
> to work, so in a sense we already have tests for those, we just can't
> quite run them yet.
> 2/4 and 3/4 are things related to ps_1_*, which I think we originally
> dismissed as not worth implementing, on the basis that native doesn't
> support emitting them and implicitly promotes them to ps_2_*. But native
> d3dx9_31 does support emitting ps_1_*, and perhaps more importantly, we
> want to be able to test that our sm1 -> glsl/spirv translator can
> correctly handle those versions.

Basically yes, except it's not quite that it's not worth implementing ps_1_* but that a full implementation requires a TON of very specific compiler stuff that's going to get in the way of other compiler improvements.

This kind of small and localized fixes are certainly okay.

[Henri Verbeet (＠hverbeet)]

> 2/4 and 3/4 are things related to ps_1_*, which I think we originally dismissed as not worth implementing, on the basis that native doesn't support emitting them and implicitly promotes them to ps_2_*.

I've certainly heard that view before, but I don't quite agree. That particular fact may certainly make it a bit harder to justify spending much time on these older shader models in a more corporate environment, but I'll happily take the patches if someone writes them.

[PATCH 0/4] MR152: vkd3d-shader/hlsl: Fixes for sm1 semantics. - approved

[Henri Verbeet (＠hverbeet)]

This merge request was approved by Henri Verbeet.