From f7581e4a383f656ea7469633a39f89fe0e3ec495 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Tue, 15 Jul 2025 12:58:22 -0700 Subject: [PATCH] brw: consider 'volatile' memory access when doing CSE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GLSL spec says (among other things): "When a volatile variable is read, its value must be re-fetched from the underlying memory, even if the shader invocation performing the read had previously fetched its value from the same memory. When a volatile variable is written, its value must be written to the underlying memory, even if the compiler can conclusively determine that its value will be overwritten by a subsequent write." The SPIR-V spec says (among other things): "Accesses to volatile memory cannot be eliminated, duplicated, or combined with other accesses." So in this commit we make sure that both writes and reads marked as volatile can't be affected by CSE. v2: Reorder patches in the series. Credits-to: Caio Oliveira Reviewed-by: Caio Oliveira Reviewed-by: Lionel Landwerlin (v1) Reviewed-by: Iván Briano (v1) Signed-off-by: Paulo Zanoni Part-of: --- src/intel/compiler/brw_inst.cpp | 17 +++++++++++++---- src/intel/compiler/brw_lower_logical_sends.cpp | 6 ++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/intel/compiler/brw_inst.cpp b/src/intel/compiler/brw_inst.cpp index 183eec8e2c8..362ee262a24 100644 --- a/src/intel/compiler/brw_inst.cpp +++ b/src/intel/compiler/brw_inst.cpp @@ -956,10 +956,19 @@ brw_inst::has_side_effects() const bool brw_inst::is_volatile() const { - return opcode == SHADER_OPCODE_MEMORY_LOAD_LOGICAL || - opcode == SHADER_OPCODE_LOAD_REG || - ((opcode == SHADER_OPCODE_SEND || - opcode == SHADER_OPCODE_SEND_GATHER) && send_is_volatile); + switch (opcode) { + case SHADER_OPCODE_MEMORY_LOAD_LOGICAL: + case SHADER_OPCODE_LOAD_REG: + return true; + case SHADER_OPCODE_MEMORY_STORE_LOGICAL: + assert(sources > MEMORY_LOGICAL_FLAGS); + return src[MEMORY_LOGICAL_FLAGS].ud & MEMORY_FLAG_VOLATILE_ACCESS; + case SHADER_OPCODE_SEND: + case SHADER_OPCODE_SEND_GATHER: + return send_is_volatile; + default: + return false; + } } void diff --git a/src/intel/compiler/brw_lower_logical_sends.cpp b/src/intel/compiler/brw_lower_logical_sends.cpp index b1dc907cb38..1aab7ca644d 100644 --- a/src/intel/compiler/brw_lower_logical_sends.cpp +++ b/src/intel/compiler/brw_lower_logical_sends.cpp @@ -1512,6 +1512,7 @@ lower_lsc_memory_logical_send(const brw_builder &bld, brw_inst *inst) (enum memory_flags) inst->src[MEMORY_LOGICAL_FLAGS].ud; const bool transpose = flags & MEMORY_FLAG_TRANSPOSE; const bool include_helpers = flags & MEMORY_FLAG_INCLUDE_HELPERS; + const bool volatile_access = flags & MEMORY_FLAG_VOLATILE_ACCESS; const brw_reg data0 = inst->src[MEMORY_LOGICAL_DATA0]; const brw_reg data1 = inst->src[MEMORY_LOGICAL_DATA1]; const bool has_side_effects = inst->has_side_effects(); @@ -1638,7 +1639,7 @@ lower_lsc_memory_logical_send(const brw_builder &bld, brw_inst *inst) inst->ex_mlen = ex_mlen; inst->header_size = 0; inst->send_has_side_effects = has_side_effects; - inst->send_is_volatile = !has_side_effects; + inst->send_is_volatile = !has_side_effects || volatile_access; inst->resize_sources(4); @@ -1708,6 +1709,7 @@ lower_hdc_memory_logical_send(const brw_builder &bld, brw_inst *inst) (enum memory_flags) inst->src[MEMORY_LOGICAL_FLAGS].ud; const bool block = flags & MEMORY_FLAG_TRANSPOSE; const bool include_helpers = flags & MEMORY_FLAG_INCLUDE_HELPERS; + const bool volatile_access = flags & MEMORY_FLAG_VOLATILE_ACCESS; const brw_reg data0 = inst->src[MEMORY_LOGICAL_DATA0]; const brw_reg data1 = inst->src[MEMORY_LOGICAL_DATA1]; const bool has_side_effects = inst->has_side_effects(); @@ -1921,7 +1923,7 @@ lower_hdc_memory_logical_send(const brw_builder &bld, brw_inst *inst) inst->ex_mlen = ex_mlen; inst->header_size = header.file != BAD_FILE ? 1 : 0; inst->send_has_side_effects = has_side_effects; - inst->send_is_volatile = !has_side_effects; + inst->send_is_volatile = !has_side_effects || volatile_access; if (block) { assert(inst->force_writemask_all);