AMDGPU: Fold inline offset for loads properly in moveToVALU on GFX9

[mareko]

Summary

This enables load merging into x2, x4, which is driven by inline offsets.

6500 shaders are affected:
Code Size in affected shaders: -15.14 %

[nhaehnle]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

Why not V_ADD_I32_e64 and V_ADD_U32_e32 as well? Those should work the same.

You can also remove the corresponding FIXME in the comment.

Also, to support e64/VOP3, the code must check for clamp and imod (abs/neg) bits and bail out when they're set. Clamp is genuinely supported for integer ops since gfx9, and while imods will not do anything useful, they *will* affect the instruction (they just affect the MSB as if the instruction were a floating point operation).

[mareko]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

I think that only V_ADD_I32_e64 (for gfx9) and V_ADD_U32_e32 (for others) can occur here, because integer addition is always translated to S_ADD and lowered in moveToVALU. If that is true, mods can't occur here.

I'll remove the FIXME.

[nhaehnle]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

You may be right about the other opcodes.

About the mods, I still think it's better to be defensive. Just because mods can't occur here *today* doesn't mean they won't start showing up at some point in the future if saturating integer arithmetic is exposed in the frontend.

[mareko]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

I can't check the mods, because the opcodes don't have any mods defined (yet). So there is nothing to do.

[arsenm]

lib/Target/AMDGPU/SIInstrInfo.cpp

3769 ↗

(On Diff #129876)

It's a defect that at this point the e32 version is ever encountered. We shouldn't be emitting VCC uses up front. Where is this coming from? I thought I fixed most of these places already

[mareko]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

moveToVALU selects it from S_ADD.

3769 ↗

(On Diff #129876)

moveToVALU selects it from S_ADD.

[nhaehnle]

lib/Target/AMDGPU/SIInstrInfo.cpp

3768–3770 ↗

(On Diff #129876)

Huh, that's odd, but you're right. Looks like integer clamping is only half-implemented after all, and not even the (dis)assembler really supports it.

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.cpp

case AMDGPU::S_BUFFER_LOAD_DWORD_SGPR: {
       unsigned VDst = MRI.createVirtualRegister(&AMDGPU::VGPR_32RegClass);
       const MachineOperand *VAddr = getNamedOperand(Inst, AMDGPU::OpName::soff);
       auto Add = MRI.getUniqueVRegDef(VAddr->getReg());
       unsigned Offset = 0;
 
       // FIXME: This isn't safe because the addressing mode doesn't work
       // correctly if vaddr is negative.
       //
-      // FIXME: Handle v_add_u32 and VOP3 form. Also don't rely on immediate
-      // being in src0.
-      //
       // FIXME: Should probably be done somewhere else, maybe SIFoldOperands.
       //
       // See if we can extract an immediate offset by recognizing one of these:
       //   V_ADD_I32_e32 dst, imm, src1
       //   V_ADD_I32_e32 dst, (S_MOV_B32 imm), src1
       // V_ADD will be removed by "Remove dead machine instructions".
-      if (Add && Add->getOpcode() == AMDGPU::V_ADD_I32_e32) {
+      if (Add &&
+          (Add->getOpcode() == AMDGPU::V_ADD_I32_e32 ||
+           Add->getOpcode() == AMDGPU::V_ADD_U32_e64)) {
+        static const unsigned SrcNames[2] = {
+          AMDGPU::OpName::src0,
+          AMDGPU::OpName::src1,
+        };
+
+        // Find a literal offset in one of source operands.
+        for (int i = 0; i < 2; i++) {
           const MachineOperand *Src =
-          getNamedOperand(*Add, AMDGPU::OpName::src0);
+            getNamedOperand(*Add, SrcNames[i]);
 
           if (Src->isReg()) {
             auto Mov = MRI.getUniqueVRegDef(Src->getReg());
             if (Mov && Mov->getOpcode() == AMDGPU::S_MOV_B32)
               Src = &Mov->getOperand(1);
           }
 
           if (Src) {
             if (Src->isImm())
               Offset = Src->getImm();
             else if (Src->isCImm())
               Offset = Src->getCImm()->getZExtValue();
           }
 
-        if (Offset && isLegalMUBUFImmOffset(Offset))
-          VAddr = getNamedOperand(*Add, AMDGPU::OpName::src1);
-        else
+          if (Offset && isLegalMUBUFImmOffset(Offset)) {
+            VAddr = getNamedOperand(*Add, SrcNames[!i]);
+            break;
+          }
+
           Offset = 0;
         }
+      }
 
       BuildMI(*MBB, Inst, Inst.getDebugLoc(),
               get(AMDGPU::BUFFER_LOAD_DWORD_OFFEN), VDst)
         .add(*VAddr) // vaddr
         .add(*getNamedOperand(Inst, AMDGPU::OpName::sbase)) // srsrc
         .addImm(0) // soffset
         .addImm(Offset) // offset
         .addImm(getNamedOperand(Inst, AMDGPU::OpName::glc)->getImm())

llvm/trunk/test/CodeGen/AMDGPU/smrd.ll

 define amdgpu_ps float @smrd_vgpr_offset(<4 x i32> inreg %desc, i32 %offset) #0 {
 main_body:
   %r = call float @llvm.SI.load.const.v4i32(<4 x i32> %desc, i32 %offset)
   ret float %r
 }
 
 ; GCN-LABEL: {{^}}smrd_vgpr_offset_imm:
 ; GCN-NEXT: %bb.
-
-; SICIVI-NEXT: buffer_load_dword v{{[0-9]}}, v0, s[0:3], 0 offen offset:4095 ;
-
-; GFX9-NEXT: v_add_u32_e32 [[ADD:v[0-9]+]], 0xfff, v0
-; GFX9-NEXT: buffer_load_dword v{{[0-9]}}, [[ADD]], s[0:3], 0 offen ;
+; GCN-NEXT: buffer_load_dword v{{[0-9]}}, v0, s[0:3], 0 offen offset:4095 ;
 define amdgpu_ps float @smrd_vgpr_offset_imm(<4 x i32> inreg %desc, i32 %offset) #0 {
 main_body:
   %off = add i32 %offset, 4095
   %r = call float @llvm.SI.load.const.v4i32(<4 x i32> %desc, i32 %off)
   ret float %r
 }
 
 ; GCN-LABEL: {{^}}smrd_vgpr_offset_imm_too_large:
main_body:
   %r6 = call float @llvm.SI.load.const.v4i32(<4 x i32> %desc, i32 32)
   call void @llvm.amdgcn.exp.f32(i32 0, i32 15, float %r1, float %r2, float %r3, float %r4, i1 true, i1 true) #0
   call void @llvm.amdgcn.exp.f32(i32 0, i32 15, float %r5, float %r6, float undef, float undef, i1 true, i1 true) #0
   ret void
 }
 
 ; GCN-LABEL: {{^}}smrd_vgpr_merged:
 ; GCN-NEXT: %bb.
-
-; SICIVI-NEXT: buffer_load_dwordx4 v[{{[0-9]}}:{{[0-9]}}], v0, s[0:3], 0 offen offset:4
-; SICIVI-NEXT: buffer_load_dwordx2 v[{{[0-9]}}:{{[0-9]}}], v0, s[0:3], 0 offen offset:28
-
-; GFX9: buffer_load_dword
-; GFX9: buffer_load_dword
-; GFX9: buffer_load_dword
-; GFX9: buffer_load_dword
-; GFX9: buffer_load_dword
-; GFX9: buffer_load_dword
+; GCN-NEXT: buffer_load_dwordx4 v[{{[0-9]}}:{{[0-9]}}], v0, s[0:3], 0 offen offset:4
+; GCN-NEXT: buffer_load_dwordx2 v[{{[0-9]}}:{{[0-9]}}], v0, s[0:3], 0 offen offset:28
 define amdgpu_ps void @smrd_vgpr_merged(<4 x i32> inreg %desc, i32 %a) #0 {
 main_body:
   %a1 = add i32 %a, 4
   %a2 = add i32 %a, 8
   %a3 = add i32 %a, 12
   %a4 = add i32 %a, 16
   %a5 = add i32 %a, 28
   %a6 = add i32 %a, 32