From 627c15cde46a76e9bce4425646c5caba11788ec4 Mon Sep 17 00:00:00 2001 From: Jordan Justen Date: Mon, 19 Oct 2015 23:13:09 -0700 Subject: [PATCH] i965/fs: Disable CSE optimization for untyped & typed surface reads 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) Signed-off-by: Jordan Justen Reviewed-by: Iago Toral Quiroga --- 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 c7628dcc2f4..3a28c8d591d 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 6f6f77ab583..204935641f3 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -987,6 +987,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 51b059fcaa1..6a2dfc9bbb6 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;