On 22.05.2017 12:18, Marek Olšák wrote:
On Mon, May 22, 2017 at 8:44 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
On 19.05.2017 17:54, Marek Olšák wrote:
From: Marek Olšák <marek.ol...@amd.com>
---
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.
Okay, that answers my question. Please add this explanation about glc=1,
coherent=false to the comment in the function. For the series:
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
Marek
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
