[AMDGPU] Cluster MIMG instructions

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[nhaehnle]

This absolutely needs a test with sampled images.

[nhaehnle]

More generally, I'm worried about whether the generic infrastructure will do something useful with the extra address arguments, so this needs quite a bit of test coverage about what happens with different texture coordinate patterns.

[foad]

Add an image_sample test.

[foad]

This absolutely needs a test with sampled images.

I've added one.

More generally, I'm worried about whether the generic infrastructure will do something useful with the extra address arguments, so this needs quite a bit of test coverage about what happens with different texture coordinate patterns.

The only code that knows about the address operands is SIInstrInfo::getMemOperandsWithOffset (which produces a list of address operands) and SIInstrInfo::shouldClusterMemOps (which consumes it). The logic is currently very simple: cluster if the first address operand (the image descriptor) is the same.

I'm sure the logic could be much cleverer, but I'd like to leave that for later patches with their own test cases. Until that happens, I'm not sure that it's worth producing tests for different texture coordinate patterns.

[arsenm]

llvm/test/CodeGen/AMDGPU/cluster_stores.ll

0–2

This could use a temp file to avoid compiling this twice. Redirect stderr to a temp file and run the dbg check on that?

92

Could use instruction checks that these are adjacent?

[foad]

Use a temporary file.
Check instructions are adjacent in the ISA.

[foad]

Ping!

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

2763

Braces

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (isMUBUF(LdSt) || isMTBUF(LdSt)) {
     // Get appropriate operand, and compute width accordingly.
     DataOpIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vdst);
     if (DataOpIdx == -1)
       DataOpIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vdata);
     Width = getOpSize(LdSt, DataOpIdx);
     return true;
   }
 
+  if (isMIMG(LdSt)) {
+    int SRsrcIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::srsrc);
+    BaseOps.push_back(&LdSt.getOperand(SRsrcIdx));
+    int VAddr0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr0);
+    if (VAddr0Idx >= 0) {
+      // GFX10 possible NSA encoding.
+      for (int I = VAddr0Idx; I < SRsrcIdx; ++I)
+        BaseOps.push_back(&LdSt.getOperand(I));
+    } else {
+      BaseOps.push_back(getNamedOperand(LdSt, AMDGPU::OpName::vaddr));
+    }
+    Offset = 0;
+    return true;
+  }
+
   if (isSMRD(LdSt)) {
     BaseOp = getNamedOperand(LdSt, AMDGPU::OpName::sbase);
     if (!BaseOp) // e.g. S_MEMTIME
       return false;
     BaseOps.push_back(BaseOp);
     OffsetOp = getNamedOperand(LdSt, AMDGPU::OpName::offset);
     Offset = OffsetOp ? OffsetOp->getImm() : 0;
     // Get appropriate operand, and compute width accordingly.
if (DataOpIdx == -1)
       DataOpIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vdata);
     Width = getOpSize(LdSt, DataOpIdx);
     return true;
   }
 
   return false;
 }
 
-static bool
-memOpsHaveSameBaseOperands(ArrayRef<const MachineOperand *> BaseOps1,
-                           ArrayRef<const MachineOperand *> BaseOps2) {
-  if (BaseOps1.size() != BaseOps2.size())
-    return false;
-  for (size_t I = 0, E = BaseOps1.size(); I < E; ++I)
-    if (!BaseOps1[I]->isIdenticalTo(*BaseOps2[I]))
-      return false;
-  return true;
-}
-
 static bool memOpsHaveSameBasePtr(const MachineInstr &MI1,
                                   ArrayRef<const MachineOperand *> BaseOps1,
                                   const MachineInstr &MI2,
                                   ArrayRef<const MachineOperand *> BaseOps2) {
-  if (memOpsHaveSameBaseOperands(BaseOps1, BaseOps2))
+  // Only examine the first "base" operand of each instruction, on the
+  // assumption that it represents the real base address of the memory access.
+  // Other operands are typically offsets or indices from this base address.
+  if (BaseOps1.front()->isIdenticalTo(*BaseOps2.front()))
     return true;
 
   if (!MI1.hasOneMemOperand() || !MI2.hasOneMemOperand())
     return false;
 
   auto MO1 = *MI1.memoperands_begin();
   auto MO2 = *MI2.memoperands_begin();
   if (MO1->getAddrSpace() != MO2->getAddrSpace())
bool SIInstrInfo::shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
   if (!memOpsHaveSameBasePtr(FirstLdSt, BaseOps1, SecondLdSt, BaseOps2))
     return false;
 
   const MachineOperand *FirstDst = nullptr;
   const MachineOperand *SecondDst = nullptr;
 
   if ((isMUBUF(FirstLdSt) && isMUBUF(SecondLdSt)) ||
       (isMTBUF(FirstLdSt) && isMTBUF(SecondLdSt)) ||
+      (isMIMG(FirstLdSt) && isMIMG(SecondLdSt)) ||
       (isFLAT(FirstLdSt) && isFLAT(SecondLdSt))) {
     const unsigned MaxGlobalLoadCluster = 7;
     if (NumLoads > MaxGlobalLoadCluster)
       return false;
 
     FirstDst = getNamedOperand(FirstLdSt, AMDGPU::OpName::vdata);
     if (!FirstDst)
       FirstDst = getNamedOperand(FirstLdSt, AMDGPU::OpName::vdst);
if (Src2->isReg() && Src2->getReg() == Reg) {
 
       return true;
     }
   }
 
   return false;
 }
 
