[PATCH vkd3d 1/5] vkd3d-shader/hlsl: Write SM4 multiplication instructions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 libs/vkd3d-shader/hlsl_sm4.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 32a4eda1..af1660f1 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -957,6 +957,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
                     write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
                     break;

+                case HLSL_OP2_MUL:
+                    write_sm4_binary_op(buffer, VKD3D_SM4_OP_MUL, &expr->node, arg1, arg2);
+                    break;
+
                 default:
                     hlsl_fixme(ctx, expr->node.loc, "SM4 float "%s" expression.", debug_hlsl_expr_op(expr->op));
                     break;

[PATCH vkd3d 2/5] vkd3d-shader/hlsl: Write SM4 division instructions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 libs/vkd3d-shader/hlsl_sm4.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index af1660f1..ace2eb66 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -957,6 +957,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
                     write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
                     break;

+                case HLSL_OP2_DIV:
+                    write_sm4_binary_op(buffer, VKD3D_SM4_OP_DIV, &expr->node, arg1, arg2);
+                    break;
+
                 case HLSL_OP2_MUL:
                     write_sm4_binary_op(buffer, VKD3D_SM4_OP_MUL, &expr->node, arg1, arg2);
                     break;

[PATCH vkd3d 2/5] vkd3d-shader/hlsl: Write SM4 division instructions.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

Il 30/08/21 19:21, Zebediah Figura ha scritto:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl_sm4.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index af1660f1..ace2eb66 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -957,6 +957,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
>                       write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
>                       break;
> -
>            case HLSL_OP2_DIV:
> -
>                write_sm4_binary_op(buffer, VKD3D_SM4_OP_DIV, &expr->node, arg1, arg2);
> -
>                break;
> -
>             case HLSL_OP2_MUL:
>                 write_sm4_binary_op(buffer, VKD3D_SM4_OP_MUL, &expr->node, arg1, arg2);
>                 break;

[PATCH vkd3d 2/5] vkd3d-shader/hlsl: Write SM4 division instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 2/5] vkd3d-shader/hlsl: Write SM4 division instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 Makefile.am                  |  5 +----
 libs/vkd3d-shader/hlsl_sm4.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index c66f7b50..8c775c4d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -277,10 +277,7 @@ XFAIL_TESTS = \
    tests/hlsl-vector-indexing-uniform.shader_test \
    tests/math.shader_test \
    tests/trigonometry.shader_test \
-	tests/writemask-assignop-0.shader_test \
-	tests/writemask-assignop-1.shader_test \
-	tests/writemask-assignop-2.shader_test \
-	tests/writemask-assignop-3.shader_test
+	tests/writemask-assignop-1.shader_test
 endif

if BUILD_DEMOS
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index ace2eb66..160da47f 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -561,6 +561,7 @@ struct sm4_register
     unsigned int idx_count;
     enum vkd3d_sm4_dimension dim;
     uint32_t immconst_uint[4];
+    unsigned int mod;
 };

struct sm4_instruction
@@ -710,6 +711,8 @@ static uint32_t sm4_register_order(const struct sm4_register *reg)
     if (reg->type == VKD3D_SM4_RT_IMMCONST)
         order += reg->dim == VKD3D_SM4_DIMENSION_VEC4 ? 4 : 1;
     order += reg->idx_count;
+    if (reg->mod)
+        ++order;
     return order;
 }

@@ -743,8 +746,13 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st
         token = sm4_encode_register(&instr->srcs[i].reg);
         token |= sm4_swizzle_type(instr->srcs[i].reg.type) << VKD3D_SM4_SWIZZLE_TYPE_SHIFT;
         token |= instr->srcs[i].swizzle << VKD3D_SM4_SWIZZLE_SHIFT;
+        if (instr->srcs[i].reg.mod)
+            token |= VKD3D_SM4_REGISTER_MODIFIER;
         put_u32(buffer, token);

+        if (instr->srcs[i].reg.mod)
+            put_u32(buffer, instr->srcs[i].reg.mod);
+
         for (j = 0; j < instr->srcs[i].reg.idx_count; ++j)
             put_u32(buffer, instr->srcs[i].reg.idx[j]);

@@ -894,6 +902,26 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
     write_sm4_instruction(buffer, &instr);
 }

+static void write_sm4_unary_op(struct vkd3d_bytecode_buffer *buffer, enum vkd3d_sm4_opcode opcode,
+        const struct hlsl_ir_node *dst, const struct hlsl_ir_node *src, unsigned int src_mod)
+{
+    struct sm4_instruction instr;
+    unsigned int writemask;
+
+    memset(&instr, 0, sizeof(instr));
+    instr.opcode = opcode;
+
+    sm4_register_from_node(&instr.dst.reg, &instr.dst.writemask, dst);
+    instr.has_dst = 1;
+
+    sm4_register_from_node(&instr.srcs[0].reg, &writemask, src);
+    instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
+    instr.srcs[0].reg.mod = src_mod;
+    instr.src_count = 1;
+
+    write_sm4_instruction(buffer, &instr);
+}
+
 static void write_sm4_binary_op(struct vkd3d_bytecode_buffer *buffer, enum vkd3d_sm4_opcode opcode,
         const struct hlsl_ir_node *dst, const struct hlsl_ir_node *src1, const struct hlsl_ir_node *src2)
 {
@@ -953,6 +981,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
         {
             switch (expr->op)
             {
+                case HLSL_OP1_NEG:
+                    write_sm4_unary_op(buffer, VKD3D_SM4_OP_MOV, &expr->node, arg1, VKD3D_SM4_REGISTER_MODIFIER_NEGATE);
+                    break;
+
                 case HLSL_OP2_ADD:
                     write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
                     break;

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

Il 30/08/21 19:21, Zebediah Figura ha scritto:
> Signed-off-by: Zebediah Figura
>
> Makefile.am                  |  5 +----
>   libs/vkd3d-shader/hlsl_sm4.c | 32 ++++++++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index c66f7b50..8c775c4d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -277,10 +277,7 @@ XFAIL_TESTS = \
>    tests/hlsl-vector-indexing-uniform.shader_test \
>    tests/math.shader_test \
>    tests/trigonometry.shader_test \
> - tests/writemask-assignop-0.shader_test \
> - tests/writemask-assignop-1.shader_test \
> - tests/writemask-assignop-2.shader_test \
> - tests/writemask-assignop-3.shader_test
> -
>
> tests/writemask-assignop-1.shader_test
> endif
>
> if BUILD_DEMOS
>
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index ace2eb66..160da47f 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -561,6 +561,7 @@ struct sm4_register
>       unsigned int idx_count;
>       enum vkd3d_sm4_dimension dim;
>       uint32_t immconst_uint[4];
> -
>
> unsigned int mod;
> };
>
> struct sm4_instruction
>
> @@ -710,6 +711,8 @@ static uint32_t sm4_register_order(const struct sm4_register *reg)
>       if (reg->type == VKD3D_SM4_RT_IMMCONST)
>           order += reg->dim == VKD3D_SM4_DIMENSION_VEC4 ? 4 : 1;
>       order += reg->idx_count;
> - if (reg->mod)
> -
>    ++order;
> return order;
> }
>
> @@ -743,8 +746,13 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st
>           token = sm4_encode_register(&instr->srcs[i].reg);
>           token |= sm4_swizzle_type(instr->srcs[i].reg.type) << VKD3D_SM4_SWIZZLE_TYPE_SHIFT;
>           token |= instr->srcs[i].swizzle << VKD3D_SM4_SWIZZLE_SHIFT;
> -
>    if (instr->srcs[i].reg.mod)
> -
>        token |= VKD3D_SM4_REGISTER_MODIFIER;
>     put_u32(buffer, token);
>
> -
>    if (instr->srcs[i].reg.mod)
> -
>        put_u32(buffer, instr->srcs[i].reg.mod);
> -
>     for (j = 0; j < instr->srcs[i].reg.idx_count; ++j)
>         put_u32(buffer, instr->srcs[i].reg.idx[j]);
>
> @@ -894,6 +902,26 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
>       write_sm4_instruction(buffer, &instr);
>   }
>
> +static void write_sm4_unary_op(struct vkd3d_bytecode_buffer *buffer, enum vkd3d_sm4_opcode opcode,
> -
>    const struct hlsl_ir_node *dst, const struct hlsl_ir_node *src, unsigned int src_mod)
>
> +{
> - struct sm4_instruction instr;
> - unsigned int writemask;
> -
> - memset(&instr, 0, sizeof(instr));
> - instr.opcode = opcode;
> -
> - sm4_register_from_node(&instr.dst.reg, &instr.dst.writemask, dst);
> - instr.has_dst = 1;
> -
> - sm4_register_from_node(&instr.srcs[0].reg, &writemask, src);
> - instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
> - instr.srcs[0].reg.mod = src_mod;
> - instr.src_count = 1;
> -
> - write_sm4_instruction(buffer, &instr);
>
> +}
> - static void write_sm4_binary_op(struct vkd3d_bytecode_buffer *buffer, enum vkd3d_sm4_opcode opcode,
>         const struct hlsl_ir_node *dst, const struct hlsl_ir_node *src1, const struct hlsl_ir_node *src2)
> {
>
> @@ -953,6 +981,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
>           {
>               switch (expr->op)
>               {
> -
>            case HLSL_OP1_NEG:
> -
>                write_sm4_unary_op(buffer, VKD3D_SM4_OP_MOV, &expr->node, arg1, VKD3D_SM4_REGISTER_MODIFIER_NEGATE);
> -
>                break;
> -
>             case HLSL_OP2_ADD:
>                 write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
>                 break;

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Matteo Bruni]

On Mon, Aug 30, 2021 at 7:21 PM Zebediah Figura  wrote:
> Signed-off-by: Zebediah Figura
>
> Makefile.am                  |  5 +----
>  libs/vkd3d-shader/hlsl_sm4.c | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 4 deletions(-)
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index ace2eb66..160da47f 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -561,6 +561,7 @@ struct sm4_register
>      unsigned int idx_count;
>      enum vkd3d_sm4_dimension dim;
>      uint32_t immconst_uint[4];
> - unsigned int mod;
>
> };
>
> struct sm4_instruction
> @@ -710,6 +711,8 @@ static uint32_t sm4_register_order(const struct sm4_register *reg)
>      if (reg->type == VKD3D_SM4_RT_IMMCONST)
>          order += reg->dim == VKD3D_SM4_DIMENSION_VEC4 ? 4 : 1;
>      order += reg->idx_count;
> - if (reg->mod)
> -
>    ++order;
> return order;
>
> }

Something that came to my mind at least twice while looking at that
function... Do you know e.g. by random looks to DXC's sources what the
size of a register in the bytecode is supposed to be called? "Order"
feels like a curious choice for a name.

No need to go investigate, just wondering if you happened to see anything.

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Zebediah Figura (she/her)]

On 8/31/21 6:33 PM, Matteo Bruni wrote:
> On Mon, Aug 30, 2021 at 7:21 PM Zebediah Figura  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > Makefile.am                  |  5 +----
> >   libs/vkd3d-shader/hlsl_sm4.c | 32 ++++++++++++++++++++++++++++++++
> >   2 files changed, 33 insertions(+), 4 deletions(-)
> > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > index ace2eb66..160da47f 100644
> > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > @@ -561,6 +561,7 @@ struct sm4_register
> >       unsigned int idx_count;
> >       enum vkd3d_sm4_dimension dim;
> >       uint32_t immconst_uint[4];
> > -
> >
> > unsigned int mod;
> > };
> >
> > struct sm4_instruction
> >
> > @@ -710,6 +711,8 @@ static uint32_t sm4_register_order(const struct sm4_register *reg)
> >       if (reg->type == VKD3D_SM4_RT_IMMCONST)
> >           order += reg->dim == VKD3D_SM4_DIMENSION_VEC4 ? 4 : 1;
> >       order += reg->idx_count;
> > - if (reg->mod)
> > -
> >    ++order;
> > return order;
> > }
>
> Something that came to my mind at least twice while looking at that
> function... Do you know e.g. by random looks to DXC's sources what the
> size of a register in the bytecode is supposed to be called? "Order"
> feels like a curious choice for a name.
>
> No need to go investigate, just wondering if you happened to see anything.

I recently discovered that the SM4 format is effectively documented in 
headers released with the DDK, namely "d3d10tokenizedprogramformat.hpp" 
and similar. Anyway, their version of SM4_REGISTER_ORDER_* is 
D3D10_SB_OPERAND_INDEX_DIMENSION_*.

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Matteo Bruni]

On Wed, Sep 1, 2021 at 3:24 AM Zebediah Figura (she/her)
 wrote:
> On 8/31/21 6:33 PM, Matteo Bruni wrote:
> > On Mon, Aug 30, 2021 at 7:21 PM Zebediah Figura  wrote:
> > > Signed-off-by: Zebediah Figura
> > >
> > > Makefile.am                  |  5 +----
> > >   libs/vkd3d-shader/hlsl_sm4.c | 32 ++++++++++++++++++++++++++++++++
> > >   2 files changed, 33 insertions(+), 4 deletions(-)
> > > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > > index ace2eb66..160da47f 100644
> > > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > > @@ -561,6 +561,7 @@ struct sm4_register
> > >       unsigned int idx_count;
> > >       enum vkd3d_sm4_dimension dim;
> > >       uint32_t immconst_uint[4];
> > > -
> > >
> > > unsigned int mod;
> > > };
> > >
> > > struct sm4_instruction
> > >
> > > @@ -710,6 +711,8 @@ static uint32_t sm4_register_order(const struct sm4_register *reg)
> > >       if (reg->type == VKD3D_SM4_RT_IMMCONST)
> > >           order += reg->dim == VKD3D_SM4_DIMENSION_VEC4 ? 4 : 1;
> > >       order += reg->idx_count;
> > > - if (reg->mod)
> > > -
> > >    ++order;
> > > return order;
> > > }
> >
> > Something that came to my mind at least twice while looking at that
> > function... Do you know e.g. by random looks to DXC's sources what the
> > size of a register in the bytecode is supposed to be called? "Order"
> > feels like a curious choice for a name.
> >
> > No need to go investigate, just wondering if you happened to see anything.
>
> I recently discovered that the SM4 format is effectively documented in
> headers released with the DDK, namely "d3d10tokenizedprogramformat.hpp"
> and similar. Anyway, their version of SM4_REGISTER_ORDER_* is
> D3D10_SB_OPERAND_INDEX_DIMENSION_*.

Oh, excellent! I'll have a look at some point.
No reason to change everything now, but going forward it's probably a
good idea to make more informed decisions so to use the same naming
principles as the original.

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Henri Verbeet]

On Wed, 1 Sept 2021 at 10:27, Matteo Bruni  wrote:
> On Wed, Sep 1, 2021 at 3:24 AM Zebediah Figura (she/her)
>  wrote:
> > I recently discovered that the SM4 format is effectively documented in
> > headers released with the DDK, namely "d3d10tokenizedprogramformat.hpp"
> > and similar. Anyway, their version of SM4_REGISTER_ORDER_* is
> > D3D10_SB_OPERAND_INDEX_DIMENSION_*.
>
> Oh, excellent! I'll have a look at some point.
> No reason to change everything now, but going forward it's probably a
> good idea to make more informed decisions so to use the same naming
> principles as the original.

This has been around for a long time. We have generally avoided
looking at the DDK, and I can't think of any cases where looking at it
would have helped much.

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Matteo Bruni]

On Wed, Sep 1, 2021 at 10:40 AM Henri Verbeet  wrote:
> On Wed, 1 Sept 2021 at 10:27, Matteo Bruni  wrote:
> > On Wed, Sep 1, 2021 at 3:24 AM Zebediah Figura (she/her)
> >  wrote:
> > > I recently discovered that the SM4 format is effectively documented in
> > > headers released with the DDK, namely "d3d10tokenizedprogramformat.hpp"
> > > and similar. Anyway, their version of SM4_REGISTER_ORDER_* is
> > > D3D10_SB_OPERAND_INDEX_DIMENSION_*.
> >
> > Oh, excellent! I'll have a look at some point.
> > No reason to change everything now, but going forward it's probably a
> > good idea to make more informed decisions so to use the same naming
> > principles as the original.
>
> This has been around for a long time. We have generally avoided
> looking at the DDK, and I can't think of any cases where looking at it
> would have helped much.

If it's dangerous then sure. I'm certain we don't NEED anything that
might be in there.

Otherwise, I don't see the problem with at least using the correct
names. It might just be that I recently discovered the effect
framework's "preshaders" are supposed to be called FXLVM and I liked
it a bit too much...

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Zebediah Figura (she/her)]

On 9/1/21 3:57 AM, Matteo Bruni wrote:
> On Wed, Sep 1, 2021 at 10:40 AM Henri Verbeet  wrote:
> > On Wed, 1 Sept 2021 at 10:27, Matteo Bruni  wrote:
> > > On Wed, Sep 1, 2021 at 3:24 AM Zebediah Figura (she/her)
> > >  wrote:
> > > > I recently discovered that the SM4 format is effectively documented in
> > > > headers released with the DDK, namely "d3d10tokenizedprogramformat.hpp"
> > > > and similar. Anyway, their version of SM4_REGISTER_ORDER_* is
> > > > D3D10_SB_OPERAND_INDEX_DIMENSION_*.
> > >
> > > Oh, excellent! I'll have a look at some point.
> > > No reason to change everything now, but going forward it's probably a
> > > good idea to make more informed decisions so to use the same naming
> > > principles as the original.
> >
> > This has been around for a long time. We have generally avoided
> > looking at the DDK, and I can't think of any cases where looking at it
> > would have helped much.
>
> If it's dangerous then sure. I'm certain we don't NEED anything that
> might be in there.
>
> Otherwise, I don't see the problem with at least using the correct
> names. It might just be that I recently discovered the effect
> framework's "preshaders" are supposed to be called FXLVM and I liked
> it a bit too much...

For what it's worth, the d3d12tokenizedprogramformat.hpp also from the 
DDK has the same definitions, and seems to be the same file as is 
shipped with the DXC source [1], and that at least is explicitly under 
an open license.

[1] 
https://github.com/microsoft/DirectXShaderCompiler/blob/cb485263b75495015e2a...

[PATCH vkd3d 3/5] vkd3d-shader/hlsl: Write SM4 negation instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 4/5] vkd3d-shader/hlsl: Merge the "type" and "base_type" rules.

[Zebediah Figura]

It's certainly invalid to declare a vector of non-scalars, but it doesn't need
to result in a syntax error. Indeed native d3dcompiler does not emit a syntax
error in this case.

Signed-off-by: Zebediah Figura 
---
 libs/vkd3d-shader/hlsl.y | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 7906356f..6a472c10 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1731,7 +1731,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t

%type <semantic> semantic

-%type <type> base_type
 %type <type> field_type
 %type <type> named_struct_spec
 %type <type> unnamed_struct_spec
@@ -2145,11 +2144,7 @@ input_mod:
         }

type:
-      base_type
-        {
-            $$ = $1;
-        }
-    | KW_VECTOR '<' base_type ',' C_INTEGER '>'
+      KW_VECTOR '<' type ',' C_INTEGER '>'
         {
             if ($3->type != HLSL_CLASS_SCALAR)
             {
@@ -2171,7 +2166,7 @@ type:

$$ = hlsl_new_type(ctx, NULL, HLSL_CLASS_VECTOR, $3->base_type, $5, 1);
         }
-    | KW_MATRIX '<' base_type ',' C_INTEGER ',' C_INTEGER '>'
+    | KW_MATRIX '<' type ',' C_INTEGER ',' C_INTEGER '>'
         {
             if ($3->type != HLSL_CLASS_SCALAR)
             {
@@ -2199,9 +2194,7 @@ type:

$$ = hlsl_new_type(ctx, NULL, HLSL_CLASS_MATRIX, $3->base_type, $7, $5);
         }
-
-base_type:
-      KW_VOID
+    | KW_VOID
         {
             $$ = ctx->builtin_types.Void;
         }

[PATCH vkd3d 4/5] vkd3d-shader/hlsl: Merge the "type" and "base_type" rules.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

Il 30/08/21 19:21, Zebediah Figura ha scritto:
> It's certainly invalid to declare a vector of non-scalars, but it doesn't need
> to result in a syntax error. Indeed native d3dcompiler does not emit a syntax
> error in this case.
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl.y | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 7906356f..6a472c10 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -1731,7 +1731,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
>
> %type <semantic> semantic
>
> -%type <type> base_type
>   %type <type> field_type
>   %type <type> named_struct_spec
>   %type <type> unnamed_struct_spec
> @@ -2145,11 +2144,7 @@ input_mod:
>           }
>
> type:
> -
>  base_type
> -
>    {
> -
>        $$ = $1;
> -
>    }
> - | KW_VECTOR '<' base_type ',' C_INTEGER '>'
> -
>  KW_VECTOR '<' type ',' C_INTEGER '>'
>     {
>         if ($3->type != HLSL_CLASS_SCALAR)
>         {
>
> @@ -2171,7 +2166,7 @@ type:
>           $$ = hlsl_new_type(ctx, NULL, HLSL_CLASS_VECTOR, $3->base_type, $5, 1);
>       }
> - | KW_MATRIX '<' base_type ',' C_INTEGER ',' C_INTEGER '>'
> - | KW_MATRIX '<' type ',' C_INTEGER ',' C_INTEGER '>'
>      {
>          if ($3->type != HLSL_CLASS_SCALAR)
>          {
>
> @@ -2199,9 +2194,7 @@ type:
>           $$ = hlsl_new_type(ctx, NULL, HLSL_CLASS_MATRIX, $3->base_type, $7, $5);
>       }
> -
>
> -base_type:
> -
>  KW_VOID
> - | KW_VOID
>      {
>          $$ = ctx->builtin_types.Void;
>      }

[PATCH vkd3d 4/5] vkd3d-shader/hlsl: Merge the "type" and "base_type" rules.

[Matteo Bruni]

Signed-off-by: Matteo Bruni 
---
I guess the original choice was mostly a "defensive" one: it's
generally better to start with a restrictive grammar and then remove
constraints as necessary rather than the other way around.

[PATCH vkd3d 4/5] vkd3d-shader/hlsl: Merge the "type" and "base_type" rules.

[Zebediah Figura (she/her)]

On 8/31/21 6:31 PM, Matteo Bruni wrote:
> Signed-off-by: Matteo Bruni
>
> I guess the original choice was mostly a "defensive" one: it's
> generally better to start with a restrictive grammar and then remove
> constraints as necessary rather than the other way around.

Sure. On the other hand, it's probably better to have less things result 
in a syntax error, so that you can emit more errors per run. But I don't 
mean to imply that the original code was unreasonable ;-)

[PATCH vkd3d 4/5] vkd3d-shader/hlsl: Merge the "type" and "base_type" rules.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 5/5] vkd3d-shader/hlsl: Use the modifiers location when validating modifiers on casts.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 libs/vkd3d-shader/hlsl.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 6a472c10..26ba4695 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2836,7 +2836,7 @@ unary_expr:

if ($2)
             {
-                hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
+                hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
                         "Modifiers are not allowed on casts.");
                 YYABORT;
             }

[PATCH vkd3d 5/5] vkd3d-shader/hlsl: Use the modifiers location when validating modifiers on casts.

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

Il 30/08/21 19:21, Zebediah Figura ha scritto:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl.y | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 6a472c10..26ba4695 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -2836,7 +2836,7 @@ unary_expr:
>           if ($2)
>           {
> -
>            hlsl_error(ctx, @3, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
> -
>            hlsl_error(ctx, @2, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER,
>                     "Modifiers are not allowed on casts.");
>             YYABORT;
>         }

[PATCH vkd3d 5/5] vkd3d-shader/hlsl: Use the modifiers location when validating modifiers on casts.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 5/5] vkd3d-shader/hlsl: Use the modifiers location when validating modifiers on casts.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[Giovanni Mascellani]

Signed-off-by: Giovanni Mascellani

Il 30/08/21 19:21, Zebediah Figura ha scritto:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl_sm4.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index 32a4eda1..af1660f1 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -957,6 +957,10 @@ static void write_sm4_expr(struct hlsl_ctx *ctx,
>                       write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
>                       break;
> -
>            case HLSL_OP2_MUL:
> -
>                write_sm4_binary_op(buffer, VKD3D_SM4_OP_MUL, &expr->node, arg1, arg2);
> -
>                break;
> -
>             default:
>                 hlsl_fixme(ctx, expr->node.loc, "SM4 float \"%s\" expression.", debug_hlsl_expr_op(expr->op));
>                 break;

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[Henri Verbeet]

Signed-off-by: Henri Verbeet