From f45c9a38db78776551ddee28c31a3a20cce287b6 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Tue, 30 Jan 2024 14:08:33 +1100 Subject: [PATCH] glsl: don't tree graft globals As per this optimisations description: "Takes assignments to variables that are dereferenced only once and pastes the RHS expression into where the variables dereferenced." However the optimisation is run at compile time before multiple shaders from the same stage could have been pasted together. So this optimisation can incorrectly assume a global is only referenced once since it cannot see the other pieces of the shader stage until link time. Here we skip the optimisation if the variable is a global. We could change it to only run at link time however this optimisation is only run at link time if we are being forced to use GLSL IR to inline a function that glsl to nir cannot handle and this will also be removed in a future patchset. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10482 Fixes: d75a36a9eeb1 ("glsl: remove do_copy_propagation_elements() optimisation pass") Acked-by: Pierre-Eric Pelloux-Prayer Part-of: (cherry picked from commit bc0178af57fe5e328580190806354982e1c41e16) --- .pick_status.json | 2 +- src/compiler/glsl/ir_variable_refcount.cpp | 9 ++++++++- src/compiler/glsl/ir_variable_refcount.h | 5 +++++ src/compiler/glsl/opt_tree_grafting.cpp | 3 ++- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 3312b0b2e97..30c62fd4c16 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1624,7 +1624,7 @@ "description": "glsl: don't tree graft globals", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "d75a36a9eeb1606fab19362746f9b5d94b98bd3a", "notes": null diff --git a/src/compiler/glsl/ir_variable_refcount.cpp b/src/compiler/glsl/ir_variable_refcount.cpp index 47e9d0c897c..0f514b8442a 100644 --- a/src/compiler/glsl/ir_variable_refcount.cpp +++ b/src/compiler/glsl/ir_variable_refcount.cpp @@ -39,6 +39,7 @@ ir_variable_refcount_visitor::ir_variable_refcount_visitor() { this->mem_ctx = ralloc_context(NULL); this->ht = _mesa_pointer_hash_table_create(NULL); + this->global = true; } static void @@ -94,8 +95,10 @@ ir_visitor_status ir_variable_refcount_visitor::visit(ir_variable *ir) { ir_variable_refcount_entry *entry = this->get_variable_entry(ir); - if (entry) + if (entry) { entry->declaration = true; + entry->is_global = this->global; + } return visit_continue; } @@ -117,10 +120,14 @@ ir_variable_refcount_visitor::visit(ir_dereference_variable *ir) ir_visitor_status ir_variable_refcount_visitor::visit_enter(ir_function_signature *ir) { + global = false; + /* We don't want to descend into the function parameters and * dead-code eliminate them, so just accept the body here. */ visit_list_elements(this, &ir->body); + + global = true; return visit_continue_with_parent; } diff --git a/src/compiler/glsl/ir_variable_refcount.h b/src/compiler/glsl/ir_variable_refcount.h index 4a90f08c91f..a31e5d59bf5 100644 --- a/src/compiler/glsl/ir_variable_refcount.h +++ b/src/compiler/glsl/ir_variable_refcount.h @@ -62,6 +62,9 @@ public: unsigned assigned_count; bool declaration; /* If the variable had a decl in the instruction stream */ + + /** Is the variable a global */ + bool is_global; }; class ir_variable_refcount_visitor : public ir_hierarchical_visitor { @@ -86,6 +89,8 @@ public: struct hash_table *ht; void *mem_ctx; + + bool global; }; #endif /* GLSL_IR_VARIABLE_REFCOUNT_H */ diff --git a/src/compiler/glsl/opt_tree_grafting.cpp b/src/compiler/glsl/opt_tree_grafting.cpp index a3f8a61cb4b..e5b96176dba 100644 --- a/src/compiler/glsl/opt_tree_grafting.cpp +++ b/src/compiler/glsl/opt_tree_grafting.cpp @@ -386,7 +386,8 @@ tree_grafting_basic_block(ir_instruction *bb_first, if (!entry->declaration || entry->assigned_count != 1 || - entry->referenced_count != 2) + entry->referenced_count != 2 || + entry->is_global) continue; /* Found a possibly graftable assignment. Now, walk through the