+static bool
+memOpsHaveSameBaseOperands(ArrayRef<const MachineOperand *> BaseOps1,
+                           ArrayRef<const MachineOperand *> BaseOps2) {
+  if (BaseOps1.size() != BaseOps2.size())
+    return false;
+  for (size_t I = 0, E = BaseOps1.size(); I < E; ++I)
+    if (!BaseOps1[I]->isIdenticalTo(*BaseOps2[I]))
+      return false;
+  return true;
+}
+
 static bool offsetsDoNotOverlap(int WidthA, int OffsetA,
                                 int WidthB, int OffsetB) {
   int LowOffset = OffsetA < OffsetB ? OffsetA : OffsetB;
   int HighOffset = OffsetA < OffsetB ? OffsetB : OffsetA;
   int LowWidth = (LowOffset == OffsetA) ? WidthA : WidthB;
   return LowOffset + LowWidth <= HighOffset;
 }
 

llvm/test/CodeGen/AMDGPU/cluster_stores.ll

-; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -debug-only=machine-scheduler -o /dev/null %s 2>&1 | FileCheck --enable-var-scope --check-prefixes=CHECK,DBG %s
-; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck --enable-var-scope --check-prefixes=CHECK,GCN %s
+; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -debug-only=machine-scheduler < %s 2> %t | FileCheck --enable-var-scope --check-prefixes=CHECK,GCN %s
+; RUN: FileCheck --enable-var-scope --check-prefixes=CHECK,DBG %s < %t
 ; REQUIRES: asserts
 
 ; CHECK-LABEL: {{^}}cluster_load_cluster_store:
 define amdgpu_kernel void @cluster_load_cluster_store(i32* noalias %lb, i32* noalias %sb) {
 bb:
 ; DBG: Cluster ld/st SU(1) - SU(2)
 
 ; DBG: Cluster ld/st SU([[L1:[0-9]+]]) - SU([[L2:[0-9]+]])
; GCN-NEXT: flat_store_dword v[{{[0-9:]+}}], [[LD4]] offset:24
   store i32 %add, i32* %sa1
   %sa2 = getelementptr inbounds i32, i32* %sb, i32 4
   store i32 %ld2, i32* %sa2
   %sa3 = getelementptr inbounds i32, i32* %sb, i32 6
   store i32 %ld3, i32* %sa3
 
   ret void
 }
