[PATCH v2 0/5] MR47: hlsl: Function calls.
-- v2: vkd3d-shader/hlsl: Get rid of the "intrinsic" field of struct hlsl_ir_function. vkd3d-shader/hlsl: Forbid returning void expressions from void functions. vkd3d-shader/hlsl: Generate IR for user-defined function calls. vkd3d-shader/hlsl: Avoid assuming that expressions have at least one argument. tests: Test specifying a UAV address as an in/out parameter to a function. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/uav-out-param.shader_test | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/uav-out-param.shader_test diff --git a/Makefile.am b/Makefile.am index 0db71d67..a0ec8990 100644 --- a/Makefile.am +++ b/Makefile.am @@ -149,6 +149,7 @@ vkd3d_shader_tests = \ tests/trigonometry.shader_test \ tests/uav.shader_test \ tests/uav-load.shader_test \ + tests/uav-out-param.shader_test \ tests/writemask-assignop-0.shader_test \ tests/writemask-assignop-1.shader_test \ tests/writemask-assignop-2.shader_test \ diff --git a/tests/uav-out-param.shader_test b/tests/uav-out-param.shader_test new file mode 100644 index 00000000..646d14f6 --- /dev/null +++ b/tests/uav-out-param.shader_test @@ -0,0 +1,27 @@ +[require] +shader model >= 5.0 + +[uav 0] +format r32g32b32a32 float +size (1, 1) + +0.1 0.2 0.3 0.4 + +[compute shader todo] + +RWTexture2D<float4> u; + +void func(inout float4 f) +{ + f.xz += 0.1; +} + + [numthreads(1, 1, 1)] +void main() +{ + func(u[uint2(0, 0)].yzwx); +} + +[test] +todo dispatch 1 1 1 +todo probe uav 0 (0, 0) rgba (0.1, 0.3, 0.3, 0.5) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 16 +++++++++------- libs/vkd3d-shader/hlsl_constant_ops.c | 2 ++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 27af49cf..6ef84969 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -467,12 +467,12 @@ static bool lower_broadcasts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, v if (instr->type != HLSL_IR_EXPR) return false; cast = hlsl_ir_expr(instr); + if (cast->op != HLSL_OP1_CAST) + return false; src_type = cast->operands[0].node->data_type; dst_type = cast->node.data_type; - if (cast->op == HLSL_OP1_CAST - && src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR - && src_type->dimx == 1) + if (src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR && src_type->dimx == 1) { struct hlsl_ir_node *replacement; struct hlsl_ir_swizzle *swizzle; @@ -1038,12 +1038,14 @@ static bool fold_redundant_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst if (instr->type == HLSL_IR_EXPR) { struct hlsl_ir_expr *expr = hlsl_ir_expr(instr); - const struct hlsl_type *src_type = expr->operands[0].node->data_type; const struct hlsl_type *dst_type = expr->node.data_type; + const struct hlsl_type *src_type; if (expr->op != HLSL_OP1_CAST) return false; + src_type = expr->operands[0].node->data_type; + if (hlsl_types_are_equal(src_type, dst_type) || (src_type->base_type == dst_type->base_type && is_vec1(src_type) && is_vec1(dst_type))) { @@ -1201,12 +1203,12 @@ static bool lower_narrowing_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *ins if (instr->type != HLSL_IR_EXPR) return false; cast = hlsl_ir_expr(instr); + if (cast->op != HLSL_OP1_CAST) + return false; src_type = cast->operands[0].node->data_type; dst_type = cast->node.data_type; - if (cast->op == HLSL_OP1_CAST - && src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR - && dst_type->dimx < src_type->dimx) + if (src_type->type <= HLSL_CLASS_VECTOR && dst_type->type <= HLSL_CLASS_VECTOR && dst_type->dimx < src_type->dimx) { struct hlsl_ir_swizzle *swizzle; struct hlsl_ir_expr *new_cast; diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c index 858f020c..ea59fb86 100644 --- a/libs/vkd3d-shader/hlsl_constant_ops.c +++ b/libs/vkd3d-shader/hlsl_constant_ops.c @@ -509,6 +509,8 @@ bool hlsl_fold_constant_exprs(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, if (instr->type != HLSL_IR_EXPR) return false; expr = hlsl_ir_expr(instr); + if (!expr->operands[0].node) + return false; if (instr->data_type->type > HLSL_CLASS_VECTOR) return false; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 55 ++++++++ libs/vkd3d-shader/hlsl.h | 17 +++ libs/vkd3d-shader/hlsl.y | 122 ++++++++++++++---- libs/vkd3d-shader/hlsl_codegen.c | 5 + tests/hlsl-function.shader_test | 24 ++-- tests/hlsl-numthreads.shader_test | 2 +- ...lsl-return-implicit-conversion.shader_test | 60 ++++----- tests/hlsl-static-initializer.shader_test | 6 +- tests/uav-out-param.shader_test | 4 +- 9 files changed, 219 insertions(+), 76 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index eddbf2c8..ff2a3e3d 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -928,6 +928,19 @@ struct hlsl_ir_store *hlsl_new_store_component(struct hlsl_ctx *ctx, struct hlsl return store; } +struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl *decl, + const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_call *call; + + if (!(call = hlsl_alloc(ctx, sizeof(*call)))) + return NULL; + + init_node(&call->node, HLSL_IR_CALL, NULL, loc); + call->decl = decl; + return &call->node; +} + struct hlsl_ir_constant *hlsl_new_constant(struct hlsl_ctx *ctx, struct hlsl_type *type, const struct vkd3d_shader_location *loc) { @@ -1513,6 +1526,7 @@ const char *hlsl_node_type_to_string(enum hlsl_ir_node_type type) { static const char * const names[] = { + "HLSL_IR_CALL", "HLSL_IR_CONSTANT", "HLSL_IR_EXPR", "HLSL_IR_IF", @@ -1629,6 +1643,32 @@ const char *debug_hlsl_swizzle(unsigned int swizzle, unsigned int size) return vkd3d_dbg_sprintf(".%s", string); } +static void dump_ir_call(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, const struct hlsl_ir_call *call) +{ + const struct hlsl_ir_function_decl *decl = call->decl; + struct vkd3d_string_buffer *string; + const struct hlsl_ir_var *param; + + if (!(string = hlsl_type_to_string(ctx, decl->return_type))) + return; + + vkd3d_string_buffer_printf(buffer, "call %s %s(", string->buffer, decl->func->name); + hlsl_release_string_buffer(ctx, string); + + LIST_FOR_EACH_ENTRY(param, decl->parameters, struct hlsl_ir_var, param_entry) + { + if (!(string = hlsl_type_to_string(ctx, param->data_type))) + return; + + vkd3d_string_buffer_printf(buffer, "%s", string->buffer); + if (list_tail(decl->parameters) != ¶m->param_entry) + vkd3d_string_buffer_printf(buffer, ", "); + + hlsl_release_string_buffer(ctx, string); + } + vkd3d_string_buffer_printf(buffer, ")"); +} + static void dump_ir_constant(struct vkd3d_string_buffer *buffer, const struct hlsl_ir_constant *constant) { struct hlsl_type *type = constant->node.data_type; @@ -1675,6 +1715,8 @@ const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) { static const char *const op_names[] = { + [HLSL_OP0_VOID] = "void", + [HLSL_OP1_ABS] = "abs", [HLSL_OP1_BIT_NOT] = "~", [HLSL_OP1_CAST] = "cast", @@ -1860,6 +1902,10 @@ static void dump_instr(struct hlsl_ctx *ctx, struct vkd3d_string_buffer *buffer, switch (instr->type) { + case HLSL_IR_CALL: + dump_ir_call(ctx, buffer, hlsl_ir_call(instr)); + break; + case HLSL_IR_CONSTANT: dump_ir_constant(buffer, hlsl_ir_constant(instr)); break; @@ -1968,6 +2014,11 @@ void hlsl_free_instr_list(struct list *list) hlsl_free_instr(node); } +static void free_ir_call(struct hlsl_ir_call *call) +{ + vkd3d_free(call); +} + static void free_ir_constant(struct hlsl_ir_constant *constant) { vkd3d_free(constant); @@ -2044,6 +2095,10 @@ void hlsl_free_instr(struct hlsl_ir_node *node) switch (node->type) { + case HLSL_IR_CALL: + free_ir_call(hlsl_ir_call(node)); + break; + case HLSL_IR_CONSTANT: free_ir_constant(hlsl_ir_constant(node)); break; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index b6a593ca..e6ca5a75 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -171,6 +171,7 @@ struct hlsl_reg enum hlsl_ir_node_type { + HLSL_IR_CALL, HLSL_IR_CONSTANT, HLSL_IR_EXPR, HLSL_IR_IF, @@ -292,6 +293,12 @@ struct hlsl_ir_function_decl const struct hlsl_attribute *const *attrs; }; +struct hlsl_ir_call +{ + struct hlsl_ir_node node; + const struct hlsl_ir_function_decl *decl; +}; + struct hlsl_ir_if { struct hlsl_ir_node node; @@ -310,6 +317,8 @@ struct hlsl_ir_loop enum hlsl_ir_expr_op { + HLSL_OP0_VOID, + HLSL_OP1_ABS, HLSL_OP1_BIT_NOT, HLSL_OP1_CAST, @@ -558,6 +567,12 @@ struct hlsl_resource_load_params struct hlsl_ir_node *coords, *lod, *texel_offset; }; +static inline struct hlsl_ir_call *hlsl_ir_call(const struct hlsl_ir_node *node) +{ + assert(node->type == HLSL_IR_CALL); + return CONTAINING_RECORD(node, struct hlsl_ir_call, node); +} + static inline struct hlsl_ir_constant *hlsl_ir_constant(const struct hlsl_ir_node *node) { assert(node->type == HLSL_IR_CONSTANT); @@ -775,6 +790,8 @@ struct hlsl_ir_node *hlsl_new_binary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_exp struct hlsl_ir_constant *hlsl_new_bool_constant(struct hlsl_ctx *ctx, bool b, const struct vkd3d_shader_location *loc); struct hlsl_buffer *hlsl_new_buffer(struct hlsl_ctx *ctx, enum hlsl_buffer_type type, const char *name, const struct hlsl_reg_reservation *reservation, struct vkd3d_shader_location loc); +struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl *decl, + const struct vkd3d_shader_location *loc); struct hlsl_ir_expr *hlsl_new_cast(struct hlsl_ctx *ctx, struct hlsl_ir_node *node, struct hlsl_type *type, const struct vkd3d_shader_location *loc); struct hlsl_ir_constant *hlsl_new_constant(struct hlsl_ctx *ctx, struct hlsl_type *type, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index f77b0b61..92d9e0bd 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1162,12 +1162,13 @@ static unsigned int evaluate_array_dimension(struct hlsl_ir_node *node) FIXME("Unhandled type %s.\n", hlsl_node_type_to_string(node->type)); return 0; + case HLSL_IR_CALL: case HLSL_IR_IF: case HLSL_IR_JUMP: case HLSL_IR_LOOP: case HLSL_IR_RESOURCE_STORE: case HLSL_IR_STORE: - WARN("Invalid node type %s.\n", hlsl_node_type_to_string(node->type)); + assert(0); return 0; default: @@ -2579,64 +2580,129 @@ static int intrinsic_function_name_compare(const void *a, const void *b) } static struct list *add_call(struct hlsl_ctx *ctx, const char *name, - struct parse_initializer *params, struct vkd3d_shader_location loc) + struct parse_initializer *args, const struct vkd3d_shader_location *loc) { const struct hlsl_ir_function_decl *decl; struct intrinsic_function *intrinsic; - if ((decl = find_function_call(ctx, name, params))) + if ((decl = find_function_call(ctx, name, args))) { - hlsl_fixme(ctx, &loc, "Call to user-defined function \"%s\".", name); - free_parse_initializer(params); - return NULL; + struct hlsl_ir_node *call; + struct hlsl_ir_var *param; + unsigned int i; + + assert(args->args_count == list_count(decl->parameters)); + + i = 0; + LIST_FOR_EACH_ENTRY(param, decl->parameters, struct hlsl_ir_var, param_entry) + { + struct hlsl_ir_node *arg = args->args[i++]; + + if (!hlsl_types_are_equal(arg->data_type, param->data_type)) + { + hlsl_fixme(ctx, &arg->loc, "Implicit cast of a function argument."); + continue; + } + + if (param->modifiers & HLSL_STORAGE_IN) + { + struct hlsl_ir_store *store; + + if (!(store = hlsl_new_simple_store(ctx, param, arg))) + goto fail; + list_add_tail(args->instrs, &store->node.entry); + } + } + + if (!(call = hlsl_new_call(ctx, decl, loc))) + goto fail; + list_add_tail(args->instrs, &call->entry); + + i = 0; + LIST_FOR_EACH_ENTRY(param, decl->parameters, struct hlsl_ir_var, param_entry) + { + struct hlsl_ir_node *arg = args->args[i++]; + + if (param->modifiers & HLSL_STORAGE_OUT) + { + struct hlsl_ir_load *load; + + if (arg->data_type->modifiers & HLSL_MODIFIER_CONST) + hlsl_error(ctx, &arg->loc, VKD3D_SHADER_ERROR_HLSL_MODIFIES_CONST, + "Output argument to \"%s\" is const.", decl->func->name); + + if (!(load = hlsl_new_var_load(ctx, param, arg->loc))) + goto fail; + list_add_tail(args->instrs, &load->node.entry); + + if (!add_assignment(ctx, args->instrs, arg, ASSIGN_OP_ASSIGN, &load->node)) + goto fail; + } + } + + if (decl->return_var) + { + struct hlsl_ir_load *load; + + if (!(load = hlsl_new_var_load(ctx, decl->return_var, *loc))) + goto fail; + list_add_tail(args->instrs, &load->node.entry); + } + else + { + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {0}; + struct hlsl_ir_node *expr; + + if (!(expr = hlsl_new_expr(ctx, HLSL_OP0_VOID, operands, ctx->builtin_types.Void, loc))) + goto fail; + list_add_tail(args->instrs, &expr->entry); + } } else if ((intrinsic = bsearch(name, intrinsic_functions, ARRAY_SIZE(intrinsic_functions), sizeof(*intrinsic_functions), intrinsic_function_name_compare))) { - if (intrinsic->param_count >= 0 && params->args_count != intrinsic->param_count) + if (intrinsic->param_count >= 0 && args->args_count != intrinsic->param_count) { - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, "Wrong number of arguments to function '%s': expected %u, but got %u.", - name, intrinsic->param_count, params->args_count); - free_parse_initializer(params); - return NULL; + name, intrinsic->param_count, args->args_count); + goto fail; } if (intrinsic->check_numeric) { unsigned int i; - for (i = 0; i < params->args_count; ++i) + for (i = 0; i < args->args_count; ++i) { - if (params->args[i]->data_type->type > HLSL_CLASS_LAST_NUMERIC) + if (args->args[i]->data_type->type > HLSL_CLASS_LAST_NUMERIC) { struct vkd3d_string_buffer *string; - if ((string = hlsl_type_to_string(ctx, params->args[i]->data_type))) - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + if ((string = hlsl_type_to_string(ctx, args->args[i]->data_type))) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Wrong type for argument %u of '%s': expected a numeric type, but got '%s'.", i + 1, name, string->buffer); hlsl_release_string_buffer(ctx, string); - free_parse_initializer(params); - return NULL; + goto fail; } } } - if (!intrinsic->handler(ctx, params, &loc)) - { - free_parse_initializer(params); - return NULL; - } + if (!intrinsic->handler(ctx, args, loc)) + goto fail; } else { - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_NOT_DEFINED, "Function \"%s\" is not defined.", name); - free_parse_initializer(params); - return NULL; + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_NOT_DEFINED, "Function \"%s\" is not defined.", name); + goto fail; } - vkd3d_free(params->args); - return params->instrs; + vkd3d_free(args->args); + return args->instrs; + +fail: + free_parse_initializer(args); + return NULL; } static struct list *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type *type, @@ -4361,7 +4427,7 @@ primary_expr: } | var_identifier '(' func_arguments ')' { - if (!($$ = add_call(ctx, $1, &$3, @1))) + if (!($$ = add_call(ctx, $1, &$3, &@1))) YYABORT; } | NEW_IDENTIFIER diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 6ef84969..debf9e49 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1604,6 +1604,7 @@ static bool dce(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) break; } + case HLSL_IR_CALL: case HLSL_IR_IF: case HLSL_IR_JUMP: case HLSL_IR_LOOP: @@ -1673,6 +1674,10 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop switch (instr->type) { + case HLSL_IR_CALL: + FIXME("We should have inlined all calls before computing liveness.\n"); + break; + case HLSL_IR_STORE: { struct hlsl_ir_store *store = hlsl_ir_store(instr); diff --git a/tests/hlsl-function.shader_test b/tests/hlsl-function.shader_test index da198083..3f4908ec 100644 --- a/tests/hlsl-function.shader_test +++ b/tests/hlsl-function.shader_test @@ -9,7 +9,7 @@ float4 main() : sv_target % It's legal to call an undefined function in unused code, though. -[pixel shader todo] +[pixel shader] float4 func(); @@ -23,7 +23,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] void func(inout float o) { @@ -37,7 +37,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] void func(inout float2 o) { @@ -51,7 +51,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] void func(out float o) { @@ -65,7 +65,7 @@ float4 main() : sv_target return x; } -[pixel shader fail todo] +[pixel shader fail] void func(inout float o) { @@ -78,7 +78,7 @@ float4 main() : sv_target return x; } -[pixel shader fail todo] +[pixel shader fail] void func() { @@ -118,7 +118,7 @@ void func() { } -[pixel shader todo] +[pixel shader] float func(in float a, out float b, inout float c) { @@ -139,10 +139,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.5, 0.6, 0.7, 0) +draw quad +todo probe all rgba (0.5, 0.6, 0.7, 0) -[pixel shader todo] +[pixel shader] void func(in float a, inout float2 b) { @@ -160,5 +160,5 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.6, 0.1, 0.5, 0) +draw quad +todo probe all rgba (0.6, 0.1, 0.5, 0) diff --git a/tests/hlsl-numthreads.shader_test b/tests/hlsl-numthreads.shader_test index 328be664..9e561ae4 100644 --- a/tests/hlsl-numthreads.shader_test +++ b/tests/hlsl-numthreads.shader_test @@ -153,7 +153,7 @@ static int x = 1; [numthreads((x = 2), 1, 1)] void main() {} -[compute shader todo] +[compute shader] static int x = 1; diff --git a/tests/hlsl-return-implicit-conversion.shader_test b/tests/hlsl-return-implicit-conversion.shader_test index a276f5bd..e88a4109 100644 --- a/tests/hlsl-return-implicit-conversion.shader_test +++ b/tests/hlsl-return-implicit-conversion.shader_test @@ -38,7 +38,7 @@ float4x1 main() : sv_target draw quad probe all rgba (0.4, 0.3, 0.2, 0.1) -[pixel shader todo] +[pixel shader] float3 func() { return float3x1(0.4, 0.3, 0.2); @@ -50,10 +50,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader todo] +[pixel shader] float3 func() { return float1x3(0.4, 0.3, 0.2); @@ -65,10 +65,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader todo] +[pixel shader] float1x3 func() { return float3(0.4, 0.3, 0.2); @@ -80,10 +80,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader todo] +[pixel shader] float3x1 func() { return float3(0.4, 0.3, 0.2); @@ -95,8 +95,8 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) [pixel shader fail] float3x1 func() @@ -120,7 +120,7 @@ float4 main() : sv_target return float4(func(), 0.0); } -[pixel shader todo] +[pixel shader] float3 func() { return float4(0.4, 0.3, 0.2, 0.1); @@ -132,10 +132,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader todo] +[pixel shader] float3 func() { return float4x1(0.4, 0.3, 0.2, 0.1); @@ -147,10 +147,10 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) -[pixel shader todo] +[pixel shader] float3 func() { return float1x4(0.4, 0.3, 0.2, 0.1); @@ -162,8 +162,8 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) [pixel shader fail todo] float3x1 func() @@ -176,7 +176,7 @@ float4 main() : sv_target return float4(func(), 0.0); } -[pixel shader todo] +[pixel shader] float3x1 func() { return float4x1(0.4, 0.3, 0.2, 0.1); @@ -188,8 +188,8 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) [pixel shader fail] float3x1 func() @@ -202,7 +202,7 @@ float4 main() : sv_target return float4(func(), 0.0); } -[pixel shader todo] +[pixel shader] float1x3 func() { return float4(0.4, 0.3, 0.2, 0.1); @@ -214,8 +214,8 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) [pixel shader fail] float1x3 func() @@ -228,7 +228,7 @@ float4 main() : sv_target return float4(func(), 0.0); } -[pixel shader todo] +[pixel shader] float1x3 func() { return float1x4(0.4, 0.3, 0.2, 0.1); @@ -240,5 +240,5 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.4, 0.3, 0.2, 0.0) +draw quad +todo probe all rgba (0.4, 0.3, 0.2, 0.0) diff --git a/tests/hlsl-static-initializer.shader_test b/tests/hlsl-static-initializer.shader_test index 0f53f4d1..526fc99d 100644 --- a/tests/hlsl-static-initializer.shader_test +++ b/tests/hlsl-static-initializer.shader_test @@ -1,4 +1,4 @@ -[pixel shader todo] +[pixel shader] float myfunc() { return 0.6; @@ -12,5 +12,5 @@ float4 main() : sv_target } [test] -todo draw quad -probe all rgba (0.8, 0.0, 0.0, 0.0) +draw quad +todo probe all rgba (0.8, 0.0, 0.0, 0.0) diff --git a/tests/uav-out-param.shader_test b/tests/uav-out-param.shader_test index 646d14f6..054a2da6 100644 --- a/tests/uav-out-param.shader_test +++ b/tests/uav-out-param.shader_test @@ -7,7 +7,7 @@ size (1, 1) 0.1 0.2 0.3 0.4 -[compute shader todo] +[compute shader] RWTexture2D<float4> u; @@ -23,5 +23,5 @@ void main() } [test] -todo dispatch 1 1 1 +dispatch 1 1 1 todo probe uav 0 (0, 0) rgba (0.1, 0.3, 0.3, 0.5) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 26 +++++++++++++++++--------- tests/hlsl-function.shader_test | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 92d9e0bd..06d7b463 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -599,21 +599,29 @@ static struct hlsl_ir_jump *add_return(struct hlsl_ctx *ctx, struct list *instrs struct hlsl_type *return_type = ctx->cur_function->return_type; struct hlsl_ir_jump *jump; - if (return_value) + if (ctx->cur_function->return_var) { - struct hlsl_ir_store *store; + if (return_value) + { + struct hlsl_ir_store *store; - if (!(return_value = add_implicit_conversion(ctx, instrs, return_value, return_type, &loc))) - return NULL; + if (!(return_value = add_implicit_conversion(ctx, instrs, return_value, return_type, &loc))) + return NULL; - if (!(store = hlsl_new_simple_store(ctx, ctx->cur_function->return_var, return_value))) + if (!(store = hlsl_new_simple_store(ctx, ctx->cur_function->return_var, return_value))) + return NULL; + list_add_after(&return_value->entry, &store->node.entry); + } + else + { + hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN, "Non-void functions must return a value."); return NULL; - list_add_after(&return_value->entry, &store->node.entry); + } } - else if (ctx->cur_function->return_var) + else { - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN, "Non-void function must return a value."); - return NULL; + if (return_value) + hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN, "Void functions cannot return a value."); } if (!(jump = hlsl_new_jump(ctx, HLSL_IR_JUMP_RETURN, loc))) diff --git a/tests/hlsl-function.shader_test b/tests/hlsl-function.shader_test index 3f4908ec..e8bb80c4 100644 --- a/tests/hlsl-function.shader_test +++ b/tests/hlsl-function.shader_test @@ -89,7 +89,7 @@ float4 main() : sv_target return func(); } -[pixel shader fail todo] +[pixel shader fail] void foo() { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
From: Zebediah Figura <zfigura(a)codeweavers.com> We have a different system of generating intrinsics, which makes it easier to deal with "polymorphic" arithmetic functions. Defining and storing intrinsics as hlsl_ir_function_decls would also require more space in memory (and more optimization passes to get rid of the parameter variables), and doesn't really save us any effort in terms of source code. --- libs/vkd3d-shader/hlsl.c | 12 ------------ libs/vkd3d-shader/hlsl.h | 1 - libs/vkd3d-shader/hlsl.y | 2 +- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index ff2a3e3d..efc31a9d 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2191,17 +2191,6 @@ void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function if (func_entry) { func = RB_ENTRY_VALUE(func_entry, struct hlsl_ir_function, entry); - if (intrinsic != func->intrinsic) - { - if (intrinsic) - { - ERR("Redeclaring a user defined function as an intrinsic.\n"); - return; - } - func->intrinsic = intrinsic; - rb_destroy(&func->overloads, free_function_decl_rb, NULL); - rb_init(&func->overloads, compare_function_decl_rb); - } decl->func = func; if ((old_entry = rb_get(&func->overloads, decl->parameters))) { @@ -2236,7 +2225,6 @@ void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function rb_init(&func->overloads, compare_function_decl_rb); decl->func = func; rb_put(&func->overloads, decl->parameters, &decl->entry); - func->intrinsic = intrinsic; rb_put(&ctx->functions, func->name, &func->entry); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e6ca5a75..7213935a 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -276,7 +276,6 @@ struct hlsl_ir_function struct rb_entry entry; const char *name; struct rb_tree overloads; - bool intrinsic; }; struct hlsl_ir_function_decl diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 06d7b463..1deb6757 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3324,7 +3324,7 @@ hlsl_prog: const struct hlsl_ir_function_decl *decl; decl = get_func_decl(&ctx->functions, $2.name, $2.decl->parameters); - if (decl && !decl->func->intrinsic) + if (decl) { if (decl->has_body && $2.decl->has_body) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47
On Thu Nov 17 10:08:27 2022 +0000, Giovanni Mascellani wrote:
This now fails, it should probably be marked as `todo`. Thanks. I don't know how I missed running tests for these :-/
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47#note_16269
On Thu Nov 17 23:49:34 2022 +0000, Zebediah Figura wrote:
changed this line in [version 2 of the diff](/wine/vkd3d/-/merge_requests/47/diffs?diff_id=19331&start_sha=b885572cb99bbbcfec6a3446e19d8464f0ac20df#9155b9453b4ec8ea0b9b025dfb55c061bd931610_2135_2134) I feel like there must have been a reason, but I can't remember it, and it's not clear to me why I should need a non-constant pointer there either. I've removed it for now.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47#note_16270
On Thu Nov 17 23:31:34 2022 +0000, Zebediah Figura wrote:
To me, the difference is: invariants which might be violated by accident deserve to be written in the code to remind the reader of the invariant, and they deserve a warning to be printed if they're violated. By contrast, invariants which can only be violated via memory corruption should, in theory, not even be written in the code—simply because memory corruption can happen anywhere and it's a waste of time to write those assertions. In my opinion the only reason we have vkd3d_unreachable() in things like switch default cases is that the compiler would warn otherwise. I did forget that vkd3d_unreachable() actually effectively includes an assert, though; I thought it was always equivalent to __builtin_unreachable(). Maybe both types should be vkd3d_unreachable(), though, as it is. I.e. if you're a user who trusts the programmer to not violate their own invariants, and you want the most efficient code... Yeah, in my intention `vkd3d_unreachable()` is equivalent to `assert(0)`, so it aborts with an error message without `NDEBUG` and it does nothing with `NDEBUG`, but with the difference that it lets the compiler knows (either via `__builtin_unreachable()` or via `__attribute__((noreturn))` that that piece of code is not meant to be reached (mostly so that the compiler spares us with its overzealous warnings without us sticking in dummy `return 0` statements; in theory there is also a performance gain, but I doubt that's ever going to be visible).
I say that I don't see a real difference between violating an invariant by accident or by memory corruption because in practice I don't know how to differentiate between the two in advance. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47#note_16348
On Fri Nov 18 11:50:50 2022 +0000, Giovanni Mascellani wrote:
Yeah, in my intention `vkd3d_unreachable()` is equivalent to `assert(0)`, so it aborts with an error message without `NDEBUG` and it does nothing with `NDEBUG`, but with the difference that it lets the compiler knows (either via `__builtin_unreachable()` or via `__attribute__((noreturn))` that that piece of code is not meant to be reached (mostly so that the compiler spares us with its overzealous warnings without us sticking in dummy `return 0` statements; in theory there is also a performance gain, but I doubt that's ever going to be visible). I say that I don't see a real difference between violating an invariant by accident or by memory corruption because in practice I don't know how to differentiate between the two in advance. After thinking about it I'm inclined to agree, there's not really any point in not using vkd3d_unreachable() for this.
I think part of my hesitation right now, honestly, is that vkd3d_unreachable() doesn't *look* like assert(), it looks like __builtin_unreachable(). Not sure I see a nice solution, though. I'd advocate for a vkd3d_assert(), but there's no nice way to mark that function as "might not return". -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/47#note_20626
participants (3)
-
Giovanni Mascellani (＠giomasce) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)