[PATCH v2 0/6] MR278: vkd3d-shader/dxil: Emit the shader instructions.
Sufficient for compiling a no-op pixel shader. This should probably be rebased on top of !263 because it introduces vkd3d_spirv_get_type_id_for_data_type(), which 263 renders unnecessary. !263 is not essential, but I think using two different type systems in the backend is not ideal. -- v2: vkd3d-shader/dxil: Emit undefined constants. vkd3d-shader/spirv: Introduce an undefined register type. vkd3d-shader/dxbc: Allow DXBC containers to have a zero checksum. vkd3d-shader/dxil: Emit the shader instructions. vkd3d-shader/spirv: Do not normalise Shader Model 6 shaders. vkd3d-shader/dxil: Return an error from sm6_parser_globals_init() on invalid operand count. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 53a4c2da..7011cd56 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -1767,7 +1767,8 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) break; case MODULE_CODE_VERSION: - dxil_record_validate_operand_count(record, 1, 1, sm6); + if (!dxil_record_validate_operand_count(record, 1, 1, sm6)) + return VKD3D_ERROR_INVALID_SHADER; if ((version = record->operands[0]) != 1) { FIXME("Unsupported format version %#"PRIx64".\n", version); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index dcf957ea..9f9015ee 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2228,6 +2228,7 @@ struct spirv_compiler uint32_t sample_positions_id; enum vkd3d_shader_type shader_type; + bool is_dxil; unsigned int branch_id; unsigned int loop_id; @@ -2428,6 +2429,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_ve rb_init(&compiler->symbol_table, vkd3d_symbol_compare); compiler->shader_type = shader_version->type; + compiler->is_dxil = shader_version->major >= 6; if ((shader_interface = vkd3d_find_struct(compile_info->next, INTERFACE_INFO))) { @@ -9480,7 +9482,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, compiler->location.column = 0; compiler->location.line = 1; - if ((result = vkd3d_shader_normalise(parser)) < 0) + if (!compiler->is_dxil && (result = vkd3d_shader_normalise(parser)) < 0) return result; instructions = parser->instructions; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> Sufficient for compiling a no-op pixel shader. --- libs/vkd3d-shader/dxil.c | 46 ++++++++++++++++++++++++ libs/vkd3d-shader/ir.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 2 ++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 7011cd56..87924393 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -1737,6 +1737,27 @@ static enum vkd3d_result sm6_parser_constants_init(struct sm6_parser *sm6, const return VKD3D_OK; } +static struct vkd3d_shader_instruction *sm6_parser_require_space(struct sm6_parser *sm6, size_t extra) +{ + if (!shader_instruction_array_reserve(&sm6->p.instructions, sm6->p.instructions.count + extra)) + { + ERR("Failed to allocate instruction.\n"); + return NULL; + } + return &sm6->p.instructions.elements[sm6->p.instructions.count]; +} + +/* Space should be reserved before calling this. It is intended to require no checking of the returned pointer. */ +static struct vkd3d_shader_instruction *sm6_parser_add_instruction(struct sm6_parser *sm6, + enum vkd3d_shader_opcode handler_idx) +{ + struct vkd3d_shader_instruction *ins = sm6_parser_require_space(sm6, 1); + assert(ins); + shader_instruction_init(ins, handler_idx); + ++sm6->p.instructions.count; + return ins; +} + static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) { const struct dxil_block *block = &sm6->root_block; @@ -1932,6 +1953,19 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const return VKD3D_OK; } +static bool sm6_block_emit_instructions(struct sm6_block *block, struct sm6_parser *sm6) +{ + /* Reserve an extra instruction for the block terminator. */ + struct vkd3d_shader_instruction *ins = sm6_parser_require_space(sm6, block->instruction_count + 1); + + if (!ins) + return false; + + memcpy(ins, block->instructions, block->instruction_count * sizeof(*block->instructions)); + sm6->p.instructions.count += block->instruction_count; + return true; +} + static enum vkd3d_result sm6_parser_module_init(struct sm6_parser *sm6, const struct dxil_block *block, unsigned int level) { @@ -2066,6 +2100,7 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t struct vkd3d_shader_version version; struct dxil_block *block; enum vkd3d_result ret; + unsigned int i; count = byte_code_size / sizeof(*byte_code); if (count < 6) @@ -2255,6 +2290,17 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + for (i = 0; i < sm6->function_count; ++i) + { + if (!sm6_block_emit_instructions(sm6->functions[i].blocks[0], sm6)) + { + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory emitting shader instructions."); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + sm6_parser_add_instruction(sm6, VKD3DSIH_RET); + } + dxil_block_destroy(&sm6->root_block); return VKD3D_OK; diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 1ce05247..2c9c85ea 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -247,7 +247,7 @@ static void shader_register_init(struct vkd3d_shader_register *reg, enum vkd3d_s reg->immconst_type = VKD3D_IMMCONST_SCALAR; } -static void shader_instruction_init(struct vkd3d_shader_instruction *ins, enum vkd3d_shader_opcode handler_idx) +void shader_instruction_init(struct vkd3d_shader_instruction *ins, enum vkd3d_shader_opcode handler_idx) { memset(ins, 0, sizeof(*ins)); ins->handler_idx = handler_idx; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 49ad7b3e..233f3c86 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -976,6 +976,8 @@ struct vkd3d_shader_instruction } declaration; }; +void shader_instruction_init(struct vkd3d_shader_instruction *ins, enum vkd3d_shader_opcode handler_idx); + static inline bool vkd3d_shader_instruction_has_texel_offset(const struct vkd3d_shader_instruction *ins) { return ins->texel_offset.u || ins->texel_offset.v || ins->texel_offset.w; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> The linux build of DXC writes a zero checksum. --- libs/vkd3d-shader/dxbc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 716b7bdb..6dbefe2e 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -179,7 +179,12 @@ static int parse_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_ read_dword(&ptr, &checksum[2]); read_dword(&ptr, &checksum[3]); vkd3d_compute_dxbc_checksum(data, data_size, calculated_checksum); - if (memcmp(checksum, calculated_checksum, sizeof(checksum))) + if (!checksum[0] && !checksum[1] && !checksum[2] && !checksum[3]) + { + WARN("Checksum is zero; calculated checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x}.\n", + calculated_checksum[0], calculated_checksum[1], calculated_checksum[2], calculated_checksum[3]); + } + else if (memcmp(checksum, calculated_checksum, sizeof(checksum))) { WARN("Checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x} does not match " "calculated checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x}.\n", -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/spirv.c | 70 ++++++++++++++++++++++-- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 9f9015ee..1a8361be 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -199,6 +199,21 @@ enum vkd3d_shader_input_sysval_semantic vkd3d_siv_from_sysval_indexed(enum vkd3d } } +static inline bool register_is_undef(const struct vkd3d_shader_register *reg) +{ + return reg->type == VKD3DSPR_UNDEF; +} + +static inline bool register_is_constant(const struct vkd3d_shader_register *reg) +{ + return (reg->type == VKD3DSPR_IMMCONST || reg->type == VKD3DSPR_IMMCONST64); +} + +static inline bool register_is_constant_or_undef(const struct vkd3d_shader_register *reg) +{ + return register_is_constant(reg) || register_is_undef(reg); +} + #define VKD3D_SPIRV_VERSION 0x00010000 #define VKD3D_SPIRV_GENERATOR_ID 18 #define VKD3D_SPIRV_GENERATOR_VERSION 8 @@ -1746,6 +1761,38 @@ static uint32_t vkd3d_spirv_get_type_id(struct vkd3d_spirv_builder *builder, } } +static uint32_t vkd3d_spirv_get_type_id_for_data_type(struct vkd3d_spirv_builder *builder, + enum vkd3d_data_type data_type, unsigned int component_count) +{ + uint32_t scalar_id; + + if (component_count == 1) + { + switch (data_type) + { + case VKD3D_DATA_FLOAT: + case VKD3D_DATA_SNORM: + case VKD3D_DATA_UNORM: + return vkd3d_spirv_get_op_type_float(builder, 32); + break; + case VKD3D_DATA_INT: + case VKD3D_DATA_UINT: + return vkd3d_spirv_get_op_type_int(builder, 32, data_type == VKD3D_DATA_INT); + break; + case VKD3D_DATA_DOUBLE: + return vkd3d_spirv_get_op_type_float(builder, 64); + default: + FIXME("Unhandled data type %#x.\n", data_type); + return 0; + } + } + else + { + scalar_id = vkd3d_spirv_get_type_id_for_data_type(builder, data_type, 1); + return vkd3d_spirv_get_op_type_vector(builder, scalar_id, component_count); + } +} + static void vkd3d_spirv_builder_init(struct vkd3d_spirv_builder *builder, const char *entry_point) { vkd3d_spirv_stream_init(&builder->debug_stream); @@ -3206,7 +3253,7 @@ static bool spirv_compiler_get_register_info(const struct spirv_compiler *compil struct vkd3d_symbol reg_symbol, *symbol; struct rb_entry *entry; - assert(reg->type != VKD3DSPR_IMMCONST && reg->type != VKD3DSPR_IMMCONST64); + assert(!register_is_constant_or_undef(reg)); if (reg->type == VKD3DSPR_TEMP) { @@ -3548,6 +3595,19 @@ static uint32_t spirv_compiler_emit_load_constant64(struct spirv_compiler *compi vkd3d_component_type_from_data_type(reg->data_type), component_count, values); } +static uint32_t spirv_compiler_emit_load_undef(struct spirv_compiler *compiler, + const struct vkd3d_shader_register *reg, DWORD write_mask) +{ + unsigned int component_count = vkd3d_write_mask_component_count(write_mask); + struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; + uint32_t type_id; + + assert(reg->type == VKD3DSPR_UNDEF); + + type_id = vkd3d_spirv_get_type_id_for_data_type(builder, reg->data_type, component_count); + return vkd3d_spirv_build_op_undef(builder, &builder->global_stream, type_id); +} + static uint32_t spirv_compiler_emit_load_scalar(struct spirv_compiler *compiler, const struct vkd3d_shader_register *reg, DWORD swizzle, DWORD write_mask, const struct vkd3d_shader_register_info *reg_info) @@ -3558,7 +3618,7 @@ static uint32_t spirv_compiler_emit_load_scalar(struct spirv_compiler *compiler, enum vkd3d_shader_component_type component_type; unsigned int skipped_component_mask; - assert(reg->type != VKD3DSPR_IMMCONST && reg->type != VKD3DSPR_IMMCONST64); + assert(!register_is_constant_or_undef(reg)); assert(vkd3d_write_mask_component_count(write_mask) == 1); component_idx = vkd3d_write_mask_get_component_idx(write_mask); @@ -3610,6 +3670,8 @@ static uint32_t spirv_compiler_emit_load_reg(struct spirv_compiler *compiler, return spirv_compiler_emit_load_constant(compiler, reg, swizzle, write_mask); else if (reg->type == VKD3DSPR_IMMCONST64) return spirv_compiler_emit_load_constant64(compiler, reg, swizzle, write_mask); + else if (reg->type == VKD3DSPR_UNDEF) + return spirv_compiler_emit_load_undef(compiler, reg, write_mask); component_count = vkd3d_write_mask_component_count(write_mask); component_type = vkd3d_component_type_from_data_type(reg->data_type); @@ -3822,7 +3884,7 @@ static void spirv_compiler_emit_store_reg(struct spirv_compiler *compiler, unsigned int src_write_mask = write_mask; uint32_t type_id; - assert(reg->type != VKD3DSPR_IMMCONST && reg->type != VKD3DSPR_IMMCONST64); + assert(!register_is_constant_or_undef(reg)); if (!spirv_compiler_get_register_info(compiler, reg, ®_info)) return; @@ -6642,7 +6704,7 @@ static void spirv_compiler_emit_mov(struct spirv_compiler *compiler, uint32_t components[VKD3D_VEC4_SIZE]; unsigned int i, component_count; - if (src->reg.type == VKD3DSPR_IMMCONST || src->reg.type == VKD3DSPR_IMMCONST64 || dst->modifiers || src->modifiers) + if (register_is_constant_or_undef(&src->reg) || dst->modifiers || src->modifiers) goto general_implementation; spirv_compiler_get_register_info(compiler, &dst->reg, &dst_reg_info); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 233f3c86..d35f49a6 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -507,6 +507,7 @@ enum vkd3d_shader_register_type VKD3DSPR_DEPTHOUTLE, VKD3DSPR_RASTERIZER, VKD3DSPR_OUTSTENCILREF, + VKD3DSPR_UNDEF, VKD3DSPR_INVALID = ~0u, }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 87924393..e2c848d1 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -208,6 +208,7 @@ struct sm6_value { const struct sm6_type *type; enum sm6_value_type value_type; + bool is_undefined; union { struct sm6_function_data function; @@ -1726,8 +1727,16 @@ static enum vkd3d_result sm6_parser_constants_init(struct sm6_parser *sm6, const WARN("Unhandled constant array.\n"); break; + case CST_CODE_UNDEF: + dxil_record_validate_operand_max_count(record, 0, sm6); + dst->u.reg.type = VKD3DSPR_UNDEF; + /* Mark as explicitly undefined, not the result of a missing constant code or instruction. */ + dst->is_undefined = true; + break; + default: FIXME("Unhandled constant code %u.\n", record->code); + dst->u.reg.type = VKD3DSPR_UNDEF; break; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278
```diff @@ -2428,6 +2429,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_ve rb_init(&compiler->symbol_table, vkd3d_symbol_compare);
compiler->shader_type = shader_version->type; + compiler->is_dxil = shader_version->major >= 6;
if ((shader_interface = vkd3d_find_struct(compile_info->next, INTERFACE_INFO))) { @@ -9480,7 +9482,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, compiler->location.column = 0; compiler->location.line = 1;
- if ((result = vkd3d_shader_normalise(parser)) < 0) + if (!compiler->is_dxil && (result = vkd3d_shader_normalise(parser)) < 0) return result;
instructions = parser->instructions; ```
I think we can just do that check inside vkd3d_shader_normalise(), and that seems preferable.
```diff +static bool sm6_block_emit_instructions(struct sm6_block *block, struct sm6_parser *sm6) +{ + /* Reserve an extra instruction for the block terminator. */ + struct vkd3d_shader_instruction *ins = sm6_parser_require_space(sm6, block->instruction_count + 1); + + if (!ins) + return false; + + memcpy(ins, block->instructions, block->instruction_count * sizeof(*block->instructions)); + sm6->p.instructions.count += block->instruction_count; + return true; +} ```
Could we append that instruction here, instead of in the caller? Or as part of parsing the block, even?
``` From 239e51923cf60b33d4dddada3fadaaf7b769ed94 Mon Sep 17 00:00:00 2001 From: Conor McCarthy <cmccarthy(a)codeweavers.com> Date: Wed, 17 May 2023 16:43:26 +1000 Subject: [PATCH 4/6] vkd3d-shader/dxbc: Allow DXBC containers to have a zero checksum.
The linux build of DXC writes a zero checksum. ```
But as far as I'm aware, such shaders are rejected by native d3d12. What are we trying to do here, exactly? If it's purely about local development testing with the Linux build of DXC, I have a tool for inspecting and modifying DXBC blobs that I've been meaning to upstream. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/278#note_39299
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)