[PATCH v3 0/3] MR101: vkd3d-shader/hlsl: Improvements to builtin typenames handling.
-- v3: vkd3d-shader/hlsl: Support case-insensitive lookup for builtin 'float' type. vkd3d-shader/hlsl: Use unsigned type for the 'dword' alias. vkd3d-shader/hlsl: Support case-insensitive lookup for builtin 'dword' type. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.c | 28 +++++++++++++++++++++---- libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/hlsl.l | 2 +- libs/vkd3d-shader/hlsl.y | 4 ++-- tests/hlsl-type-names.shader_test | 35 +++++++++++++++++++++++++++++++ 6 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 tests/hlsl-type-names.shader_test diff --git a/Makefile.am b/Makefile.am index 5eadfc29..471dce23 100644 --- a/Makefile.am +++ b/Makefile.am @@ -115,6 +115,7 @@ vkd3d_shader_tests = \ tests/hlsl-struct-assignment.shader_test \ tests/hlsl-struct-semantics.shader_test \ tests/hlsl-transpose.shader_test \ + tests/hlsl-type-names.shader_test \ tests/hlsl-vector-indexing.shader_test \ tests/hlsl-vector-indexing-uniform.shader_test \ tests/logic-operations.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index d6a87b3f..1ecf6510 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -542,15 +542,35 @@ struct hlsl_type *hlsl_new_uav_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim return type; } -struct hlsl_type *hlsl_get_type(struct hlsl_scope *scope, const char *name, bool recursive) +static const char * get_case_insensitive_typename(const char *name) +{ + static const char *const names[] = + { + "dword", + }; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(names); ++i) + if (!ascii_strcasecmp(names[i], name)) return names[i]; + + return NULL; +} + +struct hlsl_type *hlsl_get_type(struct hlsl_scope *scope, const char *name, bool recursive, bool case_insensitive) { struct rb_entry *entry = rb_get(&scope->types, name); if (entry) return RB_ENTRY_VALUE(entry, struct hlsl_type, scope_entry); + if (case_insensitive && !scope->upper && (name = get_case_insensitive_typename(name))) + { + if ((entry = rb_get(&scope->types, name))) + return RB_ENTRY_VALUE(entry, struct hlsl_type, scope_entry); + } + if (recursive && scope->upper) - return hlsl_get_type(scope->upper, name, recursive); + return hlsl_get_type(scope->upper, name, recursive, false); return NULL; } @@ -745,7 +765,7 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, bool hlsl_scope_add_type(struct hlsl_scope *scope, struct hlsl_type *type) { - if (hlsl_get_type(scope, type->name, false)) + if (hlsl_get_type(scope, type->name, false, false)) return false; rb_put(&scope->types, type->name, &type->scope_entry); @@ -2746,7 +2766,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } effect_types[] = { - {"DWORD", HLSL_CLASS_SCALAR, HLSL_TYPE_INT, 1, 1}, + {"dword", HLSL_CLASS_SCALAR, HLSL_TYPE_INT, 1, 1}, {"FLOAT", HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT, 1, 1}, {"VECTOR", HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT, 4, 1}, {"MATRIX", HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT, 4, 4}, diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 81b1a61d..23120025 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -997,7 +997,7 @@ void hlsl_free_var(struct hlsl_ir_var *decl); struct hlsl_ir_function *hlsl_get_function(struct hlsl_ctx *ctx, const char *name); struct hlsl_ir_function_decl *hlsl_get_func_decl(struct hlsl_ctx *ctx, const char *name); -struct hlsl_type *hlsl_get_type(struct hlsl_scope *scope, const char *name, bool recursive); +struct hlsl_type *hlsl_get_type(struct hlsl_scope *scope, const char *name, bool recursive, bool case_insensitive); struct hlsl_ir_var *hlsl_get_var(struct hlsl_scope *scope, const char *name); struct hlsl_type *hlsl_get_element_type_from_path_index(struct hlsl_ctx *ctx, const struct hlsl_type *type, diff --git a/libs/vkd3d-shader/hlsl.l b/libs/vkd3d-shader/hlsl.l index 69ed9d9d..59f31fcd 100644 --- a/libs/vkd3d-shader/hlsl.l +++ b/libs/vkd3d-shader/hlsl.l @@ -181,7 +181,7 @@ row_major {return KW_ROW_MAJOR; } yylval->name = hlsl_strdup(ctx, yytext); if (hlsl_get_var(ctx->cur_scope, yytext) || hlsl_get_function(ctx, yytext)) return VAR_IDENTIFIER; - else if (hlsl_get_type(ctx->cur_scope, yytext, true)) + else if (hlsl_get_type(ctx->cur_scope, yytext, true, true)) return TYPE_IDENTIFIER; else return NEW_IDENTIFIER; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 697b895d..74d280b7 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4548,7 +4548,7 @@ type_no_void: } | TYPE_IDENTIFIER { - $$ = hlsl_get_type(ctx->cur_scope, $1, true); + $$ = hlsl_get_type(ctx->cur_scope, $1, true, true); if ($$->is_minimum_precision) { if (ctx->profile->major_version < 4) @@ -4565,7 +4565,7 @@ type_no_void: } | KW_STRUCT TYPE_IDENTIFIER { - $$ = hlsl_get_type(ctx->cur_scope, $2, true); + $$ = hlsl_get_type(ctx->cur_scope, $2, true, false); if ($$->type != HLSL_CLASS_STRUCT) hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_REDEFINED, "\"%s\" redefined as a structure.", $2); vkd3d_free($2); diff --git a/tests/hlsl-type-names.shader_test b/tests/hlsl-type-names.shader_test new file mode 100644 index 00000000..8d5e5e75 --- /dev/null +++ b/tests/hlsl-type-names.shader_test @@ -0,0 +1,35 @@ +[pixel shader] +typedef float2 Dword; +typedef float3 dWord; + +float4 f() +{ + typedef Dword dword; + dword v = {1, 2}; + return float4(v.x, v.y, 3, 4); +} + +float4 f2() +{ + typedef dword dword; + dword v = 1; + return float4(v, v, v, v); +} + +float4 main() : SV_TARGET +{ + return f() + f2(); +} + +[test] +draw quad +probe all rgba (2.0, 3.0, 4.0, 5.0) + +% The "dword" alias is pre-defined as lowercase +[pixel shader fail] +typedef float2 dword; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 1ecf6510..e4797524 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2766,7 +2766,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } effect_types[] = { - {"dword", HLSL_CLASS_SCALAR, HLSL_TYPE_INT, 1, 1}, + {"dword", HLSL_CLASS_SCALAR, HLSL_TYPE_UINT, 1, 1}, {"FLOAT", HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT, 1, 1}, {"VECTOR", HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT, 4, 1}, {"MATRIX", HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT, 4, 4}, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 3 ++- tests/hlsl-type-names.shader_test | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index e4797524..3a97b87f 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -547,6 +547,7 @@ static const char * get_case_insensitive_typename(const char *name) static const char *const names[] = { "dword", + "float", }; unsigned int i; @@ -2767,7 +2768,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) effect_types[] = { {"dword", HLSL_CLASS_SCALAR, HLSL_TYPE_UINT, 1, 1}, - {"FLOAT", HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT, 1, 1}, + {"float", HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT, 1, 1}, {"VECTOR", HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT, 4, 1}, {"MATRIX", HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT, 4, 4}, {"STRING", HLSL_CLASS_OBJECT, HLSL_TYPE_STRING, 1, 1}, diff --git a/tests/hlsl-type-names.shader_test b/tests/hlsl-type-names.shader_test index 8d5e5e75..f739995d 100644 --- a/tests/hlsl-type-names.shader_test +++ b/tests/hlsl-type-names.shader_test @@ -1,19 +1,24 @@ [pixel shader] typedef float2 Dword; typedef float3 dWord; +typedef float2 fLoat; float4 f() { typedef Dword dword; - dword v = {1, 2}; - return float4(v.x, v.y, 3, 4); + typedef fLoat float; + dword v1 = {1, 2}; + float v2 = {3, 4}; + return float4(v1.x, v1.y, v2.x, v2.y); } float4 f2() { typedef dword dword; - dword v = 1; - return float4(v, v, v, v); + typedef float float; + dword v1 = 1; + float v2 = 1; + return float4(v1, v2, v1, v1); } float4 main() : SV_TARGET -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101
Removed string constants part for now until it's figured out (it was hitting fatal fixme for "uniform initializer" anyway). Updated changes better reflect what's going on. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101#note_24996
Yes, looks like it's more convoluted. "vector" and "string" are lowercase tokens that can't be used for variable names for instance. Doing "float2 sTring;" is valid, but "sTring" can't be used as a type after that. Reusing names of already declared variables as typenames is universally disallowed, so that's good.
Hrm, I thought I tested redefining non-lower-case versions of those, but I must have done it wrong. Anyway, this is still failing with the example I posted, as far as I can tell because hlsl_get_type's recursive call passes "false" rather than propagating the "case_insensitive" argument. Also, from patch 1/3:
+ for (i = 0; i < ARRAY_SIZE(names); ++i) + if (!ascii_strcasecmp(names[i], name)) return names[i];
Please put the "if" body on a new line, and use braces for multi-line bodies even if it's technically a single statement.
+ if (case_insensitive && !scope->upper && (name = get_case_insensitive_typename(name))) + { + if ((entry = rb_get(&scope->types, name))) + return RB_ENTRY_VALUE(entry, struct hlsl_type, scope_entry); + }
It looks mildly wrong that this checks "scope->upper" but not "recursive". It works, because we never pass recursive == false but case_insensitive == true, but I think it'd be clearer just to move this below the recursive call or use "else" there.
@@ -4565,7 +4565,7 @@ type_no_void: } | KW_STRUCT TYPE_IDENTIFIER { - $$ = hlsl_get_type(ctx->cur_scope, $2, true); + $$ = hlsl_get_type(ctx->cur_scope, $2, true, false); if ($$->type != HLSL_CLASS_STRUCT) hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_REDEFINED, "\"%s\" redefined as a structure.", $2); vkd3d_free($2);
This lacks tests (and also feels a little suspicious, mostly because it contradicts the lexer). -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/101#note_25048
participants (3)
-
Nikolay Sivov -
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)