[AMDGPU] use scalar shift for SALU users in frame index elimination

[alex-t]

Summary

In the frame index lowering we have to insert shift and add
instructions to adjust stack object access. We need to take care of the stack
object user kind and use scalar shift/add for scalar users.

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2242–2243

Can probably be another mov?

2242–2243

Formatting.

2244

Formatting.

2256

What if $scc is alive?
$scc shall also be marked dead-def.

llvm/test/CodeGen/AMDGPU/frame-index.mir

2

No need to pass -mattr=-promote-alloca -amdgpu-sroa=0.

8

IR is also not needed.

60

Just remove memory operand (part starting from ::) so you do not need IR.

89

Generate the checks. This one misses inlicit-def dead $scc.

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2242–2243

It would be a bit more appropriate to check the class of the use operand

[alex-t]

SCC clobbering case added. MIR test simplified.

[alex-t]

IsSALU check chamged to query operand register class.
MIR test were made auto-generated

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2242–2243

Never :)

[alex-t]

formatting

[rampitec]

Need a testcase with alive scc.

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2242–2243

Why cannot it be v_mov_b32_e64?

2243

Is not isRegUsed for the whole function and will always return true on practice?

2262

Weird formatting.

llvm/test/CodeGen/AMDGPU/frame-index.mir

2

Not done.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

This function is used to determine if SCC is live in all other places around the code.
Another option is to scan upwards until the block start is reached.

2262

This is clang-format. Otherwise lint complains

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

All other places shall mean the whole function? I'd suggest using MBB->computeRegisterLiveness().

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

I mean checks like this:

CanClobberSCC = !RS->isRegUsed(AMDGPU::SCC);

they really interested if SCC is live at exact point.

In case this is wrong we have lots of problems

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

But what point is it? The call does not take an iterator.

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

It's at the scavenging point, which is separately advanced

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

Was it advanced before the call?

[arsenm]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

It should have been advanced by the scavengeRegister call (assuming there is one on the path that reaches here). Plus PrologEpilogInserter advances before the call in the normal usage

[rampitec]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

OK, thanks.

[alex-t]

test case for SCC multiple defs added. Unnecessary compiler options removed from the test command line.

[alex-t]

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

2243

RegisterScavenger::forward takes care of that. It is called from PEI::eliminateFrameIndices on each iteration. So it updates live information stepping each instruction.
The very first call of the SIRegisterInfo::eliminateFrameIndex happens with the LiveRegs updated on the BB entrance with LiveIns.
Each next call happens after the RS->forward updated live info.
I have a test for this.

[rampitec]

In D121524#3394122, @alex-t wrote:

test case for SCC multiple defs added. Unnecessary compiler options removed from the test command line.

There is a case where you are creating readfirstlane, but I do not see a test for it.

[alex-t]

test updated

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

