On 10/26/21 07:48, Matteo Bruni wrote:
On Fri, Oct 15, 2021 at 11:54 PM Zebediah Figura zfigura@codeweavers.com wrote:
As a cheap substitute for actual variable copy-prop.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
libs/vkd3d-shader/hlsl_codegen.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index df10ca272..495ba7c47 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -34,6 +34,13 @@ static void prepend_uniform_copy(struct hlsl_ctx *ctx, struct list *instrs, stru /* Use the synthetic name for the temp, rather than the uniform, so that we * can write the uniform name into the shader reflection data. */
- if (!temp->first_write)
- {
temp->is_uniform = 1;list_add_tail(&ctx->extern_vars, &temp->extern_entry);return;- }
if (!(uniform = hlsl_new_var(ctx, temp->name, temp->data_type, temp->loc, NULL, 0, &temp->reg_reservation))) return; list_add_before(&temp->scope_entry, &uniform->scope_entry);Technically this hunk is not covered by the patch subject.
I'm not sure I love these checks being in the functions rather than in the caller. It might just be about function naming... I guess what's bugging me is that, for "good" uniforms, prepend_uniform_copy() will NOT prepend any copy but still do something significant.
It's a fair cop.
I suppose it could be renamed to lower_uniform_writes() etc...
@@ -67,6 +74,14 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct struct hlsl_ir_load *load; struct hlsl_ir_var *input;
- /* We still need to split up non-vector types. */
- if (!var->first_write && var->data_type->type <= HLSL_CLASS_VECTOR)
- {
var->is_input_semantic = 1;list_add_tail(&ctx->extern_vars, &var->extern_entry);return;- }
if (!(name = hlsl_get_string_buffer(ctx))) return; vkd3d_string_buffer_printf(name, "<input-%s%u>", semantic->name, semantic->index);It's not obvious what splitting up input matrices (which is not there at all at the moment) has to do with adding a copy from an input variable.
Are input matrices even possible?
I'll have to clarify the comment in any case...
@@ -138,6 +153,14 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct struct hlsl_ir_var *output; struct hlsl_ir_load *load;
- /* We still need to split up non-vector types. */
- if (!var->last_read && var->data_type->type <= HLSL_CLASS_VECTOR)
- {
var->is_output_semantic = 1;list_add_tail(&ctx->extern_vars, &var->extern_entry);return;- }
if (!(name = hlsl_get_string_buffer(ctx))) return; vkd3d_string_buffer_printf(name, "<output-%s%u>", semantic->name, semantic->index);@@ -1306,6 +1329,12 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun
list_move_head(&body->instrs, &ctx->static_initializers);
- /* Do an initial liveness pass for externs so that we don't create
* unnecessary temps. Note that we might modify the instruction stream* during the pass, but we're only checking whether a variable was read to* or written from at all, so it's okay. */- compute_liveness(ctx, entry_func);
We shouldn't be modifying the instructions (aside from liveness data, of course) in compute_liveness(). I guess you mean that we can safely keep using liveness data that potentially becomes somewhat invalid during / after calling prepend_uniform_copy() / prepend_input_var_copy() / append_output_var_copy() but I'm having a hard time parsing the comment.
Er, right, "the pass" means the pass after compute_liveness()...
Clearly this patch needs more clarity.