Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[AMDGPU] Set glc bit for nontemporal loads on GFX10/11 #89739

Closed
wants to merge 1 commit into from
Closed

[AMDGPU] Set glc bit for nontemporal loads on GFX10/11 #89739

wants to merge 1 commit into from

Conversation

ritter-x2a
Copy link
Member

With glc and slc set, nontemporal loads go to the L2 cache while matching entries from L0 and L1 are evicted.
Without glc set (the state prior to this patch), nontemporal loads would read from the L0 cache and affect its cache state.

See section 8.1.10 in the RDNA 1 and 2 ISA reference [1,2] for gfx10 and section 4.1.1 in the RDNA 3 ISA reference [3] for gfx11.

Fixes SWDEV-456064

[1] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna-shader-instruction-set-architecture.pdf
[2] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna2-shader-instruction-set-architecture.pdf
[3] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna3-shader-instruction-set-architecture-feb-2023_0.pdf

@ritter-x2a
[AMDGPU] Set glc bit for nontemporal loads on GFX10/11
5f1f2bb 
With glc and slc set, nontemporal loads go to the L2 cache while
matching entries from L0 and L1 are evicted.
Without glc set (the state prior to this patch), nontemporal loads would
read from the L0 cache and affect its cache state.

See section 8.1.10 in the RDNA 1 and 2 ISA reference [1,2] for gfx10 and
section 4.1.1 in the RDNA 3 ISA reference [3] for gfx11.

Fixes SWDEV-456064

---

[1] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna-shader-instruction-set-architecture.pdf
[2] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna2-shader-instruction-set-architecture.pdf
[3] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna3-shader-instruction-set-architecture-feb-2023_0.pdf
@ritter-x2a ritter-x2a self-assigned this Apr 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 23, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Fabian Ritter (ritter-x2a)

Changes

With glc and slc set, nontemporal loads go to the L2 cache while matching entries from L0 and L1 are evicted.
Without glc set (the state prior to this patch), nontemporal loads would read from the L0 cache and affect its cache state.

See section 8.1.10 in the RDNA 1 and 2 ISA reference [1,2] for gfx10 and section 4.1.1 in the RDNA 3 ISA reference [3] for gfx11.

Fixes SWDEV-456064

[1] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna-shader-instruction-set-architecture.pdf
[2] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna2-shader-instruction-set-architecture.pdf
[3] https://www.amd.com/content/dam/amd/en/documents/radeon-tech-docs/instruction-set-architectures/rdna3-shader-instruction-set-architecture-feb-2023_0.pdf

Full diff: https://github.com/llvm/llvm-project/pull/89739.diff

