[PATCH v2 0/7] MR757: hlsl: Avoid HLSL_CLASS_OBJECT, part 2.
-- v2: vkd3d-shader/hlsl: Avoid using HLSL_CLASS_OBJECT without checking the base type. vkd3d-shader/hlsl: Consider any valid register reservation to invoke manual packing. tests: Add more tests for manual packing. vkd3d-shader/hlsl: Use hlsl_type_is_resource() for unbounded array checks. tests: Test HLSL unbounded array syntax. vkd3d-shader/hlsl: Add SM5.1 shader target strings. vkd3d-shader/hlsl: Move shader version helpers to hlsl.h. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 10 ++++++++++ libs/vkd3d-shader/hlsl.y | 22 ++++++---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index f5a38a29b..062e1b0ed 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -932,6 +932,16 @@ struct hlsl_ctx bool warn_implicit_truncation; }; +static inline bool hlsl_version_ge(const struct hlsl_ctx *ctx, unsigned int major, unsigned int minor) +{ + return ctx->profile->major_version > major || (ctx->profile->major_version == major && ctx->profile->minor_version >= minor); +} + +static inline bool hlsl_version_lt(const struct hlsl_ctx *ctx, unsigned int major, unsigned int minor) +{ + return !hlsl_version_ge(ctx, major, minor); +} + struct hlsl_resource_load_params { struct hlsl_type *format; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index e02e0c540..af00c84d9 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -939,16 +939,6 @@ static bool shader_is_sm_5_1(const struct hlsl_ctx *ctx) return ctx->profile->major_version == 5 && ctx->profile->minor_version >= 1; } -static bool shader_profile_version_ge(const struct hlsl_ctx *ctx, unsigned int major, unsigned int minor) -{ - return ctx->profile->major_version > major || (ctx->profile->major_version == major && ctx->profile->minor_version >= minor); -} - -static bool shader_profile_version_lt(const struct hlsl_ctx *ctx, unsigned int major, unsigned int minor) -{ - return !shader_profile_version_ge(ctx, major, minor); -} - static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields, struct hlsl_type *type, uint32_t modifiers, struct list *defs) { @@ -1216,7 +1206,7 @@ static struct hlsl_reg_reservation parse_packoffset(struct hlsl_ctx *ctx, const struct hlsl_reg_reservation reservation = {0}; char *endptr; - if (shader_profile_version_lt(ctx, 4, 0)) + if (hlsl_version_lt(ctx, 4, 0)) return reservation; reservation.offset_index = strtoul(reg_string + 1, &endptr, 10); @@ -3967,7 +3957,7 @@ static bool intrinsic_tex(struct hlsl_ctx *ctx, const struct parse_initializer * return false; } - if (shader_profile_version_ge(ctx, 4, 0)) + if (hlsl_version_ge(ctx, 4, 0)) { unsigned int count = hlsl_sampler_dim_count(dim); struct hlsl_ir_node *divisor; @@ -4014,7 +4004,7 @@ static bool intrinsic_tex(struct hlsl_ctx *ctx, const struct parse_initializer * return false; initialize_var_components(ctx, params->instrs, var, &idx, coords); - if (shader_profile_version_ge(ctx, 4, 0)) + if (hlsl_version_ge(ctx, 4, 0)) { if (!(half = hlsl_new_float_constant(ctx, 0.5f, loc))) return false; @@ -4200,7 +4190,7 @@ static bool intrinsic_d3dcolor_to_ubyte4(struct hlsl_ctx *ctx, if (!(ret = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, arg, c, loc))) return false; - if (shader_profile_version_ge(ctx, 4, 0)) + if (hlsl_version_ge(ctx, 4, 0)) return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_TRUNC, ret, loc); return true; @@ -6474,7 +6464,7 @@ type_no_void: { validate_texture_format_type(ctx, $3, &@3); - if (shader_profile_version_lt(ctx, 4, 1)) + if (hlsl_version_lt(ctx, 4, 1)) { hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Multisampled texture object declaration needs sample count for profile %s.", ctx->profile->name); @@ -6513,7 +6503,7 @@ type_no_void: $$ = hlsl_get_type(ctx->cur_scope, $1, true, true); if ($$->is_minimum_precision) { - if (shader_profile_version_lt(ctx, 4, 0)) + if (hlsl_version_lt(ctx, 4, 0)) { hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Target profile doesn't support minimum-precision types."); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 6 ++++++ libs/vkd3d-shader/tpf.c | 18 ++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 5638a03a8..531cacf83 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -3290,7 +3290,9 @@ const struct hlsl_profile_info *hlsl_get_target_info(const char *target) {"cs_4_0", VKD3D_SHADER_TYPE_COMPUTE, 4, 0, 0, 0, false}, {"cs_4_1", VKD3D_SHADER_TYPE_COMPUTE, 4, 1, 0, 0, false}, {"cs_5_0", VKD3D_SHADER_TYPE_COMPUTE, 5, 0, 0, 0, false}, + {"cs_5_1", VKD3D_SHADER_TYPE_COMPUTE, 5, 1, 0, 0, false}, {"ds_5_0", VKD3D_SHADER_TYPE_DOMAIN, 5, 0, 0, 0, false}, + {"ds_5_1", VKD3D_SHADER_TYPE_DOMAIN, 5, 1, 0, 0, false}, {"fx_2_0", VKD3D_SHADER_TYPE_EFFECT, 2, 0, 0, 0, false}, {"fx_4_0", VKD3D_SHADER_TYPE_EFFECT, 4, 0, 0, 0, false}, {"fx_4_1", VKD3D_SHADER_TYPE_EFFECT, 4, 1, 0, 0, false}, @@ -3298,7 +3300,9 @@ const struct hlsl_profile_info *hlsl_get_target_info(const char *target) {"gs_4_0", VKD3D_SHADER_TYPE_GEOMETRY, 4, 0, 0, 0, false}, {"gs_4_1", VKD3D_SHADER_TYPE_GEOMETRY, 4, 1, 0, 0, false}, {"gs_5_0", VKD3D_SHADER_TYPE_GEOMETRY, 5, 0, 0, 0, false}, + {"gs_5_1", VKD3D_SHADER_TYPE_GEOMETRY, 5, 1, 0, 0, false}, {"hs_5_0", VKD3D_SHADER_TYPE_HULL, 5, 0, 0, 0, false}, + {"hs_5_1", VKD3D_SHADER_TYPE_HULL, 5, 1, 0, 0, false}, {"ps.1.0", VKD3D_SHADER_TYPE_PIXEL, 1, 0, 0, 0, false}, {"ps.1.1", VKD3D_SHADER_TYPE_PIXEL, 1, 1, 0, 0, false}, {"ps.1.2", VKD3D_SHADER_TYPE_PIXEL, 1, 2, 0, 0, false}, @@ -3326,6 +3330,7 @@ const struct hlsl_profile_info *hlsl_get_target_info(const char *target) {"ps_4_0_level_9_3", VKD3D_SHADER_TYPE_PIXEL, 4, 0, 9, 3, false}, {"ps_4_1", VKD3D_SHADER_TYPE_PIXEL, 4, 1, 0, 0, false}, {"ps_5_0", VKD3D_SHADER_TYPE_PIXEL, 5, 0, 0, 0, false}, + {"ps_5_1", VKD3D_SHADER_TYPE_PIXEL, 5, 1, 0, 0, false}, {"tx_1_0", VKD3D_SHADER_TYPE_TEXTURE, 1, 0, 0, 0, false}, {"vs.1.0", VKD3D_SHADER_TYPE_VERTEX, 1, 0, 0, 0, false}, {"vs.1.1", VKD3D_SHADER_TYPE_VERTEX, 1, 1, 0, 0, false}, @@ -3347,6 +3352,7 @@ const struct hlsl_profile_info *hlsl_get_target_info(const char *target) {"vs_4_0_level_9_3", VKD3D_SHADER_TYPE_VERTEX, 4, 0, 9, 3, false}, {"vs_4_1", VKD3D_SHADER_TYPE_VERTEX, 4, 1, 0, 0, false}, {"vs_5_0", VKD3D_SHADER_TYPE_VERTEX, 5, 0, 0, 0, false}, + {"vs_5_1", VKD3D_SHADER_TYPE_VERTEX, 5, 1, 0, 0, false}, }; for (i = 0; i < ARRAY_SIZE(profiles); ++i) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 4d0658313..a66090001 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3385,10 +3385,10 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) if (profile->major_version >= 5) { - put_u32(&buffer, TAG_RD11); + put_u32(&buffer, hlsl_version_ge(ctx, 5, 1) ? TAG_RD11_REVERSE : TAG_RD11); put_u32(&buffer, 15 * sizeof(uint32_t)); /* size of RDEF header including this header */ put_u32(&buffer, 6 * sizeof(uint32_t)); /* size of buffer desc */ - put_u32(&buffer, 8 * sizeof(uint32_t)); /* size of binding desc */ + put_u32(&buffer, (hlsl_version_ge(ctx, 5, 1) ? 10 : 8) * sizeof(uint32_t)); /* size of binding desc */ put_u32(&buffer, 10 * sizeof(uint32_t)); /* size of variable desc */ put_u32(&buffer, 9 * sizeof(uint32_t)); /* size of type desc */ put_u32(&buffer, 3 * sizeof(uint32_t)); /* size of member desc */ @@ -3405,6 +3405,9 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) const struct extern_resource *resource = &extern_resources[i]; uint32_t flags = 0; + if (hlsl_version_ge(ctx, 5, 1)) + hlsl_fixme(ctx, &resource->var->loc, "Shader model 5.1 resource reflection."); + if (resource->is_user_packed) flags |= D3D_SIF_USERPACKED; @@ -3437,6 +3440,9 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) if (!cbuffer->reg.allocated) continue; + if (hlsl_version_ge(ctx, 5, 1)) + hlsl_fixme(ctx, &cbuffer->loc, "Shader model 5.1 resource reflection."); + if (cbuffer->reservation.reg_type) flags |= D3D_SIF_USERPACKED; @@ -5808,13 +5814,21 @@ static void write_sm4_shdr(struct hlsl_ctx *ctx, LIST_FOR_EACH_ENTRY(cbuffer, &ctx->buffers, struct hlsl_buffer, entry) { if (cbuffer->reg.allocated) + { + if (hlsl_version_ge(ctx, 5, 1)) + hlsl_fixme(ctx, &cbuffer->loc, "Shader model 5.1 resource definition."); + write_sm4_dcl_constant_buffer(&tpf, cbuffer); + } } for (i = 0; i < extern_resources_count; ++i) { const struct extern_resource *resource = &extern_resources[i]; + if (hlsl_version_ge(ctx, 5, 1)) + hlsl_fixme(ctx, &resource->var->loc, "Shader model 5.1 resource declaration."); + if (resource->regset == HLSL_REGSET_SAMPLERS) write_sm4_dcl_samplers(&tpf, resource); else if (resource->regset == HLSL_REGSET_TEXTURES) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/unbounded-array-5.1.shader_test | 38 ++++++++++++++++++++++ tests/shader_runner.c | 1 + 3 files changed, 40 insertions(+) create mode 100644 tests/hlsl/unbounded-array-5.1.shader_test diff --git a/Makefile.am b/Makefile.am index 68e8642e0..39b041502 100644 --- a/Makefile.am +++ b/Makefile.am @@ -214,6 +214,7 @@ vkd3d_shader_tests = \ tests/hlsl/uav-rwbyteaddressbuffer.shader_test \ tests/hlsl/uav-rwstructuredbuffer.shader_test \ tests/hlsl/uav-rwtexture.shader_test \ + tests/hlsl/unbounded-array-5.1.shader_test \ tests/hlsl/uniform-parameters.shader_test \ tests/hlsl/uniform-semantics.shader_test \ tests/hlsl/vector-indexing-uniform.shader_test \ diff --git a/tests/hlsl/unbounded-array-5.1.shader_test b/tests/hlsl/unbounded-array-5.1.shader_test new file mode 100644 index 000000000..766da9649 --- /dev/null +++ b/tests/hlsl/unbounded-array-5.1.shader_test @@ -0,0 +1,38 @@ +[require] +shader model >= 5.1 +shader model < 6.0 +options: unbounded-descriptor-arrays + +[pixel shader todo] +Texture2D t[]; +SamplerState s[][2]; +uniform uint u, v; + +float4 main() : sv_target +{ + return t[u].Sample(s[1][v], 0); +} + + +[pixel shader fail] +SamplerState s[][]; + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +SamplerState s[2][]; +float4 main() : sv_target {return 0;} + + +[pixel shader fail] +float f[]; +float4 main() : sv_target {return 0;} + + +[pixel shader fail todo] +STRING s[]; +float4 main() : sv_target {return 0;} diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 5eafbcbe4..f800ded32 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -311,6 +311,7 @@ static void parse_require_directive(struct shader_runner *runner, const char *li { D3DCOMPILE_PACK_MATRIX_ROW_MAJOR, "row-major" }, { D3DCOMPILE_PACK_MATRIX_COLUMN_MAJOR, "column-major" }, { D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY, "backcompat" }, + { D3DCOMPILE_ENABLE_UNBOUNDED_DESCRIPTOR_TABLES, "unbounded-descriptor-arrays" }, }; runner->compile_options = 0; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> Not all objects can be unbounded descriptors. --- libs/vkd3d-shader/hlsl.y | 4 ++-- tests/hlsl/unbounded-array-5.1.shader_test | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index af00c84d9..e6a4f13d4 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -961,7 +961,7 @@ static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields, field->type = type; - if (shader_is_sm_5_1(ctx) && type->class == HLSL_CLASS_OBJECT) + if (shader_is_sm_5_1(ctx) && hlsl_type_is_resource(type)) { for (k = 0; k < v->arrays.count; ++k) unbounded_res_array |= (v->arrays.sizes[k] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT); @@ -2167,7 +2167,7 @@ static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v) type = basic_type; - if (shader_is_sm_5_1(ctx) && type->class == HLSL_CLASS_OBJECT) + if (shader_is_sm_5_1(ctx) && hlsl_type_is_resource(type)) { for (i = 0; i < v->arrays.count; ++i) unbounded_res_array |= (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT); diff --git a/tests/hlsl/unbounded-array-5.1.shader_test b/tests/hlsl/unbounded-array-5.1.shader_test index 766da9649..6aaea80a0 100644 --- a/tests/hlsl/unbounded-array-5.1.shader_test +++ b/tests/hlsl/unbounded-array-5.1.shader_test @@ -33,6 +33,6 @@ float f[]; float4 main() : sv_target {return 0;} -[pixel shader fail todo] +[pixel shader fail] STRING s[]; float4 main() : sv_target {return 0;} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> --- tests/hlsl/cbuffer.shader_test | 61 +++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/tests/hlsl/cbuffer.shader_test b/tests/hlsl/cbuffer.shader_test index 2140e1b88..08ada8c73 100644 --- a/tests/hlsl/cbuffer.shader_test +++ b/tests/hlsl/cbuffer.shader_test @@ -759,19 +759,71 @@ float4 main() : sv_target return 0; } -% Using register() alone is not considered manual packing for non-resources. +% register(c#) is ignored and does not count as manual packing. [pixel shader] cbuffer buffer { - float4 foo : register(c0); + float4 foo : register(c1); Texture2D tex; } +float4 main() : sv_target +{ + return foo; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 float4 0.0 0.0 0.0 0.0 +todo(glsl) draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0) + +[pixel shader fail(sm<6)] +cbuffer buffer +{ + float4 foo : register(c1); + Texture2D tex : register(t1); +} + +float4 main() : sv_target +{ + return foo; +} + +% However, valid register reservations count as manual packing even if they +% don't make sense for the given variable. +% sm6 just rejects incorrect reservations. + +[pixel shader fail(sm>=6)] +cbuffer buffer +{ + float4 bar : register(t1); +} + float4 main() : sv_target { return 0; } +[pixel shader todo fail(sm>=6)] +cbuffer buffer +{ + float4 foo : packoffset(c0); + float4 bar : register(t1); + Texture2D tex : register(s2); +} + +float4 main() : sv_target +{ + return foo + bar; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 float4 0.1 0.2 0.3 0.4 +todo draw quad +probe all rgba (1.1, 2.2, 3.3, 4.4) + [require] shader model >= 5.0 @@ -780,7 +832,7 @@ shader model >= 5.0 size (2d, 1, 1) 0.0 0.0 0.0 0.5 -[pixel shader] +[pixel shader todo] struct apple { float2 a; @@ -792,6 +844,7 @@ cbuffer buffer { float4 foo : packoffset(c3); struct apple bar : packoffset(c1); + struct apple baz : register(u3); } float4 main() : sv_target @@ -804,5 +857,5 @@ uniform 0 float4 0.0 1.0 2.0 3.0 uniform 4 float4 4.0 5.0 6.0 7.0 uniform 8 float4 8.0 9.0 10.0 11.0 uniform 12 float4 12.0 13.0 14.0 15.0 -todo(glsl) draw quad +todo(sm<6) draw quad probe all rgba (124.0, 135.0, 146.0, 150.5) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> Regardless of the type of the variable. --- libs/vkd3d-shader/hlsl_codegen.c | 4 +++- tests/hlsl/cbuffer.shader_test | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 6f2de9376..aa3e7f3a8 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -4790,7 +4790,9 @@ static void validate_buffer_offsets(struct hlsl_ctx *ctx) continue; if (var1->reg_reservation.offset_type - || (var1->data_type->class == HLSL_CLASS_OBJECT && var1->reg_reservation.reg_type)) + || var1->reg_reservation.reg_type == 's' + || var1->reg_reservation.reg_type == 't' + || var1->reg_reservation.reg_type == 'u') buffer->manually_packed_elements = true; else buffer->automatically_packed_elements = true; diff --git a/tests/hlsl/cbuffer.shader_test b/tests/hlsl/cbuffer.shader_test index 08ada8c73..ece13c4e7 100644 --- a/tests/hlsl/cbuffer.shader_test +++ b/tests/hlsl/cbuffer.shader_test @@ -805,7 +805,7 @@ float4 main() : sv_target return 0; } -[pixel shader todo fail(sm>=6)] +[pixel shader fail(sm>=6)] cbuffer buffer { float4 foo : packoffset(c0); @@ -821,7 +821,7 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 0.1 0.2 0.3 0.4 -todo draw quad +todo(glsl) draw quad probe all rgba (1.1, 2.2, 3.3, 4.4) @@ -832,7 +832,7 @@ shader model >= 5.0 size (2d, 1, 1) 0.0 0.0 0.0 0.5 -[pixel shader todo] +[pixel shader] struct apple { float2 a; @@ -857,5 +857,5 @@ uniform 0 float4 0.0 1.0 2.0 3.0 uniform 4 float4 4.0 5.0 6.0 7.0 uniform 8 float4 8.0 9.0 10.0 11.0 uniform 12 float4 12.0 13.0 14.0 15.0 -todo(sm<6) draw quad +todo(glsl) draw quad probe all rgba (124.0, 135.0, 146.0, 150.5) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
From: Zebediah Figura <zfigura(a)codeweavers.com> As the diffstat shows, HLSL_CLASS_OBJECT does not really have much in common. Resource types (TEXTURE, SAMPLER, UAV) sometimes behave similarly to each other, but do not generally behave similarly to effect-specific types (string, shader, state, view). Most consumers of HLSL_CLASS_OBJECT subsequently check the base type anyway. Hence we want to replace HLSL_TYPE_* with individual classes for object types. As a first step, change the last few places that only check HLSL_CLASS_OBJECT. --- libs/vkd3d-shader/d3dbc.c | 12 ++---------- libs/vkd3d-shader/hlsl.c | 5 ++++- libs/vkd3d-shader/hlsl_codegen.c | 4 ++-- libs/vkd3d-shader/tpf.c | 12 +++--------- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 099729fbb..97650942e 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -2572,19 +2572,11 @@ static void write_sm1_instructions(struct hlsl_ctx *ctx, struct vkd3d_bytecode_b { if (instr->data_type) { - if (instr->data_type->class == HLSL_CLASS_MATRIX) + if (instr->data_type->class != HLSL_CLASS_SCALAR && instr->data_type->class != HLSL_CLASS_VECTOR) { - /* These need to be lowered. */ - hlsl_fixme(ctx, &instr->loc, "SM1 matrix expression."); - continue; - } - else if (instr->data_type->class == HLSL_CLASS_OBJECT) - { - hlsl_fixme(ctx, &instr->loc, "Object copy."); + hlsl_fixme(ctx, &instr->loc, "Class %#x should have been lowered or removed.", instr->data_type->class); break; } - - assert(instr->data_type->class == HLSL_CLASS_SCALAR || instr->data_type->class == HLSL_CLASS_VECTOR); } switch (instr->type) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 531cacf83..0f9e89ea6 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1570,7 +1570,10 @@ bool hlsl_index_is_noncontiguous(struct hlsl_ir_index *index) bool hlsl_index_is_resource_access(struct hlsl_ir_index *index) { - return index->val.node->data_type->class == HLSL_CLASS_OBJECT; + const struct hlsl_type *type = index->val.node->data_type; + + return type->class == HLSL_CLASS_OBJECT + && (type->base_type == HLSL_TYPE_TEXTURE || type->base_type == HLSL_TYPE_UAV); } bool hlsl_index_chain_has_resource_access(struct hlsl_ir_index *index) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index aa3e7f3a8..465571b3e 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1565,7 +1565,7 @@ static bool copy_propagation_replace_with_single_instr(struct hlsl_ctx *ctx, var->name, start, start + count, debug_hlsl_swizzle(swizzle, instr_component_count), new_instr, debug_hlsl_swizzle(ret_swizzle, instr_component_count)); - if (instr->data_type->class != HLSL_CLASS_OBJECT) + if (new_instr->data_type->class == HLSL_CLASS_SCALAR || new_instr->data_type->class == HLSL_CLASS_VECTOR) { struct hlsl_ir_node *swizzle_node; @@ -1742,7 +1742,7 @@ static void copy_propagation_record_store(struct hlsl_ctx *ctx, struct hlsl_ir_s { unsigned int writemask = store->writemask; - if (store->rhs.node->data_type->class == HLSL_CLASS_OBJECT) + if (!hlsl_is_numeric_type(store->rhs.node->data_type)) writemask = VKD3DSP_WRITEMASK_0; copy_propagation_set_value(ctx, var_def, start, writemask, store->rhs.node, store->node.index); } diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index a66090001..f0f9088a9 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -5706,18 +5706,12 @@ static void write_sm4_block(const struct tpf_writer *tpf, const struct hlsl_bloc { if (instr->data_type) { - if (instr->data_type->class == HLSL_CLASS_MATRIX) + if (instr->data_type->class != HLSL_CLASS_SCALAR && instr->data_type->class != HLSL_CLASS_VECTOR) { - hlsl_fixme(tpf->ctx, &instr->loc, "Matrix operations need to be lowered."); + hlsl_fixme(tpf->ctx, &instr->loc, "Class %#x should have been lowered or removed.", + instr->data_type->class); break; } - else if (instr->data_type->class == HLSL_CLASS_OBJECT) - { - hlsl_fixme(tpf->ctx, &instr->loc, "Object copy."); - break; - } - - assert(instr->data_type->class == HLSL_CLASS_SCALAR || instr->data_type->class == HLSL_CLASS_VECTOR); if (!instr->reg.allocated) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757
This is failing to validate sm6 -> spirv on the last test in cbuffer.shader_test, but it doesn't fail locally, which is confusing. I don't suppose anyone has a hint? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66868
This is failing to validate sm6 -\> spirv on the last test in cbuffer.shader_test, but it doesn't fail locally, which is confusing. I don't suppose anyone has a hint?
Without knowing your exact local setup, SPIR-V validation depends on both building vkd3d-shader against SPIRV-Tools and having VKD3D_SHADER_CONFIG=force_validation in the environment when running the tests. Do you have both of those? My other guess would be subtle differences in the versions of SPIRV-Tools and/or dxcompiler between what you have locally and what's on the CI. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66890
Giovanni Mascellani (@giomasce) commented about tests/hlsl/unbounded-array-5.1.shader_test:
+[require] +shader model >= 5.1 +shader model < 6.0 +options: unbounded-descriptor-arrays + +[pixel shader todo] +Texture2D t[]; +SamplerState s[][2]; +uniform uint u, v; + +float4 main() : sv_target +{ + return t[u].Sample(s[1][v], 0); +} We've already discussed this in the past and I don't you don't necessarily agree, but allow me to at least ask for actually running this code, just to have an idea we're generating code at least vaguely related to the intended semantics.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66928
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
field->type = type;
- if (shader_is_sm_5_1(ctx) && type->class == HLSL_CLASS_OBJECT) + if (shader_is_sm_5_1(ctx) && hlsl_type_is_resource(type))
In light of 4577cefc9c3ed3f939a2db8b8e6d766687fa2b0e you might want to share the definition of `shader_is_sm_5_1()`. We have to different definitions which are slightly different, and on of those doesn't feel completely right. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66929
Yeah, I can see the SPIR-V validation error with SPIRV-Tools and `force_validation`: ``` vkd3d:3746511:trace:spirv_compiler_generate_spirv Structure id 45 decorated as Block for variable in PushConstant storage class must follow standard storage buffer layout rules: member 1 at offset 64 overlaps previous member ending at offset 79 vkd3d:3746511:trace:spirv_compiler_generate_spirv %push_cb_struct = OpTypeStruct %_arr_v4float_uint_5 %_arr_uint_uint_1 ``` This warning seems related too: ``` vkd3d:3746511:warn:spirv_compiler_emit_cbv_declaration Constant buffer size 76 exceeds push constant size 64. ``` I never read into detail our code to allocate constant buffers in SPIR-V, but problems related to the push constants are popping every now and then and make me suspect that that part needs some revising. Notice also the Mesa reduced `maxPushConstantsSize` to 128 for Intel GPUs some time ago, which is often less than we assume is available. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66930
On Thu Apr 4 13:50:03 2024 +0000, Giovanni Mascellani wrote:
We've already discussed this in the past and I don't you don't necessarily agree, but allow me to at least ask for actually running this code, just to have an idea we're generating code at least vaguely related to the intended semantics. It's annoying in this case because I need a way to pass sampler arrays. I'll just get rid of this test and alter the existing tests in d3d12.c to include multidimensional arrays.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66949
On Thu Apr 4 13:50:04 2024 +0000, Giovanni Mascellani wrote:
In light of 4577cefc9c3ed3f939a2db8b8e6d766687fa2b0e you might want to share the definition of `shader_is_sm_5_1()`. We have to different definitions which are slightly different, and on of those doesn't feel completely right. I'll add a patch for that.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66950
On Thu Apr 4 15:56:08 2024 +0000, Henri Verbeet wrote:
This is failing to validate sm6 -\> spirv on the last test in cbuffer.shader_test, but it doesn't fail locally, which is confusing. I don't suppose anyone has a hint? Without knowing your exact local setup, SPIR-V validation depends on both building vkd3d-shader against SPIRV-Tools and having VKD3D_SHADER_CONFIG=force_validation in the environment when running the tests. Do you have both of those? My other guess would be subtle differences in the versions of SPIRV-Tools and/or dxcompiler between what you have locally and what's on the CI. Ah, I didn't realize I need that config variable on. I even looked in the CI script for something relevant; I guess I overlooked it.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66951
It's a slightly obscure failure that happens when we pass less uniforms to the shader than it asks for. The validation failure is because vkd3d puts an internal uniform after the count in the root signature, but the count comes from the shader runner. We don't currently have a good way to get the required count from the shader via reflection. We should indeed probably stop using push constants, but I'll leave that to a separate patch. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/757#note_66961
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)