[PATCH v3 0/4] MR745: hlsl: Avoid HLSL_CLASS_OBJECT, part 1.
HLSL_CLASS_OBJECT does not really have much in common, and a lot of code currently uses it assuming that all objects are resources, which is not actually true. Most users of HLSL_CLASS_OBJECT want to check for specific types, and so we can simplify code a fair amount by promoting all object "base types" to top-level classes. This series is a first step, comprising some cleanup towards that goal. -- v3: vkd3d-shader/hlsl: Use hlsl_is_numeric_type() in type_has_object_components(). vkd3d-shader/hlsl: Simplify type_has_object_components(). https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
From: Zebediah Figura <zfigura(a)codeweavers.com> We already perform an implicit cast per component in initialize_var_components(). --- libs/vkd3d-shader/hlsl.y | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 52c217654..17bb565db 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4358,22 +4358,7 @@ static struct hlsl_block *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type return NULL; for (i = 0; i < params->args_count; ++i) - { - struct hlsl_ir_node *arg = params->args[i]; - - if (arg->data_type->class == HLSL_CLASS_OBJECT) - { - struct vkd3d_string_buffer *string; - - if ((string = hlsl_type_to_string(ctx, arg->data_type))) - hlsl_error(ctx, &arg->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, - "Invalid type %s for constructor argument.", string->buffer); - hlsl_release_string_buffer(ctx, string); - continue; - } - - initialize_var_components(ctx, params->instrs, var, &idx, arg); - } + initialize_var_components(ctx, params->instrs, var, &idx, params->args[i]); if (!(load = hlsl_new_var_load(ctx, var, loc))) return NULL; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 5c09ce04f..6f2de9376 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -427,7 +427,10 @@ static void prepend_input_copy_recurse(struct hlsl_ctx *ctx, struct hlsl_block * { field = &type->e.record.fields[i]; if (hlsl_type_is_resource(field->type)) + { + hlsl_fixme(ctx, &field->loc, "Prepend uniform copies for resource components within structs."); continue; + } validate_field_semantic(ctx, field); semantic = &field->semantic; elem_semantic_index = semantic->index; @@ -5237,25 +5240,6 @@ static void parse_numthreads_attribute(struct hlsl_ctx *ctx, const struct hlsl_a } } -static bool type_has_object_components(struct hlsl_type *type) -{ - if (type->class == HLSL_CLASS_OBJECT) - return true; - if (type->class == HLSL_CLASS_ARRAY) - return type_has_object_components(type->e.array.type); - if (type->class == HLSL_CLASS_STRUCT) - { - unsigned int i; - - for (i = 0; i < type->e.record.field_count; ++i) - { - if (type_has_object_components(type->e.record.fields[i].type)) - return true; - } - } - return false; -} - static void remove_unreachable_code(struct hlsl_ctx *ctx, struct hlsl_block *body) { struct hlsl_ir_node *instr, *next; @@ -5363,9 +5347,6 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry } else { - if (type_has_object_components(var->data_type)) - hlsl_fixme(ctx, &var->loc, "Prepend uniform copies for object components within structs."); - if (hlsl_get_multiarray_element_type(var->data_type)->class != HLSL_CLASS_STRUCT && !var->semantic.name) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
From: Zebediah Figura <zfigura(a)codeweavers.com> The extra argument is not very easy to intuit. Since all we're trying to do here is check whether the type is a struct with object components in it, write that out explicitly. --- libs/vkd3d-shader/hlsl.y | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 17bb565db..f6da4755f 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2091,12 +2091,12 @@ static void initialize_var_components(struct hlsl_ctx *ctx, struct hlsl_block *i } } -static bool type_has_object_components(struct hlsl_type *type, bool must_be_in_struct) +static bool type_has_object_components(const struct hlsl_type *type) { if (type->class == HLSL_CLASS_OBJECT) - return !must_be_in_struct; + return true; if (type->class == HLSL_CLASS_ARRAY) - return type_has_object_components(type->e.array.type, must_be_in_struct); + return type_has_object_components(type->e.array.type); if (type->class == HLSL_CLASS_STRUCT) { @@ -2104,7 +2104,7 @@ static bool type_has_object_components(struct hlsl_type *type, bool must_be_in_s for (i = 0; i < type->e.record.field_count; ++i) { - if (type_has_object_components(type->e.record.fields[i].type, false)) + if (type_has_object_components(type->e.record.fields[i].type)) return true; } } @@ -2146,6 +2146,18 @@ static void check_invalid_in_out_modifiers(struct hlsl_ctx *ctx, unsigned int mo } } +static void check_invalid_object_fields(struct hlsl_ctx *ctx, const struct hlsl_ir_var *var) +{ + const struct hlsl_type *type = var->data_type; + + while (type->class == HLSL_CLASS_ARRAY) + type = type->e.array.type; + + if (type->class == HLSL_CLASS_STRUCT && type_has_object_components(type)) + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Target profile doesn't support objects as struct members in uniform variables."); +} + static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v) { struct hlsl_type *basic_type = v->basic_type; @@ -2271,12 +2283,8 @@ static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v) if (!(modifiers & HLSL_STORAGE_STATIC)) var->storage_modifiers |= HLSL_STORAGE_UNIFORM; - if (ctx->profile->major_version < 5 && (var->storage_modifiers & HLSL_STORAGE_UNIFORM) && - type_has_object_components(var->data_type, true)) - { - hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, - "Target profile doesn't support objects as struct members in uniform variables."); - } + if (ctx->profile->major_version < 5 && (var->storage_modifiers & HLSL_STORAGE_UNIFORM)) + check_invalid_object_fields(ctx, var); if ((func = hlsl_get_first_func_decl(ctx, var->name))) { @@ -2312,7 +2320,7 @@ static void declare_var(struct hlsl_ctx *ctx, struct parse_variable_def *v) } if ((var->storage_modifiers & HLSL_STORAGE_STATIC) && type_has_numeric_components(var->data_type) - && type_has_object_components(var->data_type, false)) + && type_has_object_components(var->data_type)) { hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Static variables cannot have both numeric and resource components."); @@ -2400,7 +2408,7 @@ static struct hlsl_block *initialize_vars(struct hlsl_ctx *ctx, struct list *var /* Initialize statics to zero by default. */ - if (type_has_object_components(var->data_type, false)) + if (type_has_object_components(var->data_type)) { free_parse_variable_def(v); continue; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index f6da4755f..e69359f9b 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2093,22 +2093,21 @@ static void initialize_var_components(struct hlsl_ctx *ctx, struct hlsl_block *i static bool type_has_object_components(const struct hlsl_type *type) { - if (type->class == HLSL_CLASS_OBJECT) - return true; if (type->class == HLSL_CLASS_ARRAY) return type_has_object_components(type->e.array.type); if (type->class == HLSL_CLASS_STRUCT) { - unsigned int i; - - for (i = 0; i < type->e.record.field_count; ++i) + for (unsigned int i = 0; i < type->e.record.field_count; ++i) { if (type_has_object_components(type->e.record.fields[i].type)) return true; } + + return false; } - return false; + + return !hlsl_is_numeric_type(type); } static bool type_has_numeric_components(struct hlsl_type *type) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
On Thu Mar 28 21:41:31 2024 +0000, Zebediah Figura wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/745/diffs?diff_id=107639&start_sha=ab292ef069b05066aeb0a843ebc143523b625cd0#9155b9453b4ec8ea0b9b025dfb55c061bd931610_2274_2285) Good catch, I've updated it to handle that case as well.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745#note_66424
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/745
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)