Caio Marcelo de Oliveira Filho <caio.olive...@intel.com> writes:
> When handling 'if' in constant propagation, if a certain variable was
> killed when processing the first branch of the 'if', then the second
> would get any propagation from previous nodes. This is similar to the
> change done for copy propagation code.
>
> x = 1;
> if (...) {
> z = x; // This would turn into z = 1.
> x = 22; // x gets killed.
> } else {
> w = x; // This would NOT turn into w = 1.
> }
>
> With the change, we let constant propagation happen independently in
> the two branches and only then apply the killed values for the
> subsequent code.
>
> The new code use a single hash table for keeping the kills of both
> branches (the branches only write to it), and it gets deleted after we
> use -- instead of waiting for mem_ctx to collect it.
>
> NIR deals well with constant propagation, so it already covered for
> the missing ones that this patch fixes.
> ---
> .../glsl/opt_constant_propagation.cpp | 43 +++++++++++--------
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/src/compiler/glsl/opt_constant_propagation.cpp
> b/src/compiler/glsl/opt_constant_propagation.cpp
> index 05dc71efb72..25db3622aba 100644
> --- a/src/compiler/glsl/opt_constant_propagation.cpp
> +++ b/src/compiler/glsl/opt_constant_propagation.cpp
> @@ -122,7 +122,7 @@ public:
> void constant_folding(ir_rvalue **rvalue);
> void constant_propagation(ir_rvalue **rvalue);
> void kill(ir_variable *ir, unsigned write_mask);
> - void handle_if_block(exec_list *instructions);
> + void handle_if_block(exec_list *instructions, hash_table *kills, bool
> *killed_all);
> void handle_rvalue(ir_rvalue **rvalue);
>
> /** List of acp_entry: The available constants to propagate */
> @@ -356,15 +356,14 @@ ir_constant_propagation_visitor::visit_enter(ir_call
> *ir)
> }
>
> void
> -ir_constant_propagation_visitor::handle_if_block(exec_list *instructions)
> +ir_constant_propagation_visitor::handle_if_block(exec_list *instructions,
> hash_table *kills, bool *killed_all)
> {
> exec_list *orig_acp = this->acp;
> hash_table *orig_kills = this->kills;
> bool orig_killed_all = this->killed_all;
>
> this->acp = new(mem_ctx) exec_list;
> - this->kills = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer,
> - _mesa_key_pointer_equal);
> + this->kills = kills;
> this->killed_all = false;
>
> /* Populate the initial acp with a constant of the original */
> @@ -374,20 +373,10 @@
> ir_constant_propagation_visitor::handle_if_block(exec_list *instructions)
>
> visit_list_elements(this, instructions);
>
> - if (this->killed_all) {
> - orig_acp->make_empty();
> - }
> -
> - hash_table *new_kills = this->kills;
> + *killed_all = this->killed_all;
> this->kills = orig_kills;
> this->acp = orig_acp;
> - this->killed_all = this->killed_all || orig_killed_all;
> -
> - hash_entry *htk;
> - hash_table_foreach(new_kills, htk) {
> - kill_entry *k = (kill_entry *) htk->data;
> - kill(k->var, k->write_mask);
> - }
> + this->killed_all = orig_killed_all;
> }
>
> ir_visitor_status
> @@ -396,8 +385,26 @@ ir_constant_propagation_visitor::visit_enter(ir_if *ir)
> ir->condition->accept(this);
> handle_rvalue(&ir->condition);
>
> - handle_if_block(&ir->then_instructions);
> - handle_if_block(&ir->else_instructions);
> + hash_table *new_kills = _mesa_hash_table_create(mem_ctx,
> _mesa_hash_pointer,
> + _mesa_key_pointer_equal);
> + bool then_killed_all = false;
> + bool else_killed_all = false;
> +
> + handle_if_block(&ir->then_instructions, new_kills, &then_killed_all);
> + handle_if_block(&ir->else_instructions, new_kills, &else_killed_all);Passing in the new_kills the second time and using it as the starting kills looked strange, but since nobody else will kill from that list until our block below, it seems like this patch does what it meant to. Reviewed-by: Eric Anholt <e...@anholt.net> Possible follow-up change for someone looking at compiler perf: kill_entry doesn't seem to need to be a list since 4654439fdd766f79a78fe0d812fd916f5815e7e6, and we could probably just store the write_mask in the entry->data field and not have struct kill_entry at all!
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