5 Files Affected:

  • (modified) llvm/docs/AMDGPUUsage.rst (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp (+6-12)
  • (modified) llvm/test/CodeGen/AMDGPU/memory-legalizer-flat-nontemporal.ll (+8-8)
  • (modified) llvm/test/CodeGen/AMDGPU/memory-legalizer-global-nontemporal.ll (+4-4)
  • (modified) llvm/test/CodeGen/AMDGPU/memory-legalizer-private-nontemporal.ll (+8-8)
diff --git a/llvm/docs/AMDGPUUsage.rst b/llvm/docs/AMDGPUUsage.rst
index 029db00134c09d..428aa02fd23641 100644
--- a/llvm/docs/AMDGPUUsage.rst
+++ b/llvm/docs/AMDGPUUsage.rst
@@ -12192,7 +12192,7 @@ table :ref:`amdgpu-amdhsa-memory-model-code-sequences-gfx10-gfx11-table`.
                                                          - !volatile & nontemporal
 
                                                            1. buffer/global/flat_load
-                                                              slc=1 dlc=1
+                                                              glc=1 slc=1 dlc=1
 
                                                             - If GFX10, omit dlc=1.
 
diff --git a/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp b/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
index 62306fa667b360..16989a01162b6f 100644
--- a/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
@@ -1898,12 +1898,9 @@ bool SIGfx10CacheControl::enableVolatileAndOrNonTemporal(
   }
 
   if (IsNonTemporal) {
-    // For loads setting SLC configures L0 and L1 cache policy to HIT_EVICT
-    // and L2 cache policy to STREAM.
-    // For stores setting both GLC and SLC configures L0 and L1 cache policy
-    // to MISS_EVICT and the L2 cache policy to STREAM.
-    if (Op == SIMemOp::STORE)
-      Changed |= enableGLCBit(MI);
+    // Setting both GLC and SLC configures L0 and L1 cache policy to MISS_EVICT
+    // and the L2 cache policy to STREAM for loads and stores.
+    Changed |= enableGLCBit(MI);
     Changed |= enableSLCBit(MI);
 
     return Changed;
@@ -2170,12 +2167,9 @@ bool SIGfx11CacheControl::enableVolatileAndOrNonTemporal(
   }
 
   if (IsNonTemporal) {
-    // For loads setting SLC configures L0 and L1 cache policy to HIT_EVICT
-    // and L2 cache policy to STREAM.
-    // For stores setting both GLC and SLC configures L0 and L1 cache policy
-    // to MISS_EVICT and the L2 cache policy to STREAM.
-    if (Op == SIMemOp::STORE)
-      Changed |= enableGLCBit(MI);
+    // Setting both GLC and SLC configures L0 and L1 cache policy to MISS_EVICT
+    // and the L2 cache policy to STREAM for loads and stores.
+    Changed |= enableGLCBit(MI);
     Changed |= enableSLCBit(MI);
 
     // Set MALL NOALLOC for load and store instructions.
diff --git a/llvm/test/CodeGen/AMDGPU/memory-legalizer-flat-nontemporal.ll b/llvm/test/CodeGen/AMDGPU/memory-legalizer-flat-nontemporal.ll
index f6fac11d634c5b..ecd14d3c108573 100644
--- a/llvm/test/CodeGen/AMDGPU/memory-legalizer-flat-nontemporal.ll
+++ b/llvm/test/CodeGen/AMDGPU/memory-legalizer-flat-nontemporal.ll
@@ -32,7 +32,7 @@ define amdgpu_kernel void @flat_nontemporal_load_0(
 ; GFX10-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, s1
-; GFX10-WGP-NEXT:    flat_load_dword v2, v[0:1] slc
+; GFX10-WGP-NEXT:    flat_load_dword v2, v[0:1] glc slc
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v0, s2
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, s3
 ; GFX10-WGP-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
@@ -45,7 +45,7 @@ define amdgpu_kernel void @flat_nontemporal_load_0(
 ; GFX10-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, s1
-; GFX10-CU-NEXT:    flat_load_dword v2, v[0:1] slc
+; GFX10-CU-NEXT:    flat_load_dword v2, v[0:1] glc slc
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v0, s2
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, s3
 ; GFX10-CU-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
@@ -122,7 +122,7 @@ define amdgpu_kernel void @flat_nontemporal_load_0(
 ; GFX11-WGP-NEXT:    s_load_b128 s[0:3], s[0:1], 0x0
 ; GFX11-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-WGP-NEXT:    v_dual_mov_b32 v0, s0 :: v_dual_mov_b32 v1, s1
-; GFX11-WGP-NEXT:    flat_load_b32 v2, v[0:1] slc dlc
+; GFX11-WGP-NEXT:    flat_load_b32 v2, v[0:1] glc slc dlc
 ; GFX11-WGP-NEXT:    v_dual_mov_b32 v0, s2 :: v_dual_mov_b32 v1, s3
 ; GFX11-WGP-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
 ; GFX11-WGP-NEXT:    flat_store_b32 v[0:1], v2
@@ -133,7 +133,7 @@ define amdgpu_kernel void @flat_nontemporal_load_0(
 ; GFX11-CU-NEXT:    s_load_b128 s[0:3], s[0:1], 0x0
 ; GFX11-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-CU-NEXT:    v_dual_mov_b32 v0, s0 :: v_dual_mov_b32 v1, s1
-; GFX11-CU-NEXT:    flat_load_b32 v2, v[0:1] slc dlc
+; GFX11-CU-NEXT:    flat_load_b32 v2, v[0:1] glc slc dlc
 ; GFX11-CU-NEXT:    v_dual_mov_b32 v0, s2 :: v_dual_mov_b32 v1, s3
 ; GFX11-CU-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
 ; GFX11-CU-NEXT:    flat_store_b32 v[0:1], v2
@@ -190,7 +190,7 @@ define amdgpu_kernel void @flat_nontemporal_load_1(
 ; GFX10-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-WGP-NEXT:    v_add_co_u32 v0, s0, s0, v0
 ; GFX10-WGP-NEXT:    v_add_co_ci_u32_e64 v1, s0, s1, 0, s0
-; GFX10-WGP-NEXT:    flat_load_dword v2, v[0:1] slc
+; GFX10-WGP-NEXT:    flat_load_dword v2, v[0:1] glc slc
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v0, s2
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, s3
 ; GFX10-WGP-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
@@ -204,7 +204,7 @@ define amdgpu_kernel void @flat_nontemporal_load_1(
 ; GFX10-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-CU-NEXT:    v_add_co_u32 v0, s0, s0, v0
 ; GFX10-CU-NEXT:    v_add_co_ci_u32_e64 v1, s0, s1, 0, s0
-; GFX10-CU-NEXT:    flat_load_dword v2, v[0:1] slc
+; GFX10-CU-NEXT:    flat_load_dword v2, v[0:1] glc slc
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v0, s2
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, s3
 ; GFX10-CU-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
@@ -292,7 +292,7 @@ define amdgpu_kernel void @flat_nontemporal_load_1(
 ; GFX11-WGP-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
 ; GFX11-WGP-NEXT:    v_add_co_u32 v0, s0, s0, v0
 ; GFX11-WGP-NEXT:    v_add_co_ci_u32_e64 v1, null, s1, 0, s0
-; GFX11-WGP-NEXT:    flat_load_b32 v2, v[0:1] slc dlc
+; GFX11-WGP-NEXT:    flat_load_b32 v2, v[0:1] glc slc dlc
 ; GFX11-WGP-NEXT:    v_dual_mov_b32 v1, s3 :: v_dual_mov_b32 v0, s2
 ; GFX11-WGP-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
 ; GFX11-WGP-NEXT:    flat_store_b32 v[0:1], v2
@@ -306,7 +306,7 @@ define amdgpu_kernel void @flat_nontemporal_load_1(
 ; GFX11-CU-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
 ; GFX11-CU-NEXT:    v_add_co_u32 v0, s0, s0, v0
 ; GFX11-CU-NEXT:    v_add_co_ci_u32_e64 v1, null, s1, 0, s0
-; GFX11-CU-NEXT:    flat_load_b32 v2, v[0:1] slc dlc
+; GFX11-CU-NEXT:    flat_load_b32 v2, v[0:1] glc slc dlc
 ; GFX11-CU-NEXT:    v_dual_mov_b32 v1, s3 :: v_dual_mov_b32 v0, s2
 ; GFX11-CU-NEXT:    s_waitcnt vmcnt(0) lgkmcnt(0)
 ; GFX11-CU-NEXT:    flat_store_b32 v[0:1], v2
diff --git a/llvm/test/CodeGen/AMDGPU/memory-legalizer-global-nontemporal.ll b/llvm/test/CodeGen/AMDGPU/memory-legalizer-global-nontemporal.ll
index c95ff2c691a3cb..bedd7205809908 100644
--- a/llvm/test/CodeGen/AMDGPU/memory-legalizer-global-nontemporal.ll
+++ b/llvm/test/CodeGen/AMDGPU/memory-legalizer-global-nontemporal.ll
@@ -213,7 +213,7 @@ define amdgpu_kernel void @global_nontemporal_load_1(
 ; GFX10-WGP-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-WGP-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX10-WGP-NEXT:    global_load_dword v0, v0, s[0:1] slc
+; GFX10-WGP-NEXT:    global_load_dword v0, v0, s[0:1] glc slc
 ; GFX10-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-WGP-NEXT:    global_store_dword v1, v0, s[2:3]
 ; GFX10-WGP-NEXT:    s_endpgm
@@ -224,7 +224,7 @@ define amdgpu_kernel void @global_nontemporal_load_1(
 ; GFX10-CU-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-CU-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX10-CU-NEXT:    global_load_dword v0, v0, s[0:1] slc
+; GFX10-CU-NEXT:    global_load_dword v0, v0, s[0:1] glc slc
 ; GFX10-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-CU-NEXT:    global_store_dword v1, v0, s[2:3]
 ; GFX10-CU-NEXT:    s_endpgm
@@ -295,7 +295,7 @@ define amdgpu_kernel void @global_nontemporal_load_1(
 ; GFX11-WGP-NEXT:    s_load_b128 s[0:3], s[0:1], 0x0
 ; GFX11-WGP-NEXT:    v_dual_mov_b32 v1, 0 :: v_dual_lshlrev_b32 v0, 2, v0
 ; GFX11-WGP-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX11-WGP-NEXT:    global_load_b32 v0, v0, s[0:1] slc dlc
+; GFX11-WGP-NEXT:    global_load_b32 v0, v0, s[0:1] glc slc dlc
 ; GFX11-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-WGP-NEXT:    global_store_b32 v1, v0, s[2:3]
 ; GFX11-WGP-NEXT:    s_nop 0
@@ -307,7 +307,7 @@ define amdgpu_kernel void @global_nontemporal_load_1(
 ; GFX11-CU-NEXT:    s_load_b128 s[0:3], s[0:1], 0x0
 ; GFX11-CU-NEXT:    v_dual_mov_b32 v1, 0 :: v_dual_lshlrev_b32 v0, 2, v0
 ; GFX11-CU-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX11-CU-NEXT:    global_load_b32 v0, v0, s[0:1] slc dlc
+; GFX11-CU-NEXT:    global_load_b32 v0, v0, s[0:1] glc slc dlc
 ; GFX11-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-CU-NEXT:    global_store_b32 v1, v0, s[2:3]
 ; GFX11-CU-NEXT:    s_nop 0
diff --git a/llvm/test/CodeGen/AMDGPU/memory-legalizer-private-nontemporal.ll b/llvm/test/CodeGen/AMDGPU/memory-legalizer-private-nontemporal.ll
index 30296a9c3d8963..516dda40ac6d12 100644
--- a/llvm/test/CodeGen/AMDGPU/memory-legalizer-private-nontemporal.ll
+++ b/llvm/test/CodeGen/AMDGPU/memory-legalizer-private-nontemporal.ll
@@ -60,7 +60,7 @@ define amdgpu_kernel void @private_nontemporal_load_0(
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v0, s2
-; GFX10-WGP-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen slc
+; GFX10-WGP-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen glc slc
 ; GFX10-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-WGP-NEXT:    global_store_dword v1, v0, s[0:1]
 ; GFX10-WGP-NEXT:    s_endpgm
@@ -77,7 +77,7 @@ define amdgpu_kernel void @private_nontemporal_load_0(
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v0, s2
-; GFX10-CU-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen slc
+; GFX10-CU-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen glc slc
 ; GFX10-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-CU-NEXT:    global_store_dword v1, v0, s[0:1]
 ; GFX10-CU-NEXT:    s_endpgm
@@ -161,7 +161,7 @@ define amdgpu_kernel void @private_nontemporal_load_0(
 ; GFX11-WGP-NEXT:    s_load_b64 s[0:1], s[0:1], 0x8
 ; GFX11-WGP-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX11-WGP-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX11-WGP-NEXT:    scratch_load_b32 v0, off, s2 slc dlc
+; GFX11-WGP-NEXT:    scratch_load_b32 v0, off, s2 glc slc dlc
 ; GFX11-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-WGP-NEXT:    global_store_b32 v1, v0, s[0:1]
 ; GFX11-WGP-NEXT:    s_nop 0
@@ -175,7 +175,7 @@ define amdgpu_kernel void @private_nontemporal_load_0(
 ; GFX11-CU-NEXT:    s_load_b64 s[0:1], s[0:1], 0x8
 ; GFX11-CU-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX11-CU-NEXT:    s_waitcnt lgkmcnt(0)
-; GFX11-CU-NEXT:    scratch_load_b32 v0, off, s2 slc dlc
+; GFX11-CU-NEXT:    scratch_load_b32 v0, off, s2 glc slc dlc
 ; GFX11-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-CU-NEXT:    global_store_b32 v1, v0, s[0:1]
 ; GFX11-CU-NEXT:    s_nop 0
@@ -265,7 +265,7 @@ define amdgpu_kernel void @private_nontemporal_load_1(
 ; GFX10-WGP-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-WGP-NEXT:    v_lshl_add_u32 v0, v0, 2, s2
-; GFX10-WGP-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen slc
+; GFX10-WGP-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen glc slc
 ; GFX10-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-WGP-NEXT:    global_store_dword v1, v0, s[0:1]
 ; GFX10-WGP-NEXT:    s_endpgm
@@ -282,7 +282,7 @@ define amdgpu_kernel void @private_nontemporal_load_1(
 ; GFX10-CU-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX10-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX10-CU-NEXT:    v_lshl_add_u32 v0, v0, 2, s2
-; GFX10-CU-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen slc
+; GFX10-CU-NEXT:    buffer_load_dword v0, v0, s[8:11], 0 offen glc slc
 ; GFX10-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX10-CU-NEXT:    global_store_dword v1, v0, s[0:1]
 ; GFX10-CU-NEXT:    s_endpgm
@@ -370,7 +370,7 @@ define amdgpu_kernel void @private_nontemporal_load_1(
 ; GFX11-WGP-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX11-WGP-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-WGP-NEXT:    v_lshl_add_u32 v0, v0, 2, s2
-; GFX11-WGP-NEXT:    scratch_load_b32 v0, v0, off slc dlc
+; GFX11-WGP-NEXT:    scratch_load_b32 v0, v0, off glc slc dlc
 ; GFX11-WGP-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-WGP-NEXT:    global_store_b32 v1, v0, s[0:1]
 ; GFX11-WGP-NEXT:    s_nop 0
@@ -385,7 +385,7 @@ define amdgpu_kernel void @private_nontemporal_load_1(
 ; GFX11-CU-NEXT:    v_mov_b32_e32 v1, 0
 ; GFX11-CU-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX11-CU-NEXT:    v_lshl_add_u32 v0, v0, 2, s2
-; GFX11-CU-NEXT:    scratch_load_b32 v0, v0, off slc dlc
+; GFX11-CU-NEXT:    scratch_load_b32 v0, v0, off glc slc dlc
 ; GFX11-CU-NEXT:    s_waitcnt vmcnt(0)
 ; GFX11-CU-NEXT:    global_store_b32 v1, v0, s[0:1]
 ; GFX11-CU-NEXT:    s_nop 0

@kzhuravl kzhuravl requested a review from Pierre-vh April 23, 2024 11:37
@arsenm arsenm requested a review from t-tye April 23, 2024 11:42
@jayfoad jayfoad requested a review from perlfu April 23, 2024 12:26
@jayfoad
Copy link
Contributor

jayfoad commented Apr 23, 2024

I have found some relevant history in @perlfu's D114351 which added the code to set glc for nontemporal stores but not loads. Relevant comment from @t-tye:

It appears that this should be setting GLC=1 for stores so that L0 will be HIT_EVICT instead of MISS_EVICT. This must not be done for loads as that would make Lo MISS_EVICT.

@ritter-x2a
Copy link
Member Author

Good find, @jayfoad, thanks!
It's not clear to me why the L0 policy must not be MISS_EVICT for nontemporal loads.

According to the RDNA 3 ISA reference, the policies without glc are L2: STREAM, L1: HIT_EVICT, L0: HIT_LRU.
For the policies with glc, L2: STREAM, L1: MISS_EVICT, L0: MISS_EVICT, I would expect the same semantic behavior, and a caching behavior that favors cases where the loaded value is not reused. This seems appropriate for nontemporal loads.
I'd be happy to learn what I am missing there.

@t-tye
Copy link
Collaborator

t-tye commented Apr 24, 2024

I do not remember the details, but I suspect the choice relates to desiring a cache policy of HIT_EVICT. The chosen settings achieve that for L1. L0 is very small, so in general it is unlikely leaving a value in it has any impact on performance as it will be evicted naturally.

@ssahasra
Copy link
Collaborator

From what I understand, HIT_EVICT means that an existing line is allowed to match, while MISS_EVICT means that no match is allowed. Why would HIT_EVICT be preferred for a non-temporal operation? Is it not natural to believe that non-temporal operations expect fresh data every time, because those values do not have temporal locality?

I am not sure if this perfectly matches streaming of data. But the RCCL low-latency protocols are indeed performance dependent on always getting latest values on L2. The consumer thread is polling this 128-bit location for fresh flag, and hitting an existing line in L1 just causes the polling to continue when in fact there are fresh flags in L2.

So depending on what "non-temporal" means to different use-cases, either HIT_EVICT on L1 needs to be MISS_EVICT in general for non-temporal data, or the specific "low-latency protocol" use case needs a new kind of builtin to set that policy.

@ssahasra
Copy link
Collaborator

Correction/Question: I suppose in all these comments, HIT_EVICT is actually HIT_LRU? I did not find any mention of HIT_EVICT in the MI300 Coherence guide. HIT_LRU makes sense, and it will certainly impact RCCL. HIT_EVICT, if it existed, would not have much of an impact on anything.

@jayfoad
Copy link
Contributor

jayfoad commented Apr 25, 2024

Correction/Question: I suppose in all these comments, HIT_EVICT is actually HIT_LRU? I did not find any mention of HIT_EVICT in the MI300 Coherence guide. HIT_LRU makes sense, and it will certainly impact RCCL. HIT_EVICT, if it existed, would not have much of an impact on anything.

I have not read the MI300 Coherence guide but that's based on GFX9 right? And this issue is about GFX10/11. GFX10 definitely does have HIT_EVICT as a read policy for L0 and GL1.

@jayfoad
Copy link
Contributor

jayfoad commented Apr 25, 2024

From what I understand, HIT_EVICT means that an existing line is allowed to match, while MISS_EVICT means that no match is allowed. Why would HIT_EVICT be preferred for a non-temporal operation?

My take on this is: nontemporal means that no match is expected, but if we do get a match then, hey, why not take advantage of it?

So depending on what "non-temporal" means to different use-cases, either HIT_EVICT on L1 needs to be MISS_EVICT in general for non-temporal data, or the specific "low-latency protocol" use case needs a new kind of builtin to set that policy.

Nontemporal is only a performance hint so it does not "need" to use MISS_EVICT. But I have been told already that RCCL is relying on nontemporal to do things that it is not strictly required to do.

@ritter-x2a
Copy link
Member Author

Adding the GLC bit to nontemporal loads would cause them to avoid cache hits when the requested data is already in the cache.
This would reduce the performance of nontemporal loads unnecessarily; their point is only to signal that the data is not accessed soon after the load. We should therefore not merge this.

@ritter-x2a ritter-x2a closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jayfoad jayfoad Awaiting requested review from jayfoad

@b-sumner b-sumner Awaiting requested review from b-sumner

@Pierre-vh Pierre-vh Awaiting requested review from Pierre-vh

@t-tye t-tye Awaiting requested review from t-tye

@perlfu perlfu Awaiting requested review from perlfu

Assignees

@ritter-x2a ritter-x2a

Labels
backend:AMDGPU
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ritter-x2a @llvmbot @jayfoad @t-tye @ssahasra