[PATCH v6 0/5] MR498: vkd3d-shader/dxil: Implement the DXIL ALLOCA and STORE instructions.
-- v6: tests/shader-runner: Add a non-const-indexing test for asfloat() result storage. vkd3d-shader/spirv: Bitcast if necessary in the spirv_compiler_emit_mov() general implementation. vkd3d-shader/dxil: Implement the DXIL STORE instruction. vkd3d-shader/dxil: Implement the DXIL ALLOCA instruction. tests/shader-runner: Test an uninitialised indexable temp. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/hlsl/non-const-indexing.shader_test | 57 +++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/hlsl/non-const-indexing.shader_test b/tests/hlsl/non-const-indexing.shader_test index 5c8e1db4a..00fea3b38 100644 --- a/tests/hlsl/non-const-indexing.shader_test +++ b/tests/hlsl/non-const-indexing.shader_test @@ -194,3 +194,60 @@ uniform 8 int 3 uniform 9 int 4 todo(sm>=6) draw quad todo(sm>=6) probe all rgba (1126, 3344, 5566, 3788) + + +[pixel shader] +uniform float4 f[4]; +uniform uint4 u; +uniform uint4 v; + +float4 main() : sv_target +{ + float temp[4]; + temp[0] = f[u.x].x; + temp[1] = f[u.y].x; + temp[2] = f[u.z].x; + temp[3] = f[u.w].x; + return float4(temp[v.x], temp[v.y], temp[v.z], temp[v.w]); +} + +% FXC is incapable of compiling this correctly, but results differ for SM1-3 vs SM4-5. +[require] +shader model < 4.0 + +[test] +uniform 0 float 1.0 +uniform 4 float 2.0 +uniform 8 float 3.0 +uniform 12 float 4.0 +uniform 16 uint4 3 1 0 2 +uniform 20 uint4 0 3 1 2 +draw quad +todo probe all rgba (1.0, 1.0, 1.0, 1.0) + +[require] +shader model >= 4.0 +shader model < 6.0 + +[test] +uniform 0 float 1.0 +uniform 4 float 2.0 +uniform 8 float 3.0 +uniform 12 float 4.0 +uniform 16 uint4 3 1 0 2 +uniform 20 uint4 0 3 1 2 +draw quad +todo probe all rgba (4.0, 4.0, 4.0, 4.0) + +[require] +shader model >= 6.0 + +[test] +uniform 0 float 1.0 +uniform 4 float 2.0 +uniform 8 float 3.0 +uniform 12 float 4.0 +uniform 16 uint4 3 1 0 2 +uniform 20 uint4 0 3 1 2 +todo(sm>=6) draw quad +todo(sm>=6) probe all rgba (4.0, 3.0, 2.0, 1.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 90 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index a1065c8bc..84b7af794 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -29,6 +29,9 @@ static const uint64_t MAX_ALIGNMENT_EXPONENT = 29; static const uint64_t GLOBALVAR_FLAG_IS_CONSTANT = 1; static const uint64_t GLOBALVAR_FLAG_EXPLICIT_TYPE = 2; static const unsigned int GLOBALVAR_ADDRESS_SPACE_SHIFT = 2; +static const uint64_t ALLOCA_FLAG_IN_ALLOCA = 0x20; +static const uint64_t ALLOCA_FLAG_EXPLICIT_TYPE = 0x40; +static const uint64_t ALLOCA_ALIGNMENT_MASK = ALLOCA_FLAG_IN_ALLOCA - 1; static const unsigned int SHADER_DESCRIPTOR_TYPE_COUNT = 4; static const unsigned int dx_max_thread_group_size[3] = {1024, 1024, 64}; @@ -2688,12 +2691,15 @@ static void sm6_parser_declare_icb(struct sm6_parser *sm6, const struct sm6_type } static void sm6_parser_declare_indexable_temp(struct sm6_parser *sm6, const struct sm6_type *elem_type, - unsigned int count, unsigned int alignment, unsigned int init, struct sm6_value *dst) + unsigned int count, unsigned int alignment, unsigned int init, struct vkd3d_shader_instruction *ins, + struct sm6_value *dst) { enum vkd3d_data_type data_type = vkd3d_data_type_from_sm6_type(elem_type); - struct vkd3d_shader_instruction *ins; - ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_INDEXABLE_TEMP); + if (ins) + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_DCL_INDEXABLE_TEMP); + else + ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_INDEXABLE_TEMP); ins->declaration.indexable_temp.register_idx = sm6->indexable_temp_count++; ins->declaration.indexable_temp.register_size = count; ins->declaration.indexable_temp.alignment = alignment; @@ -2826,7 +2832,7 @@ static bool sm6_parser_declare_global(struct sm6_parser *sm6, const struct dxil_ if (is_constant) sm6_parser_declare_icb(sm6, scalar_type, count, alignment, init, dst); else - sm6_parser_declare_indexable_temp(sm6, scalar_type, count, alignment, init, dst); + sm6_parser_declare_indexable_temp(sm6, scalar_type, count, alignment, init, NULL, dst); } else if (address_space == ADDRESS_SPACE_GROUPSHARED) { @@ -3071,6 +3077,79 @@ static struct sm6_block *sm6_block_create() return block; } +static void sm6_parser_emit_alloca(struct sm6_parser *sm6, const struct dxil_record *record, + struct vkd3d_shader_instruction *ins, struct sm6_value *dst) +{ + const struct sm6_type *type[2], *elem_type; + const struct sm6_value *size; + unsigned int i, alignment; + uint64_t packed_operands; + + if (!dxil_record_validate_operand_count(record, 4, 4, sm6)) + return; + + for (i = 0; i < 2; ++i) + { + if (!(type[i] = sm6_parser_get_type(sm6, record->operands[i]))) + return; + } + + packed_operands = record->operands[3]; + if (packed_operands & ALLOCA_FLAG_IN_ALLOCA) + WARN("Ignoring in_alloca flag.\n"); + if (!(packed_operands & ALLOCA_FLAG_EXPLICIT_TYPE)) + { + FIXME("Unhandled implicit type.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Implicit result type for ALLOCA instructions is not supported."); + return; + } + packed_operands &= ~(ALLOCA_FLAG_IN_ALLOCA | ALLOCA_FLAG_EXPLICIT_TYPE); + + if (!sm6_type_is_array(type[0]) || !sm6_type_is_numeric(elem_type = type[0]->u.array.elem_type)) + { + WARN("Type is not a numeric array.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Result type of an ALLOCA instruction is not a numeric array."); + return; + } + if (!sm6_type_is_integer(type[1])) + { + WARN("Size operand type is not scalar integer.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Size operand type of an ALLOCA instruction is not scalar integer."); + return; + } + + if (!(dst->type = sm6_type_get_pointer_to_type(type[0], ADDRESS_SPACE_DEFAULT, sm6))) + { + WARN("Failed to get pointer type for type class %u.\n", type[0]->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE, + "Module does not define a pointer type for an ALLOCA instruction."); + return; + } + + if (!(size = sm6_parser_get_value_safe(sm6, record->operands[2]))) + return; + /* A size of 1 means one instance of type[0], i.e. one array. */ + if (sm6_value_get_constant_uint(size) != 1) + { + FIXME("Allocation size is not 1.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "ALLOCA instruction allocation sizes other than 1 are not supported."); + return; + } + + if (!bitcode_parse_alignment(packed_operands & ALLOCA_ALIGNMENT_MASK, &alignment)) + WARN("Invalid alignment %"PRIu64".\n", packed_operands); + packed_operands &= ~ALLOCA_ALIGNMENT_MASK; + + if (packed_operands) + WARN("Ignoring flags %#"PRIx64".\n", packed_operands); + + sm6_parser_declare_indexable_temp(sm6, elem_type, type[0]->u.array.count, alignment, 0, ins, dst); +} + static enum vkd3d_shader_opcode map_binary_op(uint64_t code, const struct sm6_type *type_a, const struct sm6_type *type_b, struct sm6_parser *sm6) { @@ -4630,6 +4709,9 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const record = block->records[i]; switch (record->code) { + case FUNC_CODE_INST_ALLOCA: + sm6_parser_emit_alloca(sm6, record, ins, dst); + break; case FUNC_CODE_INST_BINOP: sm6_parser_emit_binop(sm6, record, ins, dst); break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 60 ++++++++++++++++++++++- tests/hlsl/expr-indexing.shader_test | 8 +-- tests/hlsl/non-const-indexing.shader_test | 20 ++++---- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 84b7af794..66301b301 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2860,17 +2860,19 @@ static const struct vkd3d_shader_immediate_constant_buffer *resolve_forward_init assert(index); --index; - if (!(value = sm6_parser_get_value_safe(sm6, index)) || !sm6_value_is_icb(value)) + if (!(value = sm6_parser_get_value_safe(sm6, index)) || (!sm6_value_is_icb(value) && !sm6_value_is_undef(value))) { WARN("Invalid initialiser index %zu.\n", index); vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, "Global variable initialiser value index %zu is invalid.", index); return NULL; } - else + else if (sm6_value_is_icb(value)) { return value->u.icb; } + /* In VSIR, initialisation with undefined values of objects is implied, not explicit. */ + return NULL; } static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) @@ -4332,6 +4334,57 @@ static void sm6_parser_emit_ret(struct sm6_parser *sm6, const struct dxil_record ins->handler_idx = VKD3DSIH_NOP; } +static void sm6_parser_emit_store(struct sm6_parser *sm6, const struct dxil_record *record, + struct vkd3d_shader_instruction *ins, struct sm6_value *dst) +{ + struct vkd3d_shader_src_param *src_param; + struct vkd3d_shader_dst_param *dst_param; + const struct sm6_type *pointee_type; + const struct sm6_value *ptr, *src; + unsigned int i = 0, alignment; + uint64_t alignment_code; + + if (!(ptr = sm6_parser_get_value_by_ref(sm6, record, NULL, &i)) + || !sm6_value_validate_is_register(ptr, sm6) + || !sm6_value_validate_is_pointer(ptr, sm6)) + { + return; + } + + pointee_type = ptr->type->u.pointer.type; + if (!(src = sm6_parser_get_value_by_ref(sm6, record, pointee_type, &i))) + return; + if (!sm6_value_validate_is_numeric(src, sm6)) + return; + + if (pointee_type != src->type) + { + WARN("Type mismatch.\n"); + vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_TYPE_MISMATCH, + "Type mismatch in pointer store arguments."); + } + + if (!dxil_record_validate_operand_count(record, i + 2, i + 2, sm6)) + return; + + alignment_code = record->operands[i++]; + if (!bitcode_parse_alignment(alignment_code, &alignment)) + WARN("Invalid alignment %"PRIu64".\n", alignment_code); + + if (record->operands[i]) + WARN("Ignoring volatile modifier.\n"); + + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); + + src_param = instruction_src_params_alloc(ins, 1, sm6); + src_param_init_from_value(&src_param[0], src); + + dst_param = instruction_dst_params_alloc(ins, 1, sm6); + dst_param_init(dst_param); + dst_param->reg = ptr->u.reg; + dst_param->reg.alignment = alignment; +} + static void sm6_parser_emit_vselect(struct sm6_parser *sm6, const struct dxil_record *record, struct vkd3d_shader_instruction *ins, struct sm6_value *dst) { @@ -4738,6 +4791,9 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const is_terminator = true; ret_found = true; break; + case FUNC_CODE_INST_STORE: + sm6_parser_emit_store(sm6, record, ins, dst); + break; case FUNC_CODE_INST_VSELECT: sm6_parser_emit_vselect(sm6, record, ins, dst); break; diff --git a/tests/hlsl/expr-indexing.shader_test b/tests/hlsl/expr-indexing.shader_test index c11fa6540..3dcc5727e 100644 --- a/tests/hlsl/expr-indexing.shader_test +++ b/tests/hlsl/expr-indexing.shader_test @@ -26,7 +26,7 @@ float4 main() : sv_target uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float 2 -todo(sm>=6) draw quad +draw quad probe all rgba (10.0, 10.0, 10.0, 10.0) @@ -56,10 +56,10 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float 0 -todo(sm>=6) draw quad +draw quad probe all rgba (4.0, 4.0, 4.0, 4.0) uniform 4 float 2 -todo(sm>=6) draw quad +draw quad probe all rgba (1.0, 1.0, 1.0, 1.0) @@ -99,5 +99,5 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float 1 -todo(sm>=6) draw quad +draw quad probe all rgba (2.0, 2.0, 2.0, 2.0) diff --git a/tests/hlsl/non-const-indexing.shader_test b/tests/hlsl/non-const-indexing.shader_test index 00fea3b38..e7b814d37 100644 --- a/tests/hlsl/non-const-indexing.shader_test +++ b/tests/hlsl/non-const-indexing.shader_test @@ -155,16 +155,16 @@ float4 main() : sv_target [test] uniform 0 int 0 uniform 1 int 0 -todo(sm>=6) draw quad -todo(sm>=6) probe all rgba (100, 6, 7, 8) +draw quad +probe all rgba (100, 6, 7, 8) uniform 0 int 2 uniform 1 int 2 -todo(sm>=6) draw quad -todo(sm>=6) probe all rgba (5, 6, 100, 8) +draw quad +probe all rgba (5, 6, 100, 8) uniform 0 int 1 uniform 1 int 3 -todo(sm>=6) draw quad -todo(sm>=6) probe all rgba (5, 6, 7, 4) +draw quad +probe all rgba (5, 6, 7, 4) [pixel shader] @@ -192,8 +192,8 @@ uniform 0 float4 1 2 3 4 uniform 4 float4 5 6 7 8 uniform 8 int 3 uniform 9 int 4 -todo(sm>=6) draw quad -todo(sm>=6) probe all rgba (1126, 3344, 5566, 3788) +draw quad +probe all rgba (1126, 3344, 5566, 3788) [pixel shader] @@ -249,5 +249,5 @@ uniform 8 float 3.0 uniform 12 float 4.0 uniform 16 uint4 3 1 0 2 uniform 20 uint4 0 3 1 2 -todo(sm>=6) draw quad -todo(sm>=6) probe all rgba (4.0, 3.0, 2.0, 1.0) +draw quad +probe all rgba (4.0, 3.0, 2.0, 1.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
From: Conor McCarthy <cmccarthy(a)codeweavers.com> In SM 6, this is needed when storing an asfloat() or asuint() result in an indexable temp, because dxc performs the bitcast by casting the destination pointer. --- libs/vkd3d-shader/spirv.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index dc3d96313..de9776b2a 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -6942,6 +6942,11 @@ static void spirv_compiler_emit_mov(struct spirv_compiler *compiler, general_implementation: val_id = spirv_compiler_emit_load_src(compiler, src, dst->write_mask); + if (dst->reg.data_type != src->reg.data_type) + { + val_id = vkd3d_spirv_build_op_bitcast(builder, vkd3d_spirv_get_type_id_for_data_type(builder, + dst->reg.data_type, vkd3d_write_mask_component_count(dst->write_mask)), val_id); + } spirv_compiler_emit_store_dst(compiler, dst, val_id); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- tests/hlsl/non-const-indexing.shader_test | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/hlsl/non-const-indexing.shader_test b/tests/hlsl/non-const-indexing.shader_test index e7b814d37..82a6e321e 100644 --- a/tests/hlsl/non-const-indexing.shader_test +++ b/tests/hlsl/non-const-indexing.shader_test @@ -196,6 +196,26 @@ draw quad probe all rgba (1126, 3344, 5566, 3788) +[pixel shader] +uint i, j; + +float4 main() : sv_target +{ + float mut1[4] = {1, 2, 3, 4}; + + /* dxc emits a pointer bitcast, which results in a VSIR MOV with mismatched data types. */ + mut1[i] = asfloat(j); + + return float4(mut1[0], mut1[1], mut1[2], mut1[3]); +} + +[test] +uniform 0 uint 1 +uniform 1 uint 0x40a00000 +draw quad +probe all rgba (1, 5, 3, 4) + + [pixel shader] uniform float4 f[4]; uniform uint4 u; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
+ { + FIXME("Unhandled implicit type.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Implicit result type for ALLOCA instructions is not supported."); + return; + } + packed_operands &= ~(ALLOCA_FLAG_IN_ALLOCA | ALLOCA_FLAG_EXPLICIT_TYPE); + + if (!sm6_type_is_array(type[0]) || !sm6_type_is_numeric(elem_type = type[0]->u.array.elem_type)) + { + WARN("Type is not a numeric array.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Result type of an ALLOCA instruction is not a numeric array."); + return; + } + if (!sm6_type_is_integer(type[1])) What is `type[1]` supposed to mean? It doesn't seem to be used anywhere. Maybe you can add a comment?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498#note_55687
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/498
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)