On Tue, 2015-10-20 at 00:12 -0700, Jordan Justen wrote: > An untyped surface read is volatile because it might be affected by a > write. > > In the ES31-CTS.compute_shader.resources-max test, two back to back > read/modify/writes of an SSBO variable looked something like this: > > r1 = untyped_surface_read(ssbo_float) > r2 = r1 + 1 > untyped_surface_write(ssbo_float, r2) > r3 = untyped_surface_read(ssbo_float) > r4 = r3 + 1 > untyped_surface_write(ssbo_float, r4) > > And after CSE, we had: > > r1 = untyped_surface_read(ssbo_float) > r2 = r1 + 1 > untyped_surface_write(ssbo_float, r2) > r4 = r1 + 1 > untyped_surface_write(ssbo_float, r4)
Yeah, we cannot do CSE with SSBO loads. Patch looks good to me, but we should do the same in the vec4 CSE pass. Iago > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > --- > src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++- > src/mesa/drivers/dri/i965/brw_shader.cpp | 14 ++++++++++++++ > src/mesa/drivers/dri/i965/brw_shader.h | 6 ++++++ > 3 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > index c7628dc..3a28c8d 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > @@ -93,7 +93,8 @@ is_expression(const fs_visitor *v, const fs_inst *const > inst) > case SHADER_OPCODE_LOAD_PAYLOAD: > return !inst->is_copy_payload(v->alloc); > default: > - return inst->is_send_from_grf() && !inst->has_side_effects(); > + return inst->is_send_from_grf() && !inst->has_side_effects() && > + !inst->is_volatile(); > } > } > > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp > b/src/mesa/drivers/dri/i965/brw_shader.cpp > index 2324b56..be911ed 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp > @@ -969,6 +969,20 @@ backend_instruction::has_side_effects() const > } > } > > +bool > +backend_instruction::is_volatile() const > +{ > + switch (opcode) { > + case SHADER_OPCODE_UNTYPED_SURFACE_READ: > + case SHADER_OPCODE_UNTYPED_SURFACE_READ_LOGICAL: > + case SHADER_OPCODE_TYPED_SURFACE_READ: > + case SHADER_OPCODE_TYPED_SURFACE_READ_LOGICAL: > + return true; > + default: > + return false; > + } > +} > + > #ifndef NDEBUG > static bool > inst_is_in_block(const bblock_t *block, const backend_instruction *inst) > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h > b/src/mesa/drivers/dri/i965/brw_shader.h > index b33b08f..35ee210 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.h > +++ b/src/mesa/drivers/dri/i965/brw_shader.h > @@ -115,6 +115,12 @@ struct backend_instruction : public exec_node { > * optimize these out unless you know what you are doing. > */ > bool has_side_effects() const; > + > + /** > + * True if the instruction might be affected by side effects of other > + * instructions. > + */ > + bool is_volatile() const; > #else > struct backend_instruction { > struct exec_node link; _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev