On 15 September 2013 00:10, Francisco Jerez <curroje...@riseup.net> wrote:
> And fix the dead code elimination pass so atomic writes aren't
> optimized out in cases where the return value isn't used by the
> program.
>
As I mentioned in my comments on patch 9, I'd prefer if we went with a
different approach where we don't add ir_rvalue nodes that have side
effects at all, but instead stick with our current system where only
ir_instruction nodes can have side effects.
However, if we do wind up going with this approach, we'll need to make
has_side_effects() recurse through the expression tree, so that it can see
that expressions like (atomicCounterIncrement(c) + 1) and
foo[atomicCounterIncrement(c)] also have side effects.
We would also need to modify several other optimization passes to check for
side effects:
- opt_dead_code_local (which does the same job as opt_dead_code, but within
basic blocks)
- opt_tree_grafting - as is, this would try to transform:
uint x = atomicCounterIncrement(c);
uint y = atomicCounterIncrement(c);
uvec3 v = uvec3(y, x, y);
Into:
uint y = atomicCounterIncrement(c);
uvec3 v = uvec3(y, atomicCounterIncrement(c), y);
Which is not equivalent.
- lower_variable_index_to_cond_assign, lower_vec_index_to_cond_assign, and
lower_vector_insert (which sometimes clone arbitrary rvalues)
And possibly others I've missed.
There's also at least one contrived case which is propably not worth fixing:
- opt_flip_matrices (in principle this might try to rewrite
gl_TextureMatrix[atomicCounterIncrement(c)] *
foo[atomicCounterIncrement(c)] to foo[atomicCounterIncrement(c)] *
gl_TextureMatrixTranspose(c), which is technically not the same operation).
> ---
> src/glsl/ir.h | 16 ++++++++++++++++
> src/glsl/opt_dead_code.cpp | 3 ++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index c4b4677..4f506a3 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -139,6 +139,17 @@ public:
> virtual class ir_jump * as_jump() { return
> NULL; }
> /*@}*/
>
> + /**
> + * Determine if an IR instruction has side effects other than its
> + * returned value(s). Optimization passes are expected to be
> + * especially careful with reordering or removing these, unless
> + * they know what they are doing.
> + */
> + virtual bool has_side_effects() const
> + {
> + return false;
> + }
> +
> protected:
> ir_instruction()
> {
> @@ -2120,6 +2131,11 @@ public:
>
> virtual ir_visitor_status accept(ir_hierarchical_visitor *);
>
> + virtual bool has_side_effects() const
> + {
> + return true;
> + }
> +
> /** Kind of atomic instruction. */
> enum ir_atomic_opcode op;
>
> diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
> index b65e5c2..fd05034 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -81,7 +81,8 @@ do_dead_code(exec_list *instructions, bool
> uniform_locations_assigned)
> */
> if (entry->var->mode != ir_var_function_out &&
> entry->var->mode != ir_var_function_inout &&
> - entry->var->mode != ir_var_shader_out) {
> + entry->var->mode != ir_var_shader_out &&
> + !entry->assign->rhs->has_side_effects()) {
> entry->assign->remove();
> progress = true;
>
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
