From 26d59acadd18ca9e7eb0cbe98c91b127d0a7bcaa 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 96075929488..300e943d988 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -414,7 +414,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 b7f5e5b47aa..ce682bb9c39 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