+
+; Cluster loads from the same texture with different coordinates
+; CHECK-LABEL: {{^}}cluster_image_load:
+; DBG: {{^}}Cluster ld/st [[SU1:SU\([0-9]+\)]] - [[SU2:SU\([0-9]+\)]]
+; DBG: {{^}}[[SU1]]: {{.*}} IMAGE_LOAD
+; DBG: {{^}}[[SU2]]: {{.*}} IMAGE_LOAD
+; GCN:      image_load v
+; GCN-NEXT: image_load v
+define amdgpu_ps void @cluster_image_load(<8 x i32> inreg %src, <8 x i32> inreg %dst, i32 %x, i32 %y) {
+entry:
+  %x1 = add i32 %x, 1
+  %y1 = add i32 %y, 1
+  %val1 = call <4 x float> @llvm.amdgcn.image.load.mip.2d.v4f32.i32(i32 15, i32 %x1, i32 %y1, i32 0, <8 x i32> %src, i32 0, i32 0)
+  %x2 = add i32 %x, 2
+  %y2 = add i32 %y, 2
+  %val2 = call <4 x float> @llvm.amdgcn.image.load.mip.2d.v4f32.i32(i32 15, i32 %x2, i32 %y2, i32 0, <8 x i32> %src, i32 0, i32 0)
+  %val = fadd fast <4 x float> %val1, %val2
+  call void @llvm.amdgcn.image.store.2d.v4f32.i32(<4 x float> %val, i32 15, i32 %x, i32 %y, <8 x i32> %dst, i32 0, i32 0)
+  ret void
+}
+
+; Don't cluster loads from different textures
+; CHECK-LABEL: {{^}}no_cluster_image_load:
+; DBG-NOT: {{^}}Cluster ld/st
+define amdgpu_ps void @no_cluster_image_load(<8 x i32> inreg %src1, <8 x i32> inreg %src2, <8 x i32> inreg %dst, i32 %x, i32 %y) {
+entry:
+  %val1 = call <4 x float> @llvm.amdgcn.image.load.mip.2d.v4f32.i32(i32 15, i32 %x, i32 %y, i32 0, <8 x i32> %src1, i32 0, i32 0)
+  %val2 = call <4 x float> @llvm.amdgcn.image.load.mip.2d.v4f32.i32(i32 15, i32 %x, i32 %y, i32 0, <8 x i32> %src2, i32 0, i32 0)
+  %val = fadd fast <4 x float> %val1, %val2
+  call void @llvm.amdgcn.image.store.2d.v4f32.i32(<4 x float> %val, i32 15, i32 %x, i32 %y, <8 x i32> %dst, i32 0, i32 0)
+  ret void
+}
+
+; Cluster loads from the same texture and sampler with different coordinates
+; CHECK-LABEL: {{^}}cluster_image_sample:
+; DBG: {{^}}Cluster ld/st [[SU1:SU\([0-9]+\)]] - [[SU2:SU\([0-9]+\)]]
+; DBG: {{^}}[[SU1]]: {{.*}} IMAGE_SAMPLE
+; DBG: {{^}}[[SU2]]: {{.*}} IMAGE_SAMPLE
+; GCN:      image_sample_d
+; GCN-NEXT: image_sample_d
+define amdgpu_ps void @cluster_image_sample(<8 x i32> inreg %src, <4 x i32> inreg %smp, <8 x i32> inreg %dst, i32 %x, i32 %y) {
+entry:
+  %s = sitofp i32 %x to float
+  %t = sitofp i32 %y to float
+  %s1 = fadd float %s, 1.0
+  %t1 = fadd float %t, 1.0
+  %val1 = call <4 x float> @llvm.amdgcn.image.sample.d.2d.v4f32.f32(i32 15, float %s1, float %t1, float 0.0, float 0.0, float 0.0, float 0.0, <8 x i32> %src, <4 x i32> %smp, i1 false, i32 0, i32 0)
+  %s2 = fadd float %s, 2.0
+  %t2 = fadd float %t, 2.0
+  %val2 = call <4 x float> @llvm.amdgcn.image.sample.d.2d.v4f32.f32(i32 15, float %s2, float %t2, float 1.0, float 1.0, float 1.0, float 1.0, <8 x i32> %src, <4 x i32> %smp, i1 false, i32 0, i32 0)
+  %val = fadd fast <4 x float> %val1, %val2
+  call void @llvm.amdgcn.image.store.2d.v4f32.i32(<4 x float> %val, i32 15, i32 %x, i32 %y, <8 x i32> %dst, i32 0, i32 0)
+  ret void
+}
+
+declare <4 x float> @llvm.amdgcn.image.load.mip.2d.v4f32.i32(i32 immarg, i32, i32, i32, <8 x i32>, i32 immarg, i32 immarg)
+declare <4 x float> @llvm.amdgcn.image.sample.d.2d.v4f32.f32(i32, float, float, float, float, float, float, <8 x i32>, <4 x i32>, i1, i32, i32)
+declare void @llvm.amdgcn.image.store.2d.v4f32.i32(<4 x float>, i32 immarg, i32, i32, <8 x i32>, i32 immarg, i32 immarg)