On Mon, May 22, 2017 at 8:44 AM, Nicolai Hähnle <[email protected]> wrote: > On 19.05.2017 17:54, Marek Olšák wrote: >> >> From: Marek Olšák <[email protected]> >> >> --- >> src/amd/common/ac_llvm_build.c | 50 >> ++++++++++++++++++++++++-------- >> src/amd/common/ac_llvm_build.h | 3 +- >> src/amd/common/ac_nir_to_llvm.c | 2 +- >> src/gallium/drivers/radeonsi/si_shader.c | 23 +++++++-------- >> 4 files changed, 51 insertions(+), 27 deletions(-) >> >> diff --git a/src/amd/common/ac_llvm_build.c >> b/src/amd/common/ac_llvm_build.c >> index 87a1fb7..4b048ba 100644 >> --- a/src/amd/common/ac_llvm_build.c >> +++ b/src/amd/common/ac_llvm_build.c >> @@ -626,47 +626,73 @@ ac_build_buffer_store_dword(struct ac_llvm_context >> *ctx, >> LLVMValueRef >> ac_build_buffer_load(struct ac_llvm_context *ctx, >> LLVMValueRef rsrc, >> int num_channels, >> LLVMValueRef vindex, >> LLVMValueRef voffset, >> LLVMValueRef soffset, >> unsigned inst_offset, >> unsigned glc, >> unsigned slc, >> - bool readonly_memory) >> + bool readonly_memory, >> + bool coherent) > > > I think the series is basically fine, but the use of "coherent" here is > potentially confusing because "coherent" is already used as the memory > qualifier in shaders, and it sets glc. > > Are there any buffer loads with readonly_memory && !glc && !vindex && > !voffset where we aren't allowed to use SMEM? If possible, we could just use > that to control whether llvm.SI.load.const is used. > > I seem to recall that there were some subtle differences in bounds checking, > and perhaps those prevent the use of SMEM sometimes. But then I'd rather use > a more explicit parameter name like smem_forbidden or smem_allowed.
readonly_memory is now slightly misleading. It means that there are no stores/atomics to the same memory in the current shader, and it enforces READNONE. You need both non-coherent (stores in other shader invocations, if any, are not coherent with the current shader) and readonly_memory (no stores in the current shader touch the memory). Note that stores in the current shader are always coherent, that's why there must be none. Also, relying on GLC would be wrong, because GLC is also used to work around hw bugs, so GLC=1 can be set even when it's OK to use SMEM. Marek _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
