[PATCH 0/4] MR310: hlsl: Define some intrinsics in HLSL.
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 13 ++++--------- libs/vkd3d-shader/hlsl.c | 16 ++++++++++++++++ libs/vkd3d-shader/hlsl.h | 2 ++ libs/vkd3d-shader/hlsl_codegen.c | 24 +++++++++--------------- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 99a5bd7a..8b01bf81 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -1638,17 +1638,12 @@ static void write_sm1_uniforms(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffe if (var->is_param && var->is_uniform) { - struct vkd3d_string_buffer *name; + char *new_name; - if (!(name = hlsl_get_string_buffer(ctx))) - { - buffer->status = VKD3D_ERROR_OUT_OF_MEMORY; - return; - } - vkd3d_string_buffer_printf(name, "$%s", var->name); + if (!(new_name = hlsl_sprintf_alloc(ctx, "$%s", var->name))) + continue; vkd3d_free((char *)var->name); - var->name = hlsl_strdup(ctx, name->buffer); - hlsl_release_string_buffer(ctx, name); + var->name = new_name; } } } diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 8b706e1e..aef5a408 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -72,6 +72,22 @@ void hlsl_fixme(struct hlsl_ctx *ctx, const struct vkd3d_shader_location *loc, c ctx->result = VKD3D_ERROR_NOT_IMPLEMENTED; } +char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...) +{ + struct vkd3d_string_buffer *string; + va_list args; + char *ret; + + if (!(string = hlsl_get_string_buffer(ctx))) + return NULL; + va_start(args, fmt); + vkd3d_string_buffer_vprintf(string, fmt, args); + va_end(args); + ret = hlsl_strdup(ctx, string->buffer); + hlsl_release_string_buffer(ctx, string); + return ret; +} + bool hlsl_add_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *decl, bool local_var) { struct hlsl_scope *scope = ctx->cur_scope; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 75a1e6e4..abb73651 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -1068,6 +1068,8 @@ static inline unsigned int hlsl_sampler_dim_count(enum hlsl_sampler_dim dim) } } +char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...); + const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op); const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type); const char *debug_hlsl_writemask(unsigned int writemask); diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index bfa605f4..4a82e87b 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -163,10 +163,10 @@ static bool replace_deref_path_with_offset(struct hlsl_ctx *ctx, struct hlsl_der * work. */ static void prepend_uniform_copy(struct hlsl_ctx *ctx, struct hlsl_block *block, struct hlsl_ir_var *temp) { - struct vkd3d_string_buffer *name; struct hlsl_ir_var *uniform; struct hlsl_ir_node *store; struct hlsl_ir_load *load; + char *new_name; /* Use the synthetic name for the temp, rather than the uniform, so that we * can write the uniform name into the shader reflection data. */ @@ -180,11 +180,9 @@ static void prepend_uniform_copy(struct hlsl_ctx *ctx, struct hlsl_block *block, uniform->is_param = temp->is_param; uniform->buffer = temp->buffer; - if (!(name = hlsl_get_string_buffer(ctx))) + if (!(new_name = hlsl_sprintf_alloc(ctx, "<temp-%s>", temp->name))) return; - vkd3d_string_buffer_printf(name, "<temp-%s>", temp->name); - temp->name = hlsl_strdup(ctx, name->buffer); - hlsl_release_string_buffer(ctx, name); + temp->name = new_name; if (!(load = hlsl_new_var_load(ctx, uniform, &temp->loc))) return; @@ -235,16 +233,15 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir uint32_t index, bool output, const struct vkd3d_shader_location *loc) { struct hlsl_semantic new_semantic; - struct vkd3d_string_buffer *name; struct hlsl_ir_var *ext_var; + char *new_name; - if (!(name = hlsl_get_string_buffer(ctx))) + if (!(new_name = hlsl_sprintf_alloc(ctx, "<%s-%s%u>", output ? "output" : "input", semantic->name, index))) return NULL; - vkd3d_string_buffer_printf(name, "<%s-%s%u>", output ? "output" : "input", semantic->name, index); LIST_FOR_EACH_ENTRY(ext_var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) { - if (!ascii_strcasecmp(ext_var->name, name->buffer)) + if (!ascii_strcasecmp(ext_var->name, new_name)) { if (output) { @@ -271,25 +268,22 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir } } - hlsl_release_string_buffer(ctx, name); return ext_var; } } if (!(new_semantic.name = hlsl_strdup(ctx, semantic->name))) { - hlsl_release_string_buffer(ctx, name); + vkd3d_free(new_name); return NULL; } new_semantic.index = index; - if (!(ext_var = hlsl_new_var(ctx, hlsl_strdup(ctx, name->buffer), type, loc, &new_semantic, - modifiers, NULL))) + if (!(ext_var = hlsl_new_var(ctx, new_name, type, loc, &new_semantic, modifiers, NULL))) { - hlsl_release_string_buffer(ctx, name); + vkd3d_free(new_name); hlsl_cleanup_semantic(&new_semantic); return NULL; } - hlsl_release_string_buffer(ctx, name); if (output) ext_var->is_output_semantic = 1; else -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 165 +++++++++++++++++++++------------------ 1 file changed, 87 insertions(+), 78 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 29e0ff0c..4e3a10c8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2330,6 +2330,92 @@ static struct hlsl_ir_function_decl *find_function_call(struct hlsl_ctx *ctx, return args.decl; } +static struct hlsl_ir_node *hlsl_new_void_expr(struct hlsl_ctx *ctx, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {0}; + + return hlsl_new_expr(ctx, HLSL_OP0_VOID, operands, ctx->builtin_types.Void, loc); +} + +static bool add_user_call(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *func, + const struct parse_initializer *args, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *call; + unsigned int i; + + assert(args->args_count == func->parameters.count); + + for (i = 0; i < func->parameters.count; ++i) + { + struct hlsl_ir_var *param = func->parameters.vars[i]; + struct hlsl_ir_node *arg = args->args[i]; + + if (!hlsl_types_are_equal(arg->data_type, param->data_type)) + { + struct hlsl_ir_node *cast; + + if (!(cast = add_cast(ctx, args->instrs, arg, param->data_type, &arg->loc))) + return false; + args->args[i] = cast; + arg = cast; + } + + if (param->storage_modifiers & HLSL_STORAGE_IN) + { + struct hlsl_ir_node *store; + + if (!(store = hlsl_new_simple_store(ctx, param, arg))) + return false; + hlsl_block_add_instr(args->instrs, store); + } + } + + if (!(call = hlsl_new_call(ctx, func, loc))) + return false; + hlsl_block_add_instr(args->instrs, call); + + for (i = 0; i < func->parameters.count; ++i) + { + struct hlsl_ir_var *param = func->parameters.vars[i]; + struct hlsl_ir_node *arg = args->args[i]; + + if (param->storage_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.", func->func->name); + + if (!(load = hlsl_new_var_load(ctx, param, &arg->loc))) + return false; + hlsl_block_add_instr(args->instrs, &load->node); + + if (!add_assignment(ctx, args->instrs, arg, ASSIGN_OP_ASSIGN, &load->node)) + return false; + } + } + + if (func->return_var) + { + struct hlsl_ir_load *load; + + if (!(load = hlsl_new_var_load(ctx, func->return_var, loc))) + return false; + hlsl_block_add_instr(args->instrs, &load->node); + } + else + { + struct hlsl_ir_node *expr; + + if (!(expr = hlsl_new_void_expr(ctx, loc))) + return false; + hlsl_block_add_instr(args->instrs, expr); + } + + return true; +} + static struct hlsl_ir_node *intrinsic_float_convert_arg(struct hlsl_ctx *ctx, const struct parse_initializer *params, struct hlsl_ir_node *arg, const struct vkd3d_shader_location *loc) { @@ -3659,13 +3745,6 @@ static int intrinsic_function_name_compare(const void *a, const void *b) return strcmp(a, func->name); } -static struct hlsl_ir_node *hlsl_new_void_expr(struct hlsl_ctx *ctx, const struct vkd3d_shader_location *loc) -{ - struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {0}; - - return hlsl_new_expr(ctx, HLSL_OP0_VOID, operands, ctx->builtin_types.Void, loc); -} - static struct hlsl_block *add_call(struct hlsl_ctx *ctx, const char *name, struct parse_initializer *args, const struct vkd3d_shader_location *loc) { @@ -3674,78 +3753,8 @@ static struct hlsl_block *add_call(struct hlsl_ctx *ctx, const char *name, if ((decl = find_function_call(ctx, name, args, loc))) { - struct hlsl_ir_node *call; - unsigned int i; - - assert(args->args_count == decl->parameters.count); - - for (i = 0; i < decl->parameters.count; ++i) - { - struct hlsl_ir_var *param = decl->parameters.vars[i]; - struct hlsl_ir_node *arg = args->args[i]; - - if (!hlsl_types_are_equal(arg->data_type, param->data_type)) - { - struct hlsl_ir_node *cast; - - if (!(cast = add_cast(ctx, args->instrs, arg, param->data_type, &arg->loc))) - goto fail; - args->args[i] = cast; - arg = cast; - } - - if (param->storage_modifiers & HLSL_STORAGE_IN) - { - struct hlsl_ir_node *store; - - if (!(store = hlsl_new_simple_store(ctx, param, arg))) - goto fail; - hlsl_block_add_instr(args->instrs, store); - } - } - - if (!(call = hlsl_new_call(ctx, decl, loc))) + if (!add_user_call(ctx, decl, args, loc)) goto fail; - hlsl_block_add_instr(args->instrs, call); - - for (i = 0; i < decl->parameters.count; ++i) - { - struct hlsl_ir_var *param = decl->parameters.vars[i]; - struct hlsl_ir_node *arg = args->args[i]; - - if (param->storage_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; - hlsl_block_add_instr(args->instrs, &load->node); - - 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; - hlsl_block_add_instr(args->instrs, &load->node); - } - else - { - struct hlsl_ir_node *expr; - - if (!(expr = hlsl_new_void_expr(ctx, loc))) - goto fail; - hlsl_block_add_instr(args->instrs, expr); - } } else if ((intrinsic = bsearch(name, intrinsic_functions, ARRAY_SIZE(intrinsic_functions), sizeof(*intrinsic_functions), intrinsic_function_name_compare))) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 38 ++++++++++++++++++++++++ libs/vkd3d-shader/hlsl.h | 8 ++++- libs/vkd3d-shader/hlsl.y | 64 +++++++++++----------------------------- 3 files changed, 63 insertions(+), 47 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index aef5a408..2bbb5636 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2984,6 +2984,16 @@ void hlsl_add_function(struct hlsl_ctx *ctx, char *name, struct hlsl_ir_function struct hlsl_ir_function *func; struct rb_entry *func_entry; + if (ctx->is_internal_function) + { + char *internal_name; + + if (!(internal_name = hlsl_sprintf_alloc(ctx, "<internal>%s", name))) + return; + vkd3d_free(name); + name = internal_name; + } + func_entry = rb_get(&ctx->functions, name); if (func_entry) { @@ -3515,3 +3525,31 @@ int hlsl_compile_shader(const struct vkd3d_shader_code *hlsl, const struct vkd3d hlsl_ctx_cleanup(&ctx); return ret; } + +struct hlsl_ir_function_decl *hlsl_compile_internal_function(struct hlsl_ctx *ctx, const char *name, const char *func) +{ + const struct hlsl_ir_function_decl *saved_cur_function = ctx->cur_function; + struct vkd3d_shader_code code = {.code = func, .size = strlen(func)}; + bool saved_is_internal = ctx->is_internal_function; + void *saved_scanner = ctx->scanner; + int ret; + + TRACE("name %s, func %s.\n", debugstr_a(name), debugstr_a(func)); + + /* Save and restore everything that matters. + * Note that saving the scope stack is hard, and shouldn't be necessary. */ + + ctx->scanner = NULL; + ctx->is_internal_function = true; + ctx->cur_function = NULL; + ret = hlsl_lexer_compile(ctx, &code); + ctx->scanner = saved_scanner; + ctx->is_internal_function = saved_is_internal; + ctx->cur_function = saved_cur_function; + if (ret) + { + ERR("Failed to compile intrinsic, error %u.\n", ret); + return NULL; + } + return hlsl_get_func_decl(ctx, name); +} diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index abb73651..49351444 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -833,6 +833,10 @@ struct hlsl_ctx * compute shader profiles. It is set using the numthreads() attribute in the entry point. */ uint32_t thread_count[3]; + /* If is_internal_function is set, we are currently parsing an HLSL string + * in order to define an internal function. */ + bool is_internal_function; + /* Whether the parser is inside a state block (effects' metadata) inside a variable declaration. */ uint32_t in_state_block : 1; /* Whether the numthreads() attribute has been provided in the entry-point function. */ @@ -1068,7 +1072,7 @@ static inline unsigned int hlsl_sampler_dim_count(enum hlsl_sampler_dim dim) } } -char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...); +char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...) VKD3D_PRINTF_FUNC(2, 3); const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op); const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type); @@ -1259,6 +1263,8 @@ bool hlsl_sm4_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_sem bool output, enum vkd3d_shader_register_type *type, enum vkd3d_sm4_swizzle_type *swizzle_type, bool *has_idx); int hlsl_sm4_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, struct vkd3d_shader_code *out); +struct hlsl_ir_function_decl *hlsl_compile_internal_function(struct hlsl_ctx *ctx, const char *name, const char *func); + int hlsl_lexer_compile(struct hlsl_ctx *ctx, const struct vkd3d_shader_code *hlsl); #endif diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 4e3a10c8..a696e666 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3422,58 +3422,30 @@ static bool intrinsic_sin(struct hlsl_ctx *ctx, static bool intrinsic_smoothstep(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { - struct hlsl_ir_node *min_arg, *max_arg, *x_arg, *p, *p_num, *p_denom, *res, *one, *minus_two, *three; - - if (!elementwise_intrinsic_float_convert_args(ctx, params, loc)) - return false; - - min_arg = params->args[0]; - max_arg = params->args[1]; - x_arg = params->args[2]; - - if (!(min_arg = add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_NEG, min_arg, loc))) - return false; - - if (!(p_num = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, x_arg, min_arg, loc))) - return false; - - if (!(p_denom = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, max_arg, min_arg, loc))) - return false; - - if (!(one = hlsl_new_float_constant(ctx, 1.0, loc))) - return false; - hlsl_block_add_instr(params->instrs, one); - - if (!(p_denom = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_DIV, one, p_denom, loc))) - return false; - - if (!(p = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p_num, p_denom, loc))) - return false; - - if (!(p = add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_SAT, p, loc))) - return false; - - if (!(minus_two = hlsl_new_float_constant(ctx, -2.0, loc))) - return false; - hlsl_block_add_instr(params->instrs, minus_two); - - if (!(three = hlsl_new_float_constant(ctx, 3.0, loc))) - return false; - hlsl_block_add_instr(params->instrs, three); - - if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, minus_two, p, loc))) - return false; + struct hlsl_ir_function_decl *func; + struct hlsl_type *type; + char *name, *body; - if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, three, res, loc))) - return false; + static const char template[] = + "%s smoothstep_%s(%s low, %s high, %s x)\n" + "{\n" + " %s p = saturate((x - low) / (high - low));\n" + " return (p * p) * (3 - 2 * p);\n" + "}"; - if (!(p = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, p, loc))) + if (!(type = elementwise_intrinsic_get_common_type(ctx, params, loc))) return false; + type = hlsl_get_numeric_type(ctx, type->class, HLSL_TYPE_FLOAT, type->dimx, type->dimy); - if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, res, loc))) + name = hlsl_sprintf_alloc(ctx, "<internal>smoothstep_%s", type->name); + body = hlsl_sprintf_alloc(ctx, template, type->name, type->name, type->name, type->name, type->name, type->name); + func = hlsl_compile_internal_function(ctx, name, body); + vkd3d_free(name); + vkd3d_free(body); + if (!func) return false; - return true; + return add_user_call(ctx, func, params, loc); } static bool intrinsic_sqrt(struct hlsl_ctx *ctx, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 83 +++++++--------------------------------- 1 file changed, 13 insertions(+), 70 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a696e666..2a88a163 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3034,14 +3034,17 @@ static struct hlsl_ir_node * add_pow_expr(struct hlsl_ctx *ctx, static bool intrinsic_lit(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { - struct hlsl_ir_node *n_l_neg, *n_h_neg, *specular_or, *specular_pow, *load; - struct hlsl_ir_node *n_l, *n_h, *m, *diffuse, *zero, *store, *init; - struct hlsl_constant_value init_value; - struct hlsl_ir_load *var_load; - struct hlsl_deref var_deref; - struct hlsl_type *ret_type; - struct hlsl_ir_var *var; - struct hlsl_block block; + struct hlsl_ir_function_decl *func; + + static const char body[] = + "float4 lit(float n_l, float n_h, float m)\n" + "{\n" + " float4 ret;\n" + " ret.xw = 1.0;\n" + " ret.y = max(n_l, 0);\n" + " ret.z = (n_l < 0 || n_h < 0) ? 0 : pow(n_h, m);\n" + " return ret;\n" + "}"; if (params->args[0]->data_type->class != HLSL_CLASS_SCALAR || params->args[1]->data_type->class != HLSL_CLASS_SCALAR @@ -3051,70 +3054,10 @@ static bool intrinsic_lit(struct hlsl_ctx *ctx, return false; } - if (!(n_l = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) - return false; - - if (!(n_h = intrinsic_float_convert_arg(ctx, params, params->args[1], loc))) + if (!(func = hlsl_compile_internal_function(ctx, "<internal>lit", body))) return false; - if (!(m = intrinsic_float_convert_arg(ctx, params, params->args[2], loc))) - return false; - - ret_type = hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 4); - - if (!(var = hlsl_new_synthetic_var(ctx, "lit", ret_type, loc))) - return false; - hlsl_init_simple_deref_from_var(&var_deref, var); - - init_value.u[0].f = 1.0f; - init_value.u[1].f = 0.0f; - init_value.u[2].f = 0.0f; - init_value.u[3].f = 1.0f; - if (!(init = hlsl_new_constant(ctx, ret_type, &init_value, loc))) - return false; - hlsl_block_add_instr(params->instrs, init); - - if (!(store = hlsl_new_simple_store(ctx, var, init))) - return false; - hlsl_block_add_instr(params->instrs, store); - - if (!(zero = hlsl_new_float_constant(ctx, 0.0f, loc))) - return false; - hlsl_block_add_instr(params->instrs, zero); - - /* Diffuse component. */ - if (!(diffuse = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MAX, n_l, zero, loc))) - return false; - - if (!hlsl_new_store_component(ctx, &block, &var_deref, 1, diffuse)) - return false; - hlsl_block_add_block(params->instrs, &block); - - /* Specular component. */ - if (!(n_h_neg = add_binary_comparison_expr(ctx, params->instrs, HLSL_OP2_LESS, n_h, zero, loc))) - return false; - - if (!(n_l_neg = add_binary_comparison_expr(ctx, params->instrs, HLSL_OP2_LESS, n_l, zero, loc))) - return false; - - if (!(specular_or = add_binary_logical_expr(ctx, params->instrs, HLSL_OP2_LOGIC_OR, n_l_neg, n_h_neg, loc))) - return false; - - if (!(specular_pow = add_pow_expr(ctx, params->instrs, n_h, m, loc))) - return false; - - if (!(load = hlsl_add_conditional(ctx, params->instrs, specular_or, zero, specular_pow))) - return false; - - if (!hlsl_new_store_component(ctx, &block, &var_deref, 2, load)) - return false; - hlsl_block_add_block(params->instrs, &block); - - if (!(var_load = hlsl_new_var_load(ctx, var, loc))) - return false; - hlsl_block_add_instr(params->instrs, &var_load->node); - - return true; + return add_user_call(ctx, func, params, loc); } static bool intrinsic_log(struct hlsl_ctx *ctx, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310
We've been adding increasingly complex intrinsics, and after looking at an implementation of inverse trigonometry intrinsics sent to me privately, I've decided that reviewing particularly complex intrinsics written in C is getting unwieldy. At least, it's difficult to actually double-check the math like that. So I decided to see how difficult it would be to define these functions in HLSL instead. The main reason we haven't done that historically is because most of these functions are not really expressible in HLSL, mostly by virtue of taking polymorphic type arguments. In this patch series I resolve that by basically generating type variants one at a time when the function is invoked. Apart from that the implementation is relatively simple. We reuse the same hlsl_ctx but a new flex scanner, and save and restore a couple of ctx members so that we're in global scope, then we just invoke the function with a HLSL_IR_CALL, the same way we do with user functions. In theory, we could reuse this code for complex lowering intrinsics as well—float modulus and integer div/mod come to mind. Currently what makes that awkward is that those lowering passes are done on the entry point after inlining calls, while invoking HLSL-defined functions requires another HLSL_IR_CALL. But this is a relatively simple problem to fix; probably we either run lowering passes earlier, on all functions, or just rerun function inlining. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_42946
Here are the inverse trig intrinsics Zeb is talking about: https://gitlab.winehq.org/petrathekat/vkd3d/-/compare/master...d3d-inverse-t... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43038
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl_codegen.c:
} }
- hlsl_release_string_buffer(ctx, name);
I think we should ```c vkd3d_free(new_name); ``` here. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43628
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.h:
} }
-char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...);
Is there any particular reason to add this `__attribute__((format))` in 3/4 instead of 1/4? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43629
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
- return false; + static const char template[] = + "%s smoothstep_%s(%s low, %s high, %s x)\n" + "{\n" + " %s p = saturate((x - low) / (high - low));\n" + " return (p * p) * (3 - 2 * p);\n" + "}";
- if (!(p = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, p, loc))) + if (!(type = elementwise_intrinsic_get_common_type(ctx, params, loc))) return false; + type = hlsl_get_numeric_type(ctx, type->class, HLSL_TYPE_FLOAT, type->dimx, type->dimy);
- if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, res, loc))) + name = hlsl_sprintf_alloc(ctx, "<internal>smoothstep_%s", type->name); + body = hlsl_sprintf_alloc(ctx, template, type->name, type->name, type->name, type->name, type->name, type->name); Should this account for the possibility of the hlsl_sprintf_alloc() calls returning NULL?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43630
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.c:
ERR("Failed to insert function overload.\n"); vkd3d_free(name); return; }
Note that we are triggering this ERR if we have multiple calls to the same intrinsic, e.g.: ```hlsl float4 main(uniform float4 u) : sv_target { return lit(u.x, u.y, u.z) + lit(u.x, u.y, u.z); } ``` because we are declaring the internal multiple times. Perhaps hlsl_compile_internal_function() should check if the internal with the given name is already compiled. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43631
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
- hlsl_block_add_instr(params->instrs, minus_two); - - if (!(three = hlsl_new_float_constant(ctx, 3.0, loc))) - return false; - hlsl_block_add_instr(params->instrs, three); - - if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, minus_two, p, loc))) - return false; + struct hlsl_ir_function_decl *func; + struct hlsl_type *type; + char *name, *body;
- if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, three, res, loc))) - return false; + static const char template[] = + "%s smoothstep_%s(%s low, %s high, %s x)\n" Do we need to manually define the internal function with a new name for each overload?
I see that this is currently the case because hlsl_compile_internal_function() uses hlsl_get_func_decl() internally. But, since we are compiling the function once for each call to the intrinsic anyways, what if we change hlsl_compile_internal_function() slightly so that it only expects the return type, the clean name of the function (e.g. just "smoothstep") and the body without the signature? This so that only hlsl_compile_internal_function() has to care about the real internal name of the compiled function, maybe using an internal counter to give them a unique name e.g. `<internal-42>smoothstep`. That way we don't have to care about overloads. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43632
I think this is a very useful tool. I am ready to approve after my comments have been addressed. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43633
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.c:
ctx->result = VKD3D_ERROR_NOT_IMPLEMENTED; }
+char *hlsl_sprintf_alloc(struct hlsl_ctx *ctx, const char *fmt, ...) +{ + struct vkd3d_string_buffer *string; + va_list args; + char *ret; + + if (!(string = hlsl_get_string_buffer(ctx))) + return NULL; + va_start(args, fmt); + vkd3d_string_buffer_vprintf(string, fmt, args);
I guess if this fails an incomplete buffer is returned. Maybe we should return `NULL` and set the context result in that case. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43653
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/d3dbc.c:
if (var->is_param && var->is_uniform) { - struct vkd3d_string_buffer *name; + char *new_name;
- if (!(name = hlsl_get_string_buffer(ctx))) - { - buffer->status = VKD3D_ERROR_OUT_OF_MEMORY; - return; - } - vkd3d_string_buffer_printf(name, "$%s", var->name); + if (!(new_name = hlsl_sprintf_alloc(ctx, "$%s", var->name))) + continue;
Is the change from `return` to `continue` intentional? If there is a memory allocation error I guess we should unwind the stack as soon as possible. Maybe even allowing `write_sm1_uniforms()` and similar functions to return an error. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43654
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
- return false; + static const char template[] = + "%s smoothstep_%s(%s low, %s high, %s x)\n" + "{\n" + " %s p = saturate((x - low) / (high - low));\n" + " return (p * p) * (3 - 2 * p);\n" + "}";
- if (!(p = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, p, loc))) + if (!(type = elementwise_intrinsic_get_common_type(ctx, params, loc))) return false; + type = hlsl_get_numeric_type(ctx, type->class, HLSL_TYPE_FLOAT, type->dimx, type->dimy);
- if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, p, res, loc))) + name = hlsl_sprintf_alloc(ctx, "<internal>smoothstep_%s", type->name); + body = hlsl_sprintf_alloc(ctx, template, type->name, type->name, type->name, type->name, type->name, type->name); You don't catch allocation failures, do you?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43655
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
- hlsl_block_add_instr(params->instrs, three); - - if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, minus_two, p, loc))) - return false; + struct hlsl_ir_function_decl *func; + struct hlsl_type *type; + char *name, *body;
- if (!(res = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, three, res, loc))) - return false; + static const char template[] = + "%s smoothstep_%s(%s low, %s high, %s x)\n" + "{\n" + " %s p = saturate((x - low) / (high - low));\n" + " return (p * p) * (3 - 2 * p);\n" + "}"; POSIX allows to use the `%m$` syntax and avoid repeat many times the same argument, but unfortunately the standard `printf()` doesn't...
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43656
On Mon Aug 28 23:51:53 2023 +0000, Francisco Casas wrote:
Do we need to manually define the internal function with a new name for each overload? I see that this is currently the case because hlsl_compile_internal_function() uses hlsl_get_func_decl() internally. But, since we are compiling the function once for each call to the intrinsic anyways, what if we change hlsl_compile_internal_function() slightly so that it only expects the return type, the clean name of the function (e.g. just "smoothstep") and the body without the signature? This so that only hlsl_compile_internal_function() has to care about the real internal name of the compiled function, maybe using an internal counter to give them a unique name e.g. `<internal-42>smoothstep`. That way we don't have to care about overloads. I think I would also be in favor of generating a new name each time an internal function is parsed.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43657
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.c:
+ bool saved_is_internal = ctx->is_internal_function; + void *saved_scanner = ctx->scanner; + int ret; + + TRACE("name %s, func %s.\n", debugstr_a(name), debugstr_a(func)); + + /* Save and restore everything that matters. + * Note that saving the scope stack is hard, and shouldn't be necessary. */ + + ctx->scanner = NULL; + ctx->is_internal_function = true; + ctx->cur_function = NULL; + ret = hlsl_lexer_compile(ctx, &code); + ctx->scanner = saved_scanner; + ctx->is_internal_function = saved_is_internal; + ctx->cur_function = saved_cur_function; I can't say I am a fan of hijacking the context in this way, but overall I can live with it.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/310#note_43658
participants (5)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Petrichor Park (＠petrathekat)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)