AMDGPU: Skip waiting on lgkmcnt for global flat loads

[arsenm]

Summary

If we know the access isn't to a flat address,
the wait for LDS is not necessary.

[• tstellarAMD]

lib/Target/AMDGPU/SIInsertWaits.cpp

221–225

I'm not really sure exactly what this is doing, but as long as this accounts for the fact that the hw LGKM counter is always incremented even if the operation accesses global memory than this is fine.

Though, I think you should add some tests that have lds operations before and after a flat instruction that accesses global memory.

[arsenm]

lib/Target/AMDGPU/SIInsertWaits.cpp

221–225

I don't think this is accounting for the hardware increase

[arsenm]

This pass was replaced

[t-tye]

lib/Target/AMDGPU/SIInsertWaits.cpp

221–225

But does the hardware increasing the LGKM counter matter? The hardware will increase it, then decrease it once it determines the FLT address is targeting LDS. So all that that can effect is another memory operation waiting for LGKM, causing them to wait a bit longer. It cannot make any other memory operation satisfy their WAITCNT early so cannot break correctness.

The completion of a FLAT operation that is known to only target VMEM only needs to wait on the vmem counter.

lib/Target/AMDGPU/SIInsertWaits.cpp

 static bool readsVCCZ(unsigned Opcode) {
   return Opcode == AMDGPU::S_CBRANCH_VCCNZ || Opcode == AMDGPU::S_CBRANCH_VCCZ;
 }
 
 bool SIInsertWaits::hasOutstandingLGKM() const {
   return WaitedOn.Named.LGKM != LastIssued.Named.LGKM;
 }
 
+static bool hasGlobalMemOperand(const MachineInstr &MI) {
+  if (!MI.hasOneMemOperand())
+    return false;
+
+  MachineMemOperand *MMO = *MI.memoperands_begin();
+  unsigned AS = MMO->getAddrSpace();
+  return AS == AMDGPUAS::GLOBAL_ADDRESS ||
+         AS == AMDGPUAS::CONSTANT_ADDRESS;
+}
+
 Counters SIInsertWaits::getHwCounts(MachineInstr &MI) {
   uint64_t TSFlags = MI.getDesc().TSFlags;
   Counters Result = { { 0, 0, 0 } };
 
   Result.Named.VM = !!(TSFlags & SIInstrFlags::VM_CNT);
 
   // Only consider stores or EXP for EXP_CNT
   Result.Named.EXP = !!(TSFlags & SIInstrFlags::EXP_CNT &&
if (TII->isSMRD(MI)) {
         unsigned Size = RC->getSize();
         Result.Named.LGKM = Size > 4 ? 2 : 1;
       } else {
         // s_dcache_inv etc. do not have a a destination register. Assume we
         // want a wait on these.
         // XXX - What is the right value?
         Result.Named.LGKM = 1;
       }
+    } else if (TII->isFLAT(MI)) {
+      // If we know the pointer is not accessing a flat address, we don't need
+      // to wait for lgkm.
+      if (!hasGlobalMemOperand(MI))
+        Result.Named.LGKM = 1;
     } else {
       // DS
       Result.Named.LGKM = 1;
     }
 
   } else {
     Result.Named.LGKM = 0;
   }

test/CodeGen/AMDGPU/waitcnt-flat.ll

 ; RUN: llc < %s -mtriple=amdgcn--amdhsa -mcpu=kaveri | FileCheck --check-prefix=GCN %s
 ; RUN: llc < %s -mtriple=amdgcn--amdhsa -mcpu=fiji | FileCheck --check-prefix=GCN %s
 
 ; If flat_store_dword and flat_load_dword use different registers for the data
 ; operand, this test is not broken.  It just means it is no longer testing
 ; for the original bug.
 
-; GCN: {{^}}test:
+; GCN-LABEL: {{^}}global_test:
+; GCN: flat_store_dword v[{{[0-9]+:[0-9]+}}],
+; GCN: s_waitcnt vmcnt(0){{$}}
+; GCN: flat_load_dword
+
+; Test pointer problem
 ; XGCN: flat_store_dword v[{{[0-9]+:[0-9]+}}], [[DATA:v[0-9]+]]
 ; XGCN: s_waitcnt vmcnt(0) lgkmcnt(0)
 ; XGCN: flat_load_dword [[DATA]], v[{{[0-9]+:[0-9]+}}]
-define void @test(i32 addrspace(1)* %out, i32 %in) {
+define void @global_test(i32 addrspace(1)* %out, i32 %in) {
   store volatile i32 0, i32 addrspace(1)* %out
   %val = load volatile i32, i32 addrspace(1)* %out
   ret void
 }
+
+; GCN-LABEL: {{^}}flat_test:
+; GCN: flat_store_dword v[{{[0-9]+:[0-9]+}}],
+; GCN: s_waitcnt vmcnt(0) lgkmcnt(0){{$}}
+; GCN: flat_load_dword
+define void @flat_test(i32 addrspace(4)* %out, i32 %in) {
+  store volatile i32 0, i32 addrspace(4)* %out
+  %val = load volatile i32, i32 addrspace(4)* %out
+  ret void
+}
+
+; If the store is not through a generic pointer, the lgkmcnt is not
+; needed.
+
+; GCN-LABEL: {{^}}global_flat_test:
+; GCN: flat_store_dword v[{{[0-9]+:[0-9]+}}],
+; GCN: s_waitcnt vmcnt(0){{$}}
+; GCN: flat_load_dword
+define void @global_flat_test(i32 addrspace(1)* %out, i32 %in) {
+  store volatile i32 0, i32 addrspace(1)* %out
+  %out.cast = addrspacecast i32 addrspace(1)* %out to i32 addrspace(4)*
+  %val = load volatile i32, i32 addrspace(4)* %out.cast
+  ret void
+}
+
+; GCN-LABEL: {{^}}flat_global_test:
+; GCN: flat_store_dword v[{{[0-9]+:[0-9]+}}],
+; GCN: s_waitcnt vmcnt(0) lgkmcnt(0){{$}}
+; GCN: flat_load_dword
+define void @flat_global_test(i32 addrspace(1)* %out, i32 %in) {
+  %out.cast = addrspacecast i32 addrspace(1)* %out to i32 addrspace(4)*
+  store volatile i32 0, i32 addrspace(4)* %out.cast
+  %val = load volatile i32, i32 addrspace(1)* %out
+  ret void
+}