From: Zebediah Figura zfigura@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..8d5ebe6b --- /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] + +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)
From: Zebediah Figura zfigura@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;
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-shader/hlsl.c | 55 ++++++++ libs/vkd3d-shader/hlsl.h | 17 +++ libs/vkd3d-shader/hlsl.y | 130 +++++++++++++----- 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 | 2 +- 9 files changed, 222 insertions(+), 79 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index eddbf2c8..f44f3a03 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_call *hlsl_new_call(struct hlsl_ctx *ctx, 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; +} + 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..14835c7c 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; + 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_call *hlsl_new_call(struct hlsl_ctx *ctx, 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..80da1894 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: @@ -2122,12 +2123,12 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t struct find_function_call_args { const struct parse_initializer *params; - const struct hlsl_ir_function_decl *decl; + struct hlsl_ir_function_decl *decl; };
static void find_function_call_exact(struct rb_entry *entry, void *context) { - const struct hlsl_ir_function_decl *decl = RB_ENTRY_VALUE(entry, const struct hlsl_ir_function_decl, entry); + struct hlsl_ir_function_decl *decl = RB_ENTRY_VALUE(entry, struct hlsl_ir_function_decl, entry); struct find_function_call_args *args = context; const struct hlsl_ir_var *param; unsigned int i = 0; @@ -2142,7 +2143,7 @@ static void find_function_call_exact(struct rb_entry *entry, void *context) args->decl = decl; }
-static const struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *ctx, +static struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *ctx, const char *name, const struct parse_initializer *params) { struct find_function_call_args args = {.params = params}; @@ -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; + struct hlsl_ir_function_decl *decl;
- 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_call *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->node.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 8d5ebe6b..054a2da6 100644 --- a/tests/uav-out-param.shader_test +++ b/tests/uav-out-param.shader_test @@ -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)
From: Zebediah Figura zfigura@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 80da1894..86d9b384 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() {
From: Zebediah Figura zfigura@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 f44f3a03..b7f222cc 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 14835c7c..3c62342b 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 86d9b384..75e54aec 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) {
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
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);
Maybe this could be updated to `vkd3d_unreachable();`.
This merge request was approved by Francisco Casas.
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
}
static struct list *add_call(struct hlsl_ctx *ctx, const char *name,
I just realized we are not freeing the memory of `name`. Maybe this patch could handle it.
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.c:
free_function(RB_ENTRY_VALUE(entry, struct hlsl_ir_function, entry));}
void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function_decl *decl, bool intrinsic)
I think it also makes sense to get rid of the `intrinsic` argument.
On Thu Nov 10 22:39:44 2022 +0000, Francisco Casas wrote:
Maybe this could be updated to `vkd3d_unreachable();`.
Maybe, but the invariant here is "an hlsl_block resulting from an expression should never have one of these as the last instruction, because none of them have a value", which feels more like "invariant that could be violated by accident" than "invariant that can only be violated due to memory corruption". Granted, I think Henri has argued that the former should be ERR() and the latter unreachable(), i.e. there's no room for assert() anywhere...
On Thu Nov 10 23:15:17 2022 +0000, Francisco Casas wrote:
I just realized we are not freeing the memory of `name`. Maybe this patch could handle it.
I'll write a separate patch for it. This series probably is too late to beat code freeze anyway.
On Fri Nov 11 00:09:35 2022 +0000, Francisco Casas wrote:
I think it also makes sense to get rid of the `intrinsic` argument.
I'll write an extra patch for it. Thanks!
On Fri Nov 11 01:11:26 2022 +0000, Zebediah Figura wrote:
I'll write a separate patch for it. This series probably is too late to beat code freeze anyway.
btw, I realized that in the `primare_expr:` rule, in the `VAR_IDENTIFIER` case, we are also forgetting to do ``` vkd3d_free($1); ``` in case you see that valgrind still reports lost memory after freeing `name`.
On Fri Nov 11 14:40:08 2022 +0000, Francisco Casas wrote:
btw, I realized that in the `primary_expr:` rule, in the `VAR_IDENTIFIER` case, we are also forgetting to do
vkd3d_free($1);in case you see that valgrind still reports lost memory after freeing `name`.
Yeah, I think it's safe to say there's a *lot* of memory leaks in the parser. Most of them on error paths, but I see some not on error paths too :-/
Given the way we have to allocate lots of small pieces of memory, and free most of them at exactly one point in time when the parser destroys itself, I wonder if it's worth using a custom pool, that frees everything at once. (There's probably a name for this pattern but I don't know it.)
Giovanni Mascellani (@giomasce) commented about tests/uav-out-param.shader_test:
+[require] +shader model >= 5.0
+[uav 0] +format r32g32b32a32 float +size (1, 1)
+0.1 0.2 0.3 0.4
+[compute shader]
This now fails, it should probably be marked as `todo`.
On Fri Nov 11 01:09:52 2022 +0000, Zebediah Figura wrote:
Maybe, but the invariant here is "an hlsl_block resulting from an expression should never have one of these as the last instruction, because none of them have a value", which feels more like "invariant that could be violated by accident" than "invariant that can only be violated due to memory corruption". Granted, I think Henri has argued that the former should be ERR() and the latter unreachable(), i.e. there's no room for assert() anywhere...
I don't see a real difference between violating an invariant by accident or by cosmic ray. To me any program configuration either can be reached for some user input (which is legitimate, i.e., not considered to cause undefined behavior) or it can't. In the first case an error message must be outputted, in the second case an assertion must be triggered (and `vkd3d_unreachable()` is just a better `assert(0)`).
Just my two cents, though.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
struct find_function_call_args { const struct parse_initializer *params;
- const struct hlsl_ir_function_decl *decl;
Any reason why you can't just keep `const` here and in the following hunks, and put it in `hlsl_ir_call` too?
On Thu Nov 17 10:31:38 2022 +0000, Giovanni Mascellani wrote:
I don't see a real difference between violating an invariant by accident or by cosmic ray. To me any program configuration either can be reached for some user input (which is legitimate, i.e., not considered to cause undefined behavior) or it can't. In the first case an error message must be outputted, in the second case an assertion must be triggered (and `vkd3d_unreachable()` is just a better `assert(0)`). Just my two cents, though.
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...
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)