default: {
 
         return;
       }
 
       bool IsMUBUF = TII->isMUBUF(*MI);
 
       if (!IsMUBUF && !MFI->isEntryFunction()) {
         // Convert to a swizzled stack address by scaling by the wave size.
-        //
         // In an entry function/kernel the offset is already swizzled.
-
-        bool IsCopy = MI->getOpcode() == AMDGPU::V_MOV_B32_e32;
-        Register ResultReg =
-            IsCopy ? MI->getOperand(0).getReg()
-                   : RS->scavengeRegister(&AMDGPU::VGPR_32RegClass, MI, 0);
+        bool IsSALU = isSGPRClass(TII->getOpRegClass(*MI, FIOperandNum));
+        bool LiveSCC = RS->isRegUsed(AMDGPU::SCC);
+        const TargetRegisterClass *RC = IsSALU && !LiveSCC
+                                            ? &AMDGPU::SReg_32RegClass
+                                            : &AMDGPU::VGPR_32RegClass;
+        bool IsCopy = MI->getOpcode() == AMDGPU::V_MOV_B32_e32 ||
+                      MI->getOpcode() == AMDGPU::V_MOV_B32_e64;
+        Register ResultReg = IsCopy ? MI->getOperand(0).getReg()
+                                    : RS->scavengeRegister(RC, MI, 0);
 
         int64_t Offset = FrameInfo.getObjectOffset(Index);
         if (Offset == 0) {
+          unsigned OpCode = IsSALU && !LiveSCC ? AMDGPU::S_LSHR_B32
+                                               : AMDGPU::V_LSHRREV_B32_e64;
           // XXX - This never happens because of emergency scavenging slot at 0?
-          BuildMI(*MBB, MI, DL, TII->get(AMDGPU::V_LSHRREV_B32_e64), ResultReg)
+          auto Shift = BuildMI(*MBB, MI, DL, TII->get(OpCode), ResultReg)
                            .addImm(ST.getWavefrontSizeLog2())
                            .addReg(FrameReg);
+          if (IsSALU && !LiveSCC)
+            Shift.getInstr()->getOperand(3).setIsDead(
+                true); // Mark SCC as dead.
+          if (IsSALU && LiveSCC) {
+            Register NewDest =
+                RS->scavengeRegister(&AMDGPU::SReg_32RegClass, Shift, 0);
+            BuildMI(*MBB, MI, DL, TII->get(AMDGPU::V_READFIRSTLANE_B32),
+                    NewDest)
+                .addReg(ResultReg);
+            ResultReg = NewDest;
+          }
         } else {
-          if (auto MIB = TII->getAddNoCarry(*MBB, MI, DL, ResultReg, *RS)) {
+          MachineInstrBuilder MIB;
+          if (!IsSALU) {
+            if (MIB = TII->getAddNoCarry(*MBB, MI, DL, ResultReg, *RS)) {
               // Reuse ResultReg in intermediate step.
               Register ScaledReg = ResultReg;
 
               BuildMI(*MBB, *MIB, DL, TII->get(AMDGPU::V_LSHRREV_B32_e64),
                       ScaledReg)
                 .addImm(ST.getWavefrontSizeLog2())
                 .addReg(FrameReg);
 
               const bool IsVOP2 = MIB->getOpcode() == AMDGPU::V_ADD_U32_e32;
 
               // TODO: Fold if use instruction is another add of a constant.
               if (IsVOP2 || AMDGPU::isInlinableLiteral32(Offset, ST.hasInv2PiInlineImm())) {
                 // FIXME: This can fail
                 MIB.addImm(Offset);
                 MIB.addReg(ScaledReg, RegState::Kill);
                 if (!IsVOP2)
                   MIB.addImm(0); // clamp bit
               } else {
                 assert(MIB->getOpcode() == AMDGPU::V_ADD_CO_U32_e64 &&
                        "Need to reuse carry out register");
 
                 // Use scavenged unused carry out as offset register.
                 Register ConstOffsetReg;
                 if (!isWave32)
                   ConstOffsetReg = getSubReg(MIB.getReg(1), AMDGPU::sub0);
                 else
                   ConstOffsetReg = MIB.getReg(1);
 
                 BuildMI(*MBB, *MIB, DL, TII->get(AMDGPU::S_MOV_B32), ConstOffsetReg)
                     .addImm(Offset);
                 MIB.addReg(ConstOffsetReg, RegState::Kill);
                 MIB.addReg(ScaledReg, RegState::Kill);
                 MIB.addImm(0); // clamp bit
               }
-          } else {
+            }
+          }
+          if (!MIB || IsSALU) {
             // We have to produce a carry out, and there isn't a free SGPR pair
             // for it. We can keep the whole computation on the SALU to avoid
             // clobbering an additional register at the cost of an extra mov.
 
             // We may have 1 free scratch SGPR even though a carry out is
             // unavailable. Only one additional mov is needed.
             Register TmpScaledReg =
-                RS->scavengeRegister(&AMDGPU::SReg_32_XM0RegClass, MI, 0, false);
             Register ScaledReg = TmpScaledReg.isValid() ? TmpScaledReg : FrameReg;
 
             BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_LSHR_B32), ScaledReg)
               .addReg(FrameReg)
               .addImm(ST.getWavefrontSizeLog2());
             BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), ScaledReg)
                 .addReg(ScaledReg, RegState::Kill)
                 .addImm(Offset);
+            if (!IsSALU)
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::COPY), ResultReg)
                   .addReg(ScaledReg, RegState::Kill);
+            else
+              ResultReg = ScaledReg;
 
             // If there were truly no free SGPRs, we need to undo everything.
             if (!TmpScaledReg.isValid()) {
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_ADD_I32), ScaledReg)
-                  .addReg(ScaledReg, RegState::Kill)
-                  .addImm(-Offset);
               BuildMI(*MBB, MI, DL, TII->get(AMDGPU::S_LSHL_B32), ScaledReg)
                 .addReg(FrameReg)
                 .addImm(ST.getWavefrontSizeLog2());
             }
           }
         }
 
         // Don't introduce an extra copy if we're just materializing in a mov.

llvm/test/CodeGen/AMDGPU/frame-index.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=kaveri -verify-machineinstrs -run-pass=prologepilog -o - %s | FileCheck -check-prefix=GCN %s
+
+---
+name: func_add_constant_to_fi_divergent_i32
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 0, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+
+body:             |
+  bb.0:
+    liveins: $vgpr31, $sgpr30_sgpr31
+
+    ; GCN-LABEL: name: func_add_constant_to_fi_divergent_i32
+    ; GCN: liveins: $vgpr31, $sgpr30_sgpr31
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: renamable $vgpr0 = V_AND_B32_e32 1023, killed $vgpr31, implicit $exec
+    ; GCN-NEXT: renamable $vgpr0 = V_LSHLREV_B32_e32 2, killed $vgpr0, implicit $exec
+    ; GCN-NEXT: $vgpr1 = V_LSHRREV_B32_e64 6, $sgpr32, implicit $exec
+    ; GCN-NEXT: renamable $vgpr0 = V_ADD_CO_U32_e32 killed $vgpr1, killed $vgpr0, implicit-def dead $vcc, implicit $exec
+    ; GCN-NEXT: renamable $vgpr0, dead renamable $vcc = nuw V_ADD_CO_U32_e64 4, killed $vgpr0, 0, implicit $exec
+    ; GCN-NEXT: $m0 = S_MOV_B32 -1
+    ; GCN-NEXT: DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    ; GCN-NEXT: S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+    renamable $vgpr0 = V_AND_B32_e32 1023, killed $vgpr31, implicit $exec
+    renamable $vgpr0 = V_LSHLREV_B32_e32 2, killed $vgpr0, implicit $exec
+    renamable $vgpr0 = V_ADD_CO_U32_e32 %stack.0, killed $vgpr0, implicit-def dead $vcc, implicit $exec
+    renamable $vgpr0, dead renamable $vcc = nuw V_ADD_CO_U32_e64 4, killed $vgpr0, 0, implicit $exec
+    $m0 = S_MOV_B32 -1
+    DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+
+...
+---
+name: func_add_constant_to_fi_uniform_i32
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 0, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    liveins: $sgpr30_sgpr31
+
+    ; GCN-LABEL: name: func_add_constant_to_fi_uniform_i32
+    ; GCN: liveins: $sgpr30_sgpr31
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: $vcc_hi = S_LSHR_B32 6, $sgpr32, implicit-def dead $scc
+    ; GCN-NEXT: renamable $sgpr4 = nuw S_ADD_I32 killed $vcc_hi, 4, implicit-def dead $scc
+    ; GCN-NEXT: renamable $vgpr0 = COPY killed renamable $sgpr4, implicit $exec
+    ; GCN-NEXT: $m0 = S_MOV_B32 -1
+    ; GCN-NEXT: DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    ; GCN-NEXT: S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+    renamable $sgpr4 = nuw S_ADD_I32 %stack.0, 4, implicit-def dead $scc
+    renamable $vgpr0 = COPY killed renamable $sgpr4, implicit $exec
+    $m0 = S_MOV_B32 -1
+    DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+
+...
+---
+name: func_add_constant_to_fi_uniform_SCC_clobber_i32
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 0, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+  - { id: 1, type: default, offset: 0, size: 32, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 512, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+
+machineFunctionInfo:
+
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    liveins: $sgpr30_sgpr31
+
+    ; GCN-LABEL: name: func_add_constant_to_fi_uniform_SCC_clobber_i32
+    ; GCN: liveins: $sgpr30_sgpr31
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: $vcc_hi = S_LSHR_B32 6, $sgpr32, implicit-def dead $scc
+    ; GCN-NEXT: renamable $sgpr4 = nuw S_ADD_U32 killed $vcc_hi, 4, implicit-def $scc
+    ; GCN-NEXT: renamable $sgpr5 = S_ADDC_U32 $sgpr4, 1234567, implicit-def $scc, implicit $scc
+    ; GCN-NEXT: $vcc_hi = S_LSHR_B32 $sgpr32, 6, implicit-def $scc
+    ; GCN-NEXT: $vcc_hi = S_ADD_I32 killed $vcc_hi, 8, implicit-def $scc
+    ; GCN-NEXT: renamable $sgpr6 = S_MUL_I32 killed $vcc_hi, $sgpr5
+    ; GCN-NEXT: renamable $vgpr0 = COPY killed renamable $sgpr6, implicit $exec
+    ; GCN-NEXT: $m0 = S_MOV_B32 -1
+    ; GCN-NEXT: DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    ; GCN-NEXT: S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+    renamable $sgpr4 = nuw S_ADD_U32 %stack.0, 4, implicit-def $scc
+    renamable $sgpr5 = S_ADDC_U32 $sgpr4, 1234567, implicit-def $scc, implicit $scc
+    renamable $sgpr6 = S_MUL_I32 %stack.1, $sgpr5
+    renamable $vgpr0 = COPY killed renamable $sgpr6, implicit $exec
+    $m0 = S_MOV_B32 -1
+    DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+
+...
+---
+name: func_other_fi_user_non_inline_imm_offset_i32
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: default, offset: 0, size: 512, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 0, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+  - { id: 1, type: default, offset: 0, size: 32, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 512, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    liveins: $sgpr30_sgpr31
+
+    ; GCN-LABEL: name: func_other_fi_user_non_inline_imm_offset_i32
+    ; GCN: liveins: $sgpr30_sgpr31
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: renamable $vgpr0 = V_MOV_B32_e32 7, implicit $exec
+    ; GCN-NEXT: BUFFER_STORE_DWORD_OFFSET killed renamable $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr32, 260, 0, 0, 0, implicit $exec
+    ; GCN-NEXT: $vcc_hi = S_LSHR_B32 $sgpr32, 6, implicit-def $scc
+    ; GCN-NEXT: $vcc_hi = S_ADD_I32 killed $vcc_hi, 512, implicit-def $scc
+    ; GCN-NEXT: renamable $sgpr4 = S_MUL_I32 killed $vcc_hi, 9
+    ; GCN-NEXT: renamable $vgpr0 = COPY killed renamable $sgpr4, implicit $exec
+    ; GCN-NEXT: $m0 = S_MOV_B32 -1
+    ; GCN-NEXT: DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    ; GCN-NEXT: S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+    renamable $vgpr0 = V_MOV_B32_e32 7, implicit $exec
+    BUFFER_STORE_DWORD_OFFEN killed renamable $vgpr0, %stack.0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 260, 0, 0, 0, implicit $exec
+    renamable $sgpr4 = S_MUL_I32 %stack.1, 9
+    renamable $vgpr0 = COPY killed renamable $sgpr4, implicit $exec
+    $m0 = S_MOV_B32 -1
+    DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+
+...
+---
+name: func_add_constant_to_fi_uniform_live_SCC_i32
+tracksRegLiveness: true
+stack:
+  - { id: 0, type: default, offset: 0, size: 8, alignment: 4,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: 0, debug-info-variable: '', debug-info-expression: '',
+      debug-info-location: '' }
+
+machineFunctionInfo:
+
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    liveins: $sgpr30_sgpr31, $sgpr10
+    ; GCN-LABEL: name: func_add_constant_to_fi_uniform_live_SCC_i32
+    ; GCN: liveins: $sgpr30_sgpr31, $sgpr10
+    ; GCN-NEXT: {{  $}}
+    ; GCN-NEXT: renamable $sgpr4 = nuw S_ADD_U32 $sgpr10, 4, implicit-def $scc
+    ; GCN-NEXT: $vgpr1 = V_LSHRREV_B32_e64 6, $sgpr32, implicit $exec
+    ; GCN-NEXT: $vcc_hi = V_READFIRSTLANE_B32 $vgpr1, implicit $exec
+    ; GCN-NEXT: renamable $sgpr5 = S_MUL_I32 killed $vcc_hi, $sgpr4
+    ; GCN-NEXT: renamable $sgpr6 = S_ADDC_U32 $sgpr5, 1234567, implicit-def dead $scc, implicit $scc
+    ; GCN-NEXT: renamable $vgpr0 = COPY killed renamable $sgpr6, implicit $exec
+    ; GCN-NEXT: $m0 = S_MOV_B32 -1
+    ; GCN-NEXT: DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    ; GCN-NEXT: S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+    renamable $sgpr4 = nuw S_ADD_U32 $sgpr10, 4, implicit-def $scc
+    renamable $sgpr5 = S_MUL_I32 %stack.0, $sgpr4
+    renamable $sgpr6 = S_ADDC_U32 $sgpr5, 1234567, implicit-def dead $scc, implicit $scc
+    renamable $vgpr0 = COPY killed renamable $sgpr6, implicit $exec
+    $m0 = S_MOV_B32 -1
+    DS_WRITE_B32 undef renamable $vgpr0, killed renamable $vgpr0, 0, 0, implicit $m0, implicit $exec
+    S_SETPC_B64_return killed renamable $sgpr30_sgpr31
+
+...