[PATCH vkd3d 1/6] vkd3d-shader/hlsl: Avoid crashing when emitting a dereference with non-constant offset.
Some places assert() that the register is allocated. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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; -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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); -- 2.32.0
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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 :)
On 8/26/21 10:47 AM, Matteo Bruni wrote:
On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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...
On Thu, Aug 26, 2021 at 6:23 PM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 8/26/21 10:47 AM, Matteo Bruni wrote:
On Tue, Aug 24, 2021 at 8:18 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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.
Hi, Il 24/08/21 08:18, Zebediah Figura ha scritto:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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.
On Fri, Aug 27, 2021 at 11:01 AM Giovanni Mascellani <gmascellani(a)codeweavers.com> wrote:
Hi,
Il 24/08/21 08:18, Zebediah Figura ha scritto:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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.
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.
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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, ®->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; -- 2.32.0
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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)); } -- 2.32.0
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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; -- 2.32.0
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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; -- 2.32.0
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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.
On 8/26/21 10:48 AM, Matteo Bruni wrote:
On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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.
On Thu, Aug 26, 2021 at 6:26 PM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 8/26/21 10:48 AM, Matteo Bruni wrote:
On Tue, Aug 24, 2021 at 8:19 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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.
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
participants (6)
-
Giovanni Mascellani -
Henri Verbeet -
Matteo Bruni -
Matteo Bruni -
Zebediah Figura -
Zebediah Figura (she/her)