[PATCH vkd3d 1/6] vkd3d-shader/hlsl: Avoid crashing when emitting a dereference with non-constant offset.

[Zebediah Figura]

Some places assert() that the register is allocated.

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

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index c2aeb7b0..27592015 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -1200,7 +1200,7 @@ struct hlsl_reg hlsl_reg_from_deref(const struct hlsl_deref *deref, const struct
     if (offset_node && offset_node->type != HLSL_IR_CONSTANT)
     {
         FIXME("Dereference with non-constant offset of type %s.\n", hlsl_node_type_to_string(offset_node->type));
-        return ret;
+        offset_node = NULL;
     }

ret = var->reg;

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Zebediah Figura]

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

diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 15927671..2e304c5a 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -591,6 +591,7 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
     {
         case VKD3D_SM4_RT_CONSTBUFFER:
         case VKD3D_SM4_RT_INPUT:
+        case VKD3D_SM4_RT_TEMP:
             return VKD3D_SM4_SWIZZLE_VEC4;

default:
@@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
     }
 }

+static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
+        unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
+{
+    const struct hlsl_ir_var *var = deref->var;
+
+    if (var->is_output_semantic)
+    {
+        bool has_idx;
+
+        if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
+        {
+            if (has_idx)
+            {
+                reg->idx[0] = var->semantic.index;
+                reg->idx_count = 1;
+            }
+
+            if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
+                reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
+            else
+                reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+            *writemask = (1u << data_type->dimx) - 1;
+        }
+        else
+        {
+            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
+
+            assert(hlsl_reg.allocated);
+            reg->type = VKD3D_SM4_RT_OUTPUT;
+            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+            reg->idx[0] = hlsl_reg.id;
+            reg->idx_count = 1;
+            *writemask = hlsl_reg.writemask;
+        }
+    }
+    else
+    {
+        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
+
+        assert(hlsl_reg.allocated);
+        reg->type = VKD3D_SM4_RT_TEMP;
+        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+        reg->idx[0] = hlsl_reg.id;
+        reg->idx_count = 1;
+        *writemask = hlsl_reg.writemask;
+    }
+}
+
+static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
+{
+    assert(instr->reg.allocated);
+    reg->type = VKD3D_SM4_RT_TEMP;
+    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+    reg->idx[0] = instr->reg.id;
+    reg->idx_count = 1;
+    *writemask = instr->reg.writemask;
+}
+
 static uint32_t sm4_encode_register(const struct sm4_register *reg)
 {
     return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)
@@ -785,11 +844,40 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
     write_sm4_instruction(buffer, &instr);
 }

-static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
+static void write_sm4_store(struct hlsl_ctx *ctx,
+        struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_store *store)
+{
+    const struct hlsl_ir_node *rhs = store->rhs.node;
+    struct sm4_instruction instr;
+    unsigned int writemask;
+
+    if (store->lhs.var->data_type->type == HLSL_CLASS_MATRIX)
+    {
+        hlsl_fixme(ctx, store->node.loc, "Store to a matrix variable.\n");
+        return;
+    }
+
+    memset(&instr, 0, sizeof(instr));
+    instr.opcode = VKD3D_SM4_OP_MOV;
+
+    sm4_register_from_deref(ctx, &instr.dst.reg, &writemask, &store->lhs, rhs->data_type);
+    instr.dst.writemask = hlsl_combine_writemasks(writemask, store->writemask);
+    instr.has_dst = 1;
+
+    sm4_register_from_node(&instr.srcs[0].reg, &writemask, rhs);
+    instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
+    instr.src_count = 1;
+
+    write_sm4_instruction(buffer, &instr);
+}
+
+static void write_sm4_shdr(struct hlsl_ctx *ctx,
+        const struct hlsl_ir_function_decl *entry_func, struct dxbc_writer *dxbc)
 {
     const struct hlsl_profile_info *profile = ctx->profile;
     struct vkd3d_bytecode_buffer buffer = {0};
     const struct hlsl_buffer *cbuffer;
+    const struct hlsl_ir_node *instr;
     const struct hlsl_ir_var *var;
     size_t token_count_position;

@@ -824,6 +912,30 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
     if (ctx->temp_count)
         write_sm4_dcl_temps(&buffer, ctx->temp_count);

+    LIST_FOR_EACH_ENTRY(instr, entry_func->body, struct hlsl_ir_node, entry)
+    {
+        if (instr->data_type)
+        {
+            if (instr->data_type->type == HLSL_CLASS_MATRIX)
+            {
+                FIXME("Matrix operations need to be lowered.\n");
+                break;
+            }
+
+            assert(instr->data_type->type == HLSL_CLASS_SCALAR || instr->data_type->type == HLSL_CLASS_VECTOR);
+        }
+
+        switch (instr->type)
+        {
+            case HLSL_IR_STORE:
+                write_sm4_store(ctx, &buffer, hlsl_ir_store(instr));
+                break;
+
+            default:
+                FIXME("Unhandled instruction type %s.\n", hlsl_node_type_to_string(instr->type));
+        }
+    }
+
     write_sm4_ret(&buffer);

set_u32(&buffer, token_count_position, bytecode_get_size(&buffer) / sizeof(uint32_t));
@@ -842,7 +954,7 @@ int hlsl_sm4_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
     write_sm4_signature(ctx, &dxbc, false);
     write_sm4_signature(ctx, &dxbc, true);
     write_sm4_rdef(ctx, &dxbc);
-    write_sm4_shdr(ctx, &dxbc);
+    write_sm4_shdr(ctx, entry_func, &dxbc);

if (!(ret = ctx->result))
         ret = dxbc_writer_write(&dxbc, out);

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Matteo Bruni]

On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura  wrote:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl_sm4.c | 116 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 114 insertions(+), 2 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index 15927671..2e304c5a 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
>      }
>  }
>
> +static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
> -
>    unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
>
> +{
> - const struct hlsl_ir_var *var = deref->var;
> -
> - if (var->is_output_semantic)
> - {
> -
>    bool has_idx;
> -
> -
>    if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
> -
>    {
> -
>        if (has_idx)
> -
>        {
> -
>            reg->idx[0] = var->semantic.index;
> -
>            reg->idx_count = 1;
> -
>        }
> -
> -
>        if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
> -
>            reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
> -
>        else
> -
>            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>        *writemask = (1u << data_type->dimx) - 1;
> -
>    }
> -
>    else
> -
>    {
> -
>        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> -
> -
>        assert(hlsl_reg.allocated);
> -
>        reg->type = VKD3D_SM4_RT_OUTPUT;
> -
>        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>        reg->idx[0] = hlsl_reg.id;
> -
>        reg->idx_count = 1;
> -
>        *writemask = hlsl_reg.writemask;
> -
>    }
> - }
> - else
> - {
> -
>    struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> -
> -
>    assert(hlsl_reg.allocated);
> -
>    reg->type = VKD3D_SM4_RT_TEMP;
> -
>    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>    reg->idx[0] = hlsl_reg.id;
> -
>    reg->idx_count = 1;
> -
>    *writemask = hlsl_reg.writemask;
> - }
>
> +}
> -
>
> +static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
> +{
> - assert(instr->reg.allocated);
> - reg->type = VKD3D_SM4_RT_TEMP;
> - reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> - reg->idx[0] = instr->reg.id;
> - reg->idx_count = 1;
> - *writemask = instr->reg.writemask;
>
> +}
> -
>
> static uint32_t sm4_encode_register(const struct sm4_register *reg)
>  {
>      return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)

I'm a bit annoyed by the fact that sm4_register_from_deref() here
takes into account only the subset of derefs / registers that matter
for the store instruction. OTOH this is "fixed" in the next patch and
there aren't clearly better alternatives, so I guess I'll bite the
bullet :)

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Zebediah Figura (she/her)]

On 8/26/21 10:47 AM, Matteo Bruni wrote:
> On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > libs/vkd3d-shader/hlsl_sm4.c | 116 ++++++++++++++++++++++++++++++++++-
> >   1 file changed, 114 insertions(+), 2 deletions(-)
> >
> > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > index 15927671..2e304c5a 100644
> > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > @@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
> >       }
> >   }
> >
> > +static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
> > -
> >    unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
> >
> > +{
> > - const struct hlsl_ir_var *var = deref->var;
> > -
> > - if (var->is_output_semantic)
> > - {
> > -
> >    bool has_idx;
> > -
> > -
> >    if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
> > -
> >    {
> > -
> >        if (has_idx)
> > -
> >        {
> > -
> >            reg->idx[0] = var->semantic.index;
> > -
> >            reg->idx_count = 1;
> > -
> >        }
> > -
> > -
> >        if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
> > -
> >            reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
> > -
> >        else
> > -
> >            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >        *writemask = (1u << data_type->dimx) - 1;
> > -
> >    }
> > -
> >    else
> > -
> >    {
> > -
> >        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > -
> > -
> >        assert(hlsl_reg.allocated);
> > -
> >        reg->type = VKD3D_SM4_RT_OUTPUT;
> > -
> >        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >        reg->idx[0] = hlsl_reg.id;
> > -
> >        reg->idx_count = 1;
> > -
> >        *writemask = hlsl_reg.writemask;
> > -
> >    }
> > - }
> > - else
> > - {
> > -
> >    struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > -
> > -
> >    assert(hlsl_reg.allocated);
> > -
> >    reg->type = VKD3D_SM4_RT_TEMP;
> > -
> >    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >    reg->idx[0] = hlsl_reg.id;
> > -
> >    reg->idx_count = 1;
> > -
> >    *writemask = hlsl_reg.writemask;
> > - }
> >
> > +}
> > -
> >
> > +static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
> > +{
> > - assert(instr->reg.allocated);
> > - reg->type = VKD3D_SM4_RT_TEMP;
> > - reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > - reg->idx[0] = instr->reg.id;
> > - reg->idx_count = 1;
> > - *writemask = instr->reg.writemask;
> >
> > +}
> > - static uint32_t sm4_encode_register(const struct sm4_register *reg)
> > {
> >     return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)
>
> I'm a bit annoyed by the fact that sm4_register_from_deref() here
> takes into account only the subset of derefs / registers that matter
> for the store instruction. OTOH this is "fixed" in the next patch and
> there aren't clearly better alternatives, so I guess I'll bite the
> bullet :)

I considered doing it the other way, but I didn't really want to add 
dead code. Probably the right thing to do was add a FIXME...

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Matteo Bruni]

On Thu, Aug 26, 2021 at 6:23 PM Zebediah Figura (she/her)
 wrote:
> On 8/26/21 10:47 AM, Matteo Bruni wrote:
> > On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura  wrote:
> > > Signed-off-by: Zebediah Figura
> > >
> > > libs/vkd3d-shader/hlsl_sm4.c | 116 ++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 114 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > > index 15927671..2e304c5a 100644
> > > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > > @@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
> > >       }
> > >   }
> > >
> > > +static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
> > > -
> > >    unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
> > >
> > > +{
> > > - const struct hlsl_ir_var *var = deref->var;
> > > -
> > > - if (var->is_output_semantic)
> > > - {
> > > -
> > >    bool has_idx;
> > > -
> > > -
> > >    if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
> > > -
> > >    {
> > > -
> > >        if (has_idx)
> > > -
> > >        {
> > > -
> > >            reg->idx[0] = var->semantic.index;
> > > -
> > >            reg->idx_count = 1;
> > > -
> > >        }
> > > -
> > > -
> > >        if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
> > > -
> > >            reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
> > > -
> > >        else
> > > -
> > >            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > > -
> > >        *writemask = (1u << data_type->dimx) - 1;
> > > -
> > >    }
> > > -
> > >    else
> > > -
> > >    {
> > > -
> > >        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > > -
> > > -
> > >        assert(hlsl_reg.allocated);
> > > -
> > >        reg->type = VKD3D_SM4_RT_OUTPUT;
> > > -
> > >        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > > -
> > >        reg->idx[0] = hlsl_reg.id;
> > > -
> > >        reg->idx_count = 1;
> > > -
> > >        *writemask = hlsl_reg.writemask;
> > > -
> > >    }
> > > - }
> > > - else
> > > - {
> > > -
> > >    struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > > -
> > > -
> > >    assert(hlsl_reg.allocated);
> > > -
> > >    reg->type = VKD3D_SM4_RT_TEMP;
> > > -
> > >    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > > -
> > >    reg->idx[0] = hlsl_reg.id;
> > > -
> > >    reg->idx_count = 1;
> > > -
> > >    *writemask = hlsl_reg.writemask;
> > > - }
> > >
> > > +}
> > > -
> > >
> > > +static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
> > > +{
> > > - assert(instr->reg.allocated);
> > > - reg->type = VKD3D_SM4_RT_TEMP;
> > > - reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > > - reg->idx[0] = instr->reg.id;
> > > - reg->idx_count = 1;
> > > - *writemask = instr->reg.writemask;
> > >
> > > +}
> > > - static uint32_t sm4_encode_register(const struct sm4_register *reg)
> > > {
> > >     return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)
> >
> > I'm a bit annoyed by the fact that sm4_register_from_deref() here
> > takes into account only the subset of derefs / registers that matter
> > for the store instruction. OTOH this is "fixed" in the next patch and
> > there aren't clearly better alternatives, so I guess I'll bite the
> > bullet :)
>
> I considered doing it the other way, but I didn't really want to add
> dead code. Probably the right thing to do was add a FIXME...

Eh, it's fine. Annoyed isn't quite the word anyway, more like mildly
surprised / confused, and it's all settled with a more thorough look.

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Giovanni Mascellani]

Hi,

Il 24/08/21 08:18, Zebediah Figura ha scritto:
> Signed-off-by: Zebediah Figura
>
> libs/vkd3d-shader/hlsl_sm4.c | 116 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 114 insertions(+), 2 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index 15927671..2e304c5a 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> @@ -591,6 +591,7 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
>       {
>           case VKD3D_SM4_RT_CONSTBUFFER:
>           case VKD3D_SM4_RT_INPUT:
> -
>    case VKD3D_SM4_RT_TEMP:
>         return VKD3D_SM4_SWIZZLE_VEC4;
>
>     default:
>
> @@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
>       }
>   }
>
> +static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
> -
>    unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
>
> +{
> - const struct hlsl_ir_var *var = deref->var;
> -
> - if (var->is_output_semantic)
> - {
> -
>    bool has_idx;
> -
> -
>    if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
> -
>    {
> -
>        if (has_idx)
> -
>        {
> -
>            reg->idx[0] = var->semantic.index;
> -
>            reg->idx_count = 1;
> -
>        }
> -
> -
>        if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
> -
>            reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
> -
>        else
> -
>            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>        *writemask = (1u << data_type->dimx) - 1;
> -
>    }
> -
>    else
> -
>    {
> -
>        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> -
> -
>        assert(hlsl_reg.allocated);
> -
>        reg->type = VKD3D_SM4_RT_OUTPUT;
> -
>        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>        reg->idx[0] = hlsl_reg.id;
> -
>        reg->idx_count = 1;
> -
>        *writemask = hlsl_reg.writemask;
> -
>    }
> - }
> - else
> - {
> -
>    struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> -
> -
>    assert(hlsl_reg.allocated);
> -
>    reg->type = VKD3D_SM4_RT_TEMP;
> -
>    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> -
>    reg->idx[0] = hlsl_reg.id;
> -
>    reg->idx_count = 1;
> -
>    *writemask = hlsl_reg.writemask;
> - }
>
> +}
> -
>
> +static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
> +{
> - assert(instr->reg.allocated);
> - reg->type = VKD3D_SM4_RT_TEMP;
> - reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> - reg->idx[0] = instr->reg.id;
> - reg->idx_count = 1;
> - *writemask = instr->reg.writemask;
>
> +}
> -

I think I would use "node" as variable name instead of "instr", so there 
is one less term equivalence to keep in the mind when reading the code. 
But it's not a big deal.
> static uint32_t sm4_encode_register(const struct sm4_register *reg)
>   {
>       return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)
> @@ -785,11 +844,40 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
>       write_sm4_instruction(buffer, &instr);
>   }
>
> -static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
> +static void write_sm4_store(struct hlsl_ctx *ctx,
> -
>    struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_store *store)
>
> +{
> - const struct hlsl_ir_node *rhs = store->rhs.node;
> - struct sm4_instruction instr;
> - unsigned int writemask;
> -
> - if (store->lhs.var->data_type->type == HLSL_CLASS_MATRIX)
> - {
> -
>    hlsl_fixme(ctx, store->node.loc, "Store to a matrix variable.\n");
> -
>    return;
> - }
> -
> - memset(&instr, 0, sizeof(instr));
> - instr.opcode = VKD3D_SM4_OP_MOV;
> -
> - sm4_register_from_deref(ctx, &instr.dst.reg, &writemask, &store->lhs, rhs->data_type);
> - instr.dst.writemask = hlsl_combine_writemasks(writemask, store->writemask);
> - instr.has_dst = 1;
> -
> - sm4_register_from_node(&instr.srcs[0].reg, &writemask, rhs);
> - instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
> - instr.src_count = 1;
> -
> - write_sm4_instruction(buffer, &instr);
>
> +}
> -
>
> +static void write_sm4_shdr(struct hlsl_ctx *ctx,
> -
>    const struct hlsl_ir_function_decl *entry_func, struct dxbc_writer *dxbc)
> {
>     const struct hlsl_profile_info *profile = ctx->profile;
>     struct vkd3d_bytecode_buffer buffer = {0};
>     const struct hlsl_buffer *cbuffer;
> - const struct hlsl_ir_node *instr;
>  const struct hlsl_ir_var *var;
>  size_t token_count_position;
>
> @@ -824,6 +912,30 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
>       if (ctx->temp_count)
>           write_sm4_dcl_temps(&buffer, ctx->temp_count);
> -
>
> LIST_FOR_EACH_ENTRY(instr, entry_func->body, struct hlsl_ir_node, entry)
> -
>
> {
> -
>    if (instr->data_type)
> -
>    {
> -
>        if (instr->data_type->type == HLSL_CLASS_MATRIX)
> -
>        {
> -
>            FIXME("Matrix operations need to be lowered.\n");
> -
>            break;
> -
>        }
> -
> -
>        assert(instr->data_type->type == HLSL_CLASS_SCALAR || instr->data_type->type == HLSL_CLASS_VECTOR);
> -
>    }
> -
> -
>    switch (instr->type)
> -
>    {
> -
>        case HLSL_IR_STORE:
> -
>            write_sm4_store(ctx, &buffer, hlsl_ir_store(instr));
> -
>            break;
> -
> -
>        default:
> -
>            FIXME("Unhandled instruction type %s.\n", hlsl_node_type_to_string(instr->type));
> -
>    }
> -
>
> }
> -
> write_sm4_ret(&buffer);
>
> set_u32(&buffer, token_count_position, bytecode_get_size(&buffer) / sizeof(uint32_t));

I would split this part to a dedicated function that writes a block of 
instructions, and which we can reuse when we implement codegen for loops 
and conditions. The declaration might be something like:

static void write_sm4_block(struct hlsl_ctx *ctx, struct 
vkd3d_bytecode_buffer *buffer, const struct list *instrs);

Also, I would split the change in two commits: one that introduces 
write_sm4_block (including the check on matrix class) and another one 
that really implements store.
> @@ -842,7 +954,7 @@ int hlsl_sm4_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
>       write_sm4_signature(ctx, &dxbc, false);
>       write_sm4_signature(ctx, &dxbc, true);
>       write_sm4_rdef(ctx, &dxbc);
> - write_sm4_shdr(ctx, &dxbc);
> -
>
> write_sm4_shdr(ctx, entry_func, &dxbc);
>
> if (!(ret = ctx->result))
>      ret = dxbc_writer_write(&dxbc, out);

Thanks, Giovanni.

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Matteo Bruni]

On Fri, Aug 27, 2021 at 11:01 AM Giovanni Mascellani
 wrote:
> Hi,
>
> Il 24/08/21 08:18, Zebediah Figura ha scritto:
> > Signed-off-by: Zebediah Figura
> >
> > libs/vkd3d-shader/hlsl_sm4.c | 116 ++++++++++++++++++++++++++++++++++-
> >   1 file changed, 114 insertions(+), 2 deletions(-)
> >
> > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > index 15927671..2e304c5a 100644
> > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > @@ -591,6 +591,7 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
> >       {
> >           case VKD3D_SM4_RT_CONSTBUFFER:
> >           case VKD3D_SM4_RT_INPUT:
> > -
> >    case VKD3D_SM4_RT_TEMP:
> >         return VKD3D_SM4_SWIZZLE_VEC4;
> >
> >     default:
> >
> > @@ -599,6 +600,64 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
> >       }
> >   }
> >
> > +static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *reg,
> > -
> >    unsigned int *writemask, const struct hlsl_deref *deref, const struct hlsl_type *data_type)
> >
> > +{
> > - const struct hlsl_ir_var *var = deref->var;
> > -
> > - if (var->is_output_semantic)
> > - {
> > -
> >    bool has_idx;
> > -
> > -
> >    if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, true, &reg->type, &has_idx))
> > -
> >    {
> > -
> >        if (has_idx)
> > -
> >        {
> > -
> >            reg->idx[0] = var->semantic.index;
> > -
> >            reg->idx_count = 1;
> > -
> >        }
> > -
> > -
> >        if (reg->type == VKD3D_SM4_RT_DEPTHOUT)
> > -
> >            reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
> > -
> >        else
> > -
> >            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >        *writemask = (1u << data_type->dimx) - 1;
> > -
> >    }
> > -
> >    else
> > -
> >    {
> > -
> >        struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > -
> > -
> >        assert(hlsl_reg.allocated);
> > -
> >        reg->type = VKD3D_SM4_RT_OUTPUT;
> > -
> >        reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >        reg->idx[0] = hlsl_reg.id;
> > -
> >        reg->idx_count = 1;
> > -
> >        *writemask = hlsl_reg.writemask;
> > -
> >    }
> > - }
> > - else
> > - {
> > -
> >    struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
> > -
> > -
> >    assert(hlsl_reg.allocated);
> > -
> >    reg->type = VKD3D_SM4_RT_TEMP;
> > -
> >    reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > -
> >    reg->idx[0] = hlsl_reg.id;
> > -
> >    reg->idx_count = 1;
> > -
> >    *writemask = hlsl_reg.writemask;
> > - }
> >
> > +}
> > -
> >
> > +static void sm4_register_from_node(struct sm4_register *reg, unsigned int *writemask, const struct hlsl_ir_node *instr)
> > +{
> > - assert(instr->reg.allocated);
> > - reg->type = VKD3D_SM4_RT_TEMP;
> > - reg->dim = VKD3D_SM4_DIMENSION_VEC4;
> > - reg->idx[0] = instr->reg.id;
> > - reg->idx_count = 1;
> > - *writemask = instr->reg.writemask;
> >
> > +}
> > -
>
> I think I would use "node" as variable name instead of "instr", so there
> is one less term equivalence to keep in the mind when reading the code.
> But it's not a big deal.

Actually we're moving away from node to instruction. Node is a
historical artifact of the original IR which was tree-based and it
doesn't quite fit nowadays.
> > static uint32_t sm4_encode_register(const struct sm4_register *reg)
> >   {
> >       return (reg->type << VKD3D_SM4_REGISTER_TYPE_SHIFT)
> > @@ -785,11 +844,40 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
> >       write_sm4_instruction(buffer, &instr);
> >   }
> >
> > -static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
> > +static void write_sm4_store(struct hlsl_ctx *ctx,
> > -
> >    struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_store *store)
> >
> > +{
> > - const struct hlsl_ir_node *rhs = store->rhs.node;
> > - struct sm4_instruction instr;
> > - unsigned int writemask;
> > -
> > - if (store->lhs.var->data_type->type == HLSL_CLASS_MATRIX)
> > - {
> > -
> >    hlsl_fixme(ctx, store->node.loc, "Store to a matrix variable.\n");
> > -
> >    return;
> > - }
> > -
> > - memset(&instr, 0, sizeof(instr));
> > - instr.opcode = VKD3D_SM4_OP_MOV;
> > -
> > - sm4_register_from_deref(ctx, &instr.dst.reg, &writemask, &store->lhs, rhs->data_type);
> > - instr.dst.writemask = hlsl_combine_writemasks(writemask, store->writemask);
> > - instr.has_dst = 1;
> > -
> > - sm4_register_from_node(&instr.srcs[0].reg, &writemask, rhs);
> > - instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
> > - instr.src_count = 1;
> > -
> > - write_sm4_instruction(buffer, &instr);
> >
> > +}
> > -
> >
> > +static void write_sm4_shdr(struct hlsl_ctx *ctx,
> > -
> >    const struct hlsl_ir_function_decl *entry_func, struct dxbc_writer *dxbc)
> > {
> >     const struct hlsl_profile_info *profile = ctx->profile;
> >     struct vkd3d_bytecode_buffer buffer = {0};
> >     const struct hlsl_buffer *cbuffer;
> > - const struct hlsl_ir_node *instr;
> >  const struct hlsl_ir_var *var;
> >  size_t token_count_position;
> >
> > @@ -824,6 +912,30 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
> >       if (ctx->temp_count)
> >           write_sm4_dcl_temps(&buffer, ctx->temp_count);
> > -
> >
> > LIST_FOR_EACH_ENTRY(instr, entry_func->body, struct hlsl_ir_node, entry)
> > -
> >
> > {
> > -
> >    if (instr->data_type)
> > -
> >    {
> > -
> >        if (instr->data_type->type == HLSL_CLASS_MATRIX)
> > -
> >        {
> > -
> >            FIXME("Matrix operations need to be lowered.\n");
> > -
> >            break;
> > -
> >        }
> > -
> > -
> >        assert(instr->data_type->type == HLSL_CLASS_SCALAR || instr->data_type->type == HLSL_CLASS_VECTOR);
> > -
> >    }
> > -
> > -
> >    switch (instr->type)
> > -
> >    {
> > -
> >        case HLSL_IR_STORE:
> > -
> >            write_sm4_store(ctx, &buffer, hlsl_ir_store(instr));
> > -
> >            break;
> > -
> > -
> >        default:
> > -
> >            FIXME("Unhandled instruction type %s.\n", hlsl_node_type_to_string(instr->type));
> > -
> >    }
> > -
> >
> > }
> > -
> > write_sm4_ret(&buffer);
> >
> > set_u32(&buffer, token_count_position, bytecode_get_size(&buffer) / sizeof(uint32_t));
>
> I would split this part to a dedicated function that writes a block of
> instructions, and which we can reuse when we implement codegen for loops
> and conditions. The declaration might be something like:
>
> static void write_sm4_block(struct hlsl_ctx *ctx, struct
> vkd3d_bytecode_buffer *buffer, const struct list *instrs);
>
> Also, I would split the change in two commits: one that introduces
> write_sm4_block (including the check on matrix class) and another one
> that really implements store.

You don't want to introduce dead code though, so making that kind of
split at this point is a bit awkward.

Personally I'm generally for introducing shared helpers as they become
necessary rather than upfront. For example, if at the time of
implementing conditional or loop instructions, as it's likely, some
existing code parts can be nicely reused, a new helper can be created
by factoring out the relevant code. An upside of waiting for a second
user is that you're sure to make the helper(s) you know you need
rather than what you now think you'll need at some point in the
future.
One case where you want to split code early is when functions are
getting too big to be manageable otherwise. That doesn't seem to be
the case here though.

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Giovanni Mascellani]

Hi,

Il 27/08/21 11:52, Matteo Bruni ha scritto:
> You don't want to introduce dead code though, so making that kind of
> split at this point is a bit awkward.

My suggestion would not introduce dead code (i.e., code that never gets 
executed). You move some code to another function, but still call it.
> Personally I'm generally for introducing shared helpers as they become
> necessary rather than upfront. For example, if at the time of
> implementing conditional or loop instructions, as it's likely, some
> existing code parts can be nicely reused, a new helper can be created
> by factoring out the relevant code. An upside of waiting for a second
> user is that you're sure to make the helper(s) you know you need
> rather than what you now think you'll need at some point in the
> future.

Well, in a sense I am already that second user: I have patches for ifs 
and loops, including a patch that introduces the write_sm4_block helper 
I suggested (with precisely that signature).

That said, no problem going through another patch.

Thanks, Giovanni.

[PATCH vkd3d 2/6] vkd3d-shader/hlsl: Write SM4 store instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 3/6] vkd3d-shader/hlsl: Write SM4 load instructions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 Makefile.am                  |  1 -
 libs/vkd3d-shader/hlsl_sm4.c | 60 +++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index fc1714f7..1bbe1c5f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -274,7 +274,6 @@ XFAIL_TESTS = \
    tests/hlsl-static-initializer.shader_test \
    tests/hlsl-storage-qualifiers.shader_test \
    tests/hlsl-struct-assignment.shader_test \
-	tests/hlsl-struct-semantics.shader_test \
    tests/hlsl-vector-indexing.shader_test \
    tests/hlsl-vector-indexing-uniform.shader_test \
    tests/math.shader_test \
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 2e304c5a..723b6005 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -605,7 +605,42 @@ static void sm4_register_from_deref(struct hlsl_ctx *ctx, struct sm4_register *r
 {
     const struct hlsl_ir_var *var = deref->var;

-    if (var->is_output_semantic)
+    if (var->is_uniform)
+    {
+        reg->type = VKD3D_SM4_RT_CONSTBUFFER;
+        reg->idx[0] = var->buffer->reg.id;
+        reg->idx[1] = var->buffer_offset / 4;
+        reg->idx_count = 2;
+        *writemask = ((1u << data_type->dimx) - 1) << (var->buffer_offset & 3);
+    }
+    else if (var->is_input_semantic)
+    {
+        bool has_idx;
+
+        if (hlsl_sm4_register_from_semantic(ctx, &var->semantic, false, &reg->type, &has_idx))
+        {
+            if (has_idx)
+            {
+                reg->idx[0] = var->semantic.index;
+                reg->idx_count = 1;
+            }
+
+            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+            *writemask = (1u << data_type->dimx) - 1;
+        }
+        else
+        {
+            struct hlsl_reg hlsl_reg = hlsl_reg_from_deref(deref, data_type);
+
+            assert(hlsl_reg.allocated);
+            reg->type = VKD3D_SM4_RT_INPUT;
+            reg->dim = VKD3D_SM4_DIMENSION_VEC4;
+            reg->idx[0] = hlsl_reg.id;
+            reg->idx_count = 1;
+            *writemask = hlsl_reg.writemask;
+        }
+    }
+    else if (var->is_output_semantic)
     {
         bool has_idx;

@@ -844,6 +879,25 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
     write_sm4_instruction(buffer, &instr);
 }

+static void write_sm4_load(struct hlsl_ctx *ctx,
+        struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_load *load)
+{
+    struct sm4_instruction instr;
+    unsigned int writemask;
+
+    memset(&instr, 0, sizeof(instr));
+    instr.opcode = VKD3D_SM4_OP_MOV;
+
+    sm4_register_from_node(&instr.dst.reg, &instr.dst.writemask, &load->node);
+    instr.has_dst = 1;
+
+    sm4_register_from_deref(ctx, &instr.srcs[0].reg, &writemask, &load->src, load->node.data_type);
+    instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
+    instr.src_count = 1;
+
+    write_sm4_instruction(buffer, &instr);
+}
+
 static void write_sm4_store(struct hlsl_ctx *ctx,
         struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_store *store)
 {
@@ -927,6 +981,10 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx,

switch (instr->type)
         {
+            case HLSL_IR_LOAD:
+                write_sm4_load(ctx, &buffer, hlsl_ir_load(instr));
+                break;
+
             case HLSL_IR_STORE:
                 write_sm4_store(ctx, &buffer, hlsl_ir_store(instr));
                 break;

[PATCH vkd3d 3/6] vkd3d-shader/hlsl: Write SM4 load instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 3/6] vkd3d-shader/hlsl: Write SM4 load instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 4/6] vkd3d-shader/hlsl: Write SM4 swizzle instructions.

[Zebediah Figura]

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

diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 723b6005..73bac7b7 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -925,6 +925,26 @@ static void write_sm4_store(struct hlsl_ctx *ctx,
     write_sm4_instruction(buffer, &instr);
 }

+static void write_sm4_swizzle(struct hlsl_ctx *ctx,
+        struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_swizzle *swizzle)
+{
+    struct sm4_instruction instr;
+    unsigned int writemask;
+
+    memset(&instr, 0, sizeof(instr));
+    instr.opcode = VKD3D_SM4_OP_MOV;
+
+    sm4_register_from_node(&instr.dst.reg, &instr.dst.writemask, &swizzle->node);
+    instr.has_dst = 1;
+
+    sm4_register_from_node(&instr.srcs[0].reg, &writemask, swizzle->val.node);
+    instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_combine_swizzles(hlsl_swizzle_from_writemask(writemask),
+            swizzle->swizzle, swizzle->node.data_type->dimx), instr.dst.writemask);
+    instr.src_count = 1;
+
+    write_sm4_instruction(buffer, &instr);
+}
+
 static void write_sm4_shdr(struct hlsl_ctx *ctx,
         const struct hlsl_ir_function_decl *entry_func, struct dxbc_writer *dxbc)
 {
@@ -989,6 +1009,10 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx,
                 write_sm4_store(ctx, &buffer, hlsl_ir_store(instr));
                 break;

+            case HLSL_IR_SWIZZLE:
+                write_sm4_swizzle(ctx, &buffer, hlsl_ir_swizzle(instr));
+                break;
+
             default:
                 FIXME("Unhandled instruction type %s.\n", hlsl_node_type_to_string(instr->type));
         }

[PATCH vkd3d 4/6] vkd3d-shader/hlsl: Write SM4 swizzle instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 4/6] vkd3d-shader/hlsl: Write SM4 swizzle instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 5/6] vkd3d-shader/hlsl: Write SM4 constant instructions.

[Zebediah Figura]

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

diff --git a/Makefile.am b/Makefile.am
index 1bbe1c5f..9d0410ab 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -277,14 +277,6 @@ XFAIL_TESTS = \
    tests/hlsl-vector-indexing.shader_test \
    tests/hlsl-vector-indexing-uniform.shader_test \
    tests/math.shader_test \
-	tests/swizzle-0.shader_test \
-	tests/swizzle-1.shader_test \
-	tests/swizzle-2.shader_test \
-	tests/swizzle-3.shader_test \
-	tests/swizzle-4.shader_test \
-	tests/swizzle-5.shader_test \
-	tests/swizzle-6.shader_test \
-	tests/swizzle-7.shader_test \
    tests/trigonometry.shader_test \
    tests/writemask-assignop-0.shader_test \
    tests/writemask-assignop-1.shader_test \
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 73bac7b7..27058fef 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -560,6 +560,7 @@ struct sm4_register
     uint32_t idx[2];
     unsigned int idx_count;
     enum vkd3d_sm4_dimension dim;
+    uint32_t immconst_uint[4];
 };

struct sm4_instruction
@@ -589,6 +590,9 @@ static unsigned int sm4_swizzle_type(enum vkd3d_sm4_register_type type)
 {
     switch (type)
     {
+        case VKD3D_SM4_RT_IMMCONST:
+            return VKD3D_SM4_SWIZZLE_NONE;
+
         case VKD3D_SM4_RT_CONSTBUFFER:
         case VKD3D_SM4_RT_INPUT:
         case VKD3D_SM4_RT_TEMP:
@@ -743,6 +747,17 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st

for (j = 0; j < instr->srcs[i].reg.idx_count; ++j)
             put_u32(buffer, instr->srcs[i].reg.idx[j]);
+
+        if (instr->srcs[i].reg.type == VKD3D_SM4_RT_IMMCONST)
+        {
+            put_u32(buffer, instr->srcs[i].reg.immconst_uint[0]);
+            if (instr->srcs[i].reg.dim == VKD3D_SM4_DIMENSION_VEC4)
+            {
+                put_u32(buffer, instr->srcs[i].reg.immconst_uint[1]);
+                put_u32(buffer, instr->srcs[i].reg.immconst_uint[2]);
+                put_u32(buffer, instr->srcs[i].reg.immconst_uint[3]);
+            }
+        }
     }

for (j = 0; j < instr->idx_count; ++j)
@@ -879,6 +894,30 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
     write_sm4_instruction(buffer, &instr);
 }

+static void write_sm4_constant(struct hlsl_ctx *ctx,
+        struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_constant *constant)
+{
+    const unsigned int dimx = constant->node.data_type->dimx;
+    struct sm4_instruction instr;
+    unsigned int i;
+
+    memset(&instr, 0, sizeof(instr));
+    instr.opcode = VKD3D_SM4_OP_MOV;
+
+    sm4_register_from_node(&instr.dst.reg, &instr.dst.writemask, &constant->node);
+    instr.has_dst = 1;
+
+    instr.srcs[0].reg.dim = (dimx > 1) ? VKD3D_SM4_DIMENSION_VEC4 : VKD3D_SM4_DIMENSION_SCALAR;
+    instr.srcs[0].reg.type = VKD3D_SM4_RT_IMMCONST;
+    instr.srcs[0].reg.idx[0] = constant->reg.id;
+    instr.srcs[0].reg.idx_count = 1;
+    for (i = 0; i < dimx; ++i)
+        instr.srcs[0].reg.immconst_uint[i] = constant->value.u[i];
+    instr.src_count = 1,
+
+    write_sm4_instruction(buffer, &instr);
+}
+
 static void write_sm4_load(struct hlsl_ctx *ctx,
         struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_load *load)
 {
@@ -1001,6 +1040,10 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx,

switch (instr->type)
         {
+            case HLSL_IR_CONSTANT:
+                write_sm4_constant(ctx, &buffer, hlsl_ir_constant(instr));
+                break;
+
             case HLSL_IR_LOAD:
                 write_sm4_load(ctx, &buffer, hlsl_ir_load(instr));
                 break;

[PATCH vkd3d 5/6] vkd3d-shader/hlsl: Write SM4 constant instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 5/6] vkd3d-shader/hlsl: Write SM4 constant instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 Makefile.am                  |  1 -
 libs/vkd3d-shader/hlsl_sm4.c | 62 ++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 9d0410ab..c66f7b50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -273,7 +273,6 @@ XFAIL_TESTS = \
    tests/hlsl-return-void.shader_test \
    tests/hlsl-static-initializer.shader_test \
    tests/hlsl-storage-qualifiers.shader_test \
-	tests/hlsl-struct-assignment.shader_test \
    tests/hlsl-vector-indexing.shader_test \
    tests/hlsl-vector-indexing-uniform.shader_test \
    tests/math.shader_test \
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 27058fef..32a4eda1 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -894,6 +894,27 @@ static void write_sm4_ret(struct vkd3d_bytecode_buffer *buffer)
     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)
+{
+    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, src1);
+    instr.srcs[0].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
+    sm4_register_from_node(&instr.srcs[1].reg, &writemask, src2);
+    instr.srcs[1].swizzle = hlsl_map_swizzle(hlsl_swizzle_from_writemask(writemask), instr.dst.writemask);
+    instr.src_count = 2;
+
+    write_sm4_instruction(buffer, &instr);
+}
+
 static void write_sm4_constant(struct hlsl_ctx *ctx,
         struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_constant *constant)
 {
@@ -918,6 +939,43 @@ static void write_sm4_constant(struct hlsl_ctx *ctx,
     write_sm4_instruction(buffer, &instr);
 }

+static void write_sm4_expr(struct hlsl_ctx *ctx,
+        struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_expr *expr)
+{
+    const struct hlsl_ir_node *arg1 = expr->operands[0].node;
+    const struct hlsl_ir_node *arg2 = expr->operands[1].node;
+
+    assert(expr->node.reg.allocated);
+
+    switch (expr->node.data_type->base_type)
+    {
+        case HLSL_TYPE_FLOAT:
+        {
+            switch (expr->op)
+            {
+                case HLSL_OP2_ADD:
+                    write_sm4_binary_op(buffer, VKD3D_SM4_OP_ADD, &expr->node, arg1, arg2);
+                    break;
+
+                default:
+                    hlsl_fixme(ctx, expr->node.loc, "SM4 float "%s" expression.", debug_hlsl_expr_op(expr->op));
+                    break;
+            }
+            break;
+        }
+
+        default:
+        {
+            struct vkd3d_string_buffer *string;
+
+            if ((string = hlsl_type_to_string(ctx, expr->node.data_type)))
+                hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", string->buffer);
+            hlsl_release_string_buffer(ctx, string);
+            break;
+        }
+    }
+}
+
 static void write_sm4_load(struct hlsl_ctx *ctx,
         struct vkd3d_bytecode_buffer *buffer, const struct hlsl_ir_load *load)
 {
@@ -1044,6 +1102,10 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx,
                 write_sm4_constant(ctx, &buffer, hlsl_ir_constant(instr));
                 break;

+            case HLSL_IR_EXPR:
+                write_sm4_expr(ctx, &buffer, hlsl_ir_expr(instr));
+                break;
+
             case HLSL_IR_LOAD:
                 write_sm4_load(ctx, &buffer, hlsl_ir_load(instr));
                 break;

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Matteo Bruni]

On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura  wrote:
> Signed-off-by: Zebediah Figura
>
> Makefile.am                  |  1 -
>  libs/vkd3d-shader/hlsl_sm4.c | 62 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 1 deletion(-)
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index 27058fef..32a4eda1 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> -
>    default:
> -
>    {
> -
>        struct vkd3d_string_buffer *string;
> -
> -
>        if ((string = hlsl_type_to_string(ctx, expr->node.data_type)))
> -
>            hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", string->buffer);
> -
>        hlsl_release_string_buffer(ctx, string);
> -
>        break;
> -
>    }
> - }
>
> +}

This means that the hlsl_fixme(), and thus the compilation failure,
only happens when hlsl_type_to_string() succeeds. It's a pretty
theoretical issue but still a bit surprising.

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Zebediah Figura (she/her)]

On 8/26/21 10:48 AM, Matteo Bruni wrote:
> On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura  wrote:
> > Signed-off-by: Zebediah Figura
> >
> > Makefile.am                  |  1 -
> >   libs/vkd3d-shader/hlsl_sm4.c | 62 ++++++++++++++++++++++++++++++++++++
> >   2 files changed, 62 insertions(+), 1 deletion(-)
> > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > index 27058fef..32a4eda1 100644
> > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > -
> >    default:
> > -
> >    {
> > -
> >        struct vkd3d_string_buffer *string;
> > -
> > -
> >        if ((string = hlsl_type_to_string(ctx, expr->node.data_type)))
> > -
> >            hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", string->buffer);
> > -
> >        hlsl_release_string_buffer(ctx, string);
> > -
> >        break;
> > -
> >    }
> > - }
> >
> > +}
>
> This means that the hlsl_fixme(), and thus the compilation failure,
> only happens when hlsl_type_to_string() succeeds. It's a pretty
> theoretical issue but still a bit surprising.

That's true in general. On the other hand, if hlsl_type_to_string fails, 
it'll set ctx->result to VKD3D_ERROR_OUT_OF_MEMORY, which 
vkd3d_shader_compile() will return. Since we almost certainly can't 
print an error at that point anyway, I think that qualifies as the best 
we can do.

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Matteo Bruni]

On Thu, Aug 26, 2021 at 6:26 PM Zebediah Figura (she/her)
 wrote:
> On 8/26/21 10:48 AM, Matteo Bruni wrote:
> > On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura  wrote:
> > > Signed-off-by: Zebediah Figura
> > >
> > > Makefile.am                  |  1 -
> > >   libs/vkd3d-shader/hlsl_sm4.c | 62 ++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 62 insertions(+), 1 deletion(-)
> > > diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> > > index 27058fef..32a4eda1 100644
> > > --- a/libs/vkd3d-shader/hlsl_sm4.c
> > > +++ b/libs/vkd3d-shader/hlsl_sm4.c
> > > -
> > >    default:
> > > -
> > >    {
> > > -
> > >        struct vkd3d_string_buffer *string;
> > > -
> > > -
> > >        if ((string = hlsl_type_to_string(ctx, expr->node.data_type)))
> > > -
> > >            hlsl_fixme(ctx, expr->node.loc, "SM4 %s expression.", string->buffer);
> > > -
> > >        hlsl_release_string_buffer(ctx, string);
> > > -
> > >        break;
> > > -
> > >    }
> > > - }
> > >
> > > +}
> >
> > This means that the hlsl_fixme(), and thus the compilation failure,
> > only happens when hlsl_type_to_string() succeeds. It's a pretty
> > theoretical issue but still a bit surprising.
>
> That's true in general. On the other hand, if hlsl_type_to_string fails,
> it'll set ctx->result to VKD3D_ERROR_OUT_OF_MEMORY, which
> vkd3d_shader_compile() will return. Since we almost certainly can't
> print an error at that point anyway, I think that qualifies as the best
> we can do.

Ah, indeed. That's certainly good enough for me.

[PATCH vkd3d 6/6] vkd3d-shader/hlsl: Write SM4 addition instructions.

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[Matteo Bruni]

Signed-off-by: Matteo Bruni

[Henri Verbeet]

Signed-off-by: Henri Verbeet