[PATCH v2 0/5] MR98: hlsl: Matrix majority, 2023 edition.
-- v2: vkd3d-shader/hlsl: Avoid warning for each variable when in/out modifiers are specified on non-parameters. vkd3d-shader/hlsl: Remove a redundant warning for mutually exclusive majority modifiers. vkd3d-shader/hlsl: Check for majority modifiers on non-matrices in apply_type_modifiers(). https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- tests/hlsl-majority-pragma.shader_test | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/hlsl-majority-pragma.shader_test b/tests/hlsl-majority-pragma.shader_test index e7fc75cd..d269e859 100644 --- a/tests/hlsl-majority-pragma.shader_test +++ b/tests/hlsl-majority-pragma.shader_test @@ -19,3 +19,31 @@ uniform 8 float4 0.1 0.3 0.0 0.0 uniform 12 float4 0.2 0.4 0.0 0.0 draw quad probe all rgba (0.17, 0.39, 0.17, 0.39) 1 + +% The documentation claims these strings are subject to macro expansion. +% They are not. + +[pixel shader] + +#define KEY pack_matrix +#pragma KEY(row_major) +#define VALUE row_major +#pragma pack_matrix(VALUE) +#define PRAGMA pack_matrix(row_major) +#pragma PRAGMA + +uniform float2x2 r; + +float4 main() : sv_target +{ + float4 ret; + ret.xy = mul(r, float2(0.5, 0.6)); + ret.zw = 0.5; + return ret; +} + +[test] +uniform 0 float4 0.1 0.2 0.0 0.0 +uniform 4 float4 0.3 0.4 0.0 0.0 +draw quad +probe all rgba (0.23, 0.34, 0.5, 0.5) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl-invalid.shader_test | 18 ----- tests/majority-syntax.shader_test | 108 ++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 18 deletions(-) create mode 100644 tests/majority-syntax.shader_test diff --git a/Makefile.am b/Makefile.am index f9199472..cab5c63f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,6 +116,7 @@ vkd3d_shader_tests = \ tests/hlsl-vector-indexing.shader_test \ tests/hlsl-vector-indexing-uniform.shader_test \ tests/logic-operations.shader_test \ + tests/majority-syntax.shader_test \ tests/math.shader_test \ tests/matrix-semantics.shader_test \ tests/max.shader_test \ diff --git a/tests/hlsl-invalid.shader_test b/tests/hlsl-invalid.shader_test index e56205cb..ad062652 100644 --- a/tests/hlsl-invalid.shader_test +++ b/tests/hlsl-invalid.shader_test @@ -86,15 +86,6 @@ uniform float4 main() : sv_target return 0; } -[pixel shader fail] -typedef row_major float4x4 matrix_t; -typedef column_major matrix_t matrix2_t; - -float4 main() : sv_target -{ - return 0; -} - [pixel shader fail] float4 main() { @@ -228,15 +219,6 @@ float4 main() : sv_target return float4(0, 0, 0, 0); } -[pixel shader fail] -typedef row_major float4x4 mat_t; -column_major mat_t m; - -float4 main() : sv_target -{ - return float4(0, 0, 0, 0); -} - [pixel shader fail] typedef struct apple { diff --git a/tests/majority-syntax.shader_test b/tests/majority-syntax.shader_test new file mode 100644 index 00000000..63ba18f3 --- /dev/null +++ b/tests/majority-syntax.shader_test @@ -0,0 +1,108 @@ +[pixel shader] +typedef row_major float2x2 mat_t; + +row_major mat_t m; + +float4 main() : sv_target +{ + return float4(m[0], m[1]); +} + +[test] +uniform 0 float4 0.1 0.3 0.0 0.0 +uniform 4 float4 0.2 0.4 0.0 0.0 +draw quad +probe all rgba (0.1, 0.3, 0.2, 0.4) + +[pixel shader fail] +row_major row_major float4x4 m; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader fail] +typedef row_major float4x4 mat_t; +column_major mat_t m; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader fail] +typedef row_major float4x4 matrix_t; +typedef column_major matrix_t matrix2_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +typedef column_major float4 myfloat_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +column_major float4 f; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +struct apple +{ + float4x4 m; +}; +typedef column_major struct apple apple_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +struct apple +{ + float4x4 m; +}; +column_major struct apple a; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +typedef float4x4 myarray_t[2]; + +row_major myarray_t a; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +float4 main(uniform row_major float4 f) : sv_target +{ + return 0; +} + +[pixel shader fail todo] +struct apple +{ + row_major float4 f; +}; + +float4 main() : sv_target +{ + return 0; +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 7 ++++--- tests/majority-syntax.shader_test | 10 +++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 76b0eae7..588d8ad8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -903,6 +903,10 @@ static struct hlsl_type *apply_type_modifiers(struct hlsl_ctx *ctx, struct hlsl_ else default_majority = HLSL_MODIFIER_ROW_MAJOR; } + else if (type->type != HLSL_CLASS_MATRIX) + { + check_invalid_matrix_modifiers(ctx, *modifiers, loc); + } if (!default_majority && !(*modifiers & HLSL_TYPE_MODIFIERS_MASK)) return type; @@ -2041,9 +2045,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t } vkd3d_free(v->arrays.sizes); - if (type->type != HLSL_CLASS_MATRIX) - check_invalid_matrix_modifiers(ctx, modifiers, v->loc); - if (modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)) { struct vkd3d_string_buffer *string; diff --git a/tests/majority-syntax.shader_test b/tests/majority-syntax.shader_test index 63ba18f3..4ab385a5 100644 --- a/tests/majority-syntax.shader_test +++ b/tests/majority-syntax.shader_test @@ -48,7 +48,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] column_major float4 f; float4 main() : sv_target @@ -68,7 +68,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] struct apple { float4x4 m; @@ -80,7 +80,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] typedef float4x4 myarray_t[2]; row_major myarray_t a; @@ -90,13 +90,13 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] float4 main(uniform row_major float4 f) : sv_target { return 0; } -[pixel shader fail todo] +[pixel shader fail] struct apple { row_major float4 f; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> We will warn subsequently in apply_type_modifiers() or add_typedef(). --- libs/vkd3d-shader/hlsl.y | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 588d8ad8..1a9345ef 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -408,12 +408,6 @@ static DWORD add_modifiers(struct hlsl_ctx *ctx, DWORD modifiers, DWORD mod, con hlsl_release_string_buffer(ctx, string); return modifiers; } - if ((mod & HLSL_MODIFIERS_MAJORITY_MASK) && (modifiers & HLSL_MODIFIERS_MAJORITY_MASK)) - { - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, - "'row_major' and 'column_major' modifiers are mutually exclusive."); - return modifiers; - } return modifiers | mod; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 1a9345ef..3cc86c99 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1940,10 +1940,11 @@ static bool type_has_numeric_components(struct hlsl_type *type) } static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, - DWORD modifiers, struct list *var_list) + unsigned int modifiers, const struct vkd3d_shader_location *modifiers_loc, struct list *var_list) { struct parse_variable_def *v, *v_next; struct hlsl_ir_function_decl *func; + unsigned int invalid_modifiers; struct list *statements_list; struct hlsl_ir_var *var; struct hlsl_type *type; @@ -1963,6 +1964,17 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t if (!var_list) return statements_list; + invalid_modifiers = modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT); + if (invalid_modifiers) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_modifiers_to_string(ctx, invalid_modifiers))) + hlsl_error(ctx, modifiers_loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, + "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); + hlsl_release_string_buffer(ctx, string); + } + LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry) { bool unbounded_res_array = false; @@ -2039,16 +2051,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t } vkd3d_free(v->arrays.sizes); - if (modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)) - { - struct vkd3d_string_buffer *string; - - if ((string = hlsl_modifiers_to_string(ctx, modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)))) - hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, - "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); - hlsl_release_string_buffer(ctx, string); - } - if (!(var = hlsl_new_var(ctx, v->name, type, v->loc, &v->semantic, modifiers, &v->reg_reservation))) { free_parse_variable_def(v); @@ -3808,7 +3810,7 @@ struct_declaration: if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1))) YYABORT; - $$ = declare_vars(ctx, type, modifiers, $3); + $$ = declare_vars(ctx, type, modifiers, &@1, $3); } struct_spec: @@ -4531,7 +4533,7 @@ declaration: if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1))) YYABORT; - $$ = declare_vars(ctx, type, modifiers, $3); + $$ = declare_vars(ctx, type, modifiers, &@1, $3); } variables_def_optional: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
On Thu Feb 16 20:32:34 2023 +0000, Giovanni Mascellani wrote:
That makes sense, but I think it would be sensible to at least deduplicate that code using a helper. I ended up just passing the modifier location to declare_vars().
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24584
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)