[AMDGPU] Eliminate SGPR to VGPR copy when possible

[rampitec]

Summary

SGPRs are generally cheaper, so try to use them over VGPRs.

[arsenm]

Typo in test file name

lib/Target/AMDGPU/SIFixSGPRCopies.cpp

182–187

This probably breaks if you have a copy that looks like a sub register extract

192

Why do you check the parent?

193

Why do you need to check this? Isn't this filtering the global isel pseudos which should never exist here?

194

Avoid doing the weird pointer arithmetic?

198

I forsee this breaking cases with multiple uses

test/CodeGen/AMDGPU/opt-sgpr-to-vgrp-copy.ll

25

Probably need a MIR test to stress multiple uses

[rampitec]

lib/Target/AMDGPU/SIFixSGPRCopies.cpp

182–187

The should not be a problem. I'm changing VGPR class to equivalent SGPR class, they support the same set of subregs.

192

A copy can be in a divergent control flow. If it is the same block we can be sure that is actually the value we are going to use.

193

isOperandLegal does not work with generic opcodes because they have no RC classes assigned to operands.

194

How do I get operand number otherwise?

198

Why? I'm checking all uses for operand legality. It also does not happen if there is a second def of the same register.

[rampitec]

Fixed typo in test name.

[rampitec]

test/CodeGen/AMDGPU/opt-sgpr-to-vgrp-copy.ll

25

Can you be more specific, multiple uses of what? Do you mean multiple uses of the same COPY?

[rampitec]

Replaced test with mir to check multiple use case.

[rampitec]

Removed pointer arithmetic on operands.

[rampitec]

Added subreg-extract like copy test.
Cleaned mir test.

lib/Target/AMDGPU/SIFixSGPRCopies.cpp

182–187

I have added test for that.

[rampitec]

lib/Target/AMDGPU/SIFixSGPRCopies.cpp

198

I have added test for multiple uses.

[rampitec]

Ping.

[arsenm]

LGTM. It's possible to reduce the mir testcase further though

test/CodeGen/AMDGPU/opt-sgpr-to-vgpr-copy.mir

102–117 ↗

(On Diff #102698)

You can strip out most of this stuff

[rampitec]

Reduced test.

lib/Target/AMDGPU/SIFixSGPRCopies.cpp

 }
 
 static bool isSGPRToVGPRCopy(const TargetRegisterClass *SrcRC,
                              const TargetRegisterClass *DstRC,
                              const SIRegisterInfo &TRI) {
   return TRI.isSGPRClass(SrcRC) && TRI.hasVGPRs(DstRC);
 }
 
+static bool tryChangeVGPRtoSGPRinCopy(MachineInstr &MI,
+                                      const SIRegisterInfo *TRI,
+                                      const SIInstrInfo *TII) {
+  MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+  auto &Src = MI.getOperand(1);
+  unsigned DstReg = MI.getOperand(0).getReg();
+  unsigned SrcReg = Src.getReg();
+  if (!TargetRegisterInfo::isVirtualRegister(SrcReg) ||
+      !TargetRegisterInfo::isVirtualRegister(DstReg))
+    return false;
+
+  for (const auto &MO : MRI.reg_nodbg_operands(DstReg)) {
+    const auto *UseMI = MO.getParent();
+    if (UseMI == &MI)
+      continue;
+    if (MO.isDef() || UseMI->getParent() != MI.getParent() ||
+        UseMI->getOpcode() <= TargetOpcode::GENERIC_OP_END ||
+        !TII->isOperandLegal(*UseMI, &MO - &UseMI->getOperand(0), &Src))
+      return false;
+  }
+  // Change VGPR to SGPR destination.
+  MRI.setRegClass(DstReg, TRI->getEquivalentSGPRClass(MRI.getRegClass(DstReg)));
+  return true;
+}
+
 // Distribute an SGPR->VGPR copy of a REG_SEQUENCE into a VGPR REG_SEQUENCE.
 //
 // SGPRx = ...
 // SGPRy = REG_SEQUENCE SGPRx, sub0 ...
 // VGPRz = COPY SGPRy
 //
 // ==>
 //
if (TargetRegisterInfo::isPhysicalRegister(CopyUse.getOperand(0).getReg()))
     return false;
 
   const TargetRegisterClass *SrcRC, *DstRC;
   std::tie(SrcRC, DstRC) = getCopyRegClasses(CopyUse, *TRI, MRI);
 
   if (!isSGPRToVGPRCopy(SrcRC, DstRC, *TRI))
     return false;
 
+  if (tryChangeVGPRtoSGPRinCopy(CopyUse, TRI, TII))
+    return true;
+
   // TODO: Could have multiple extracts?
   unsigned SubReg = CopyUse.getOperand(1).getSubReg();
   if (SubReg != AMDGPU::NoSubRegister)
     return false;
 
   MRI.setRegClass(DstReg, DstRC);
 
   // SGPRx = ...
for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end();
           // s_mov_b32.
           if (isSafeToFoldImmIntoCopy(&MI, DefMI, TII, SMovOp, Imm)) {
             MI.getOperand(1).ChangeToImmediate(Imm);
             MI.addImplicitDefUseOperands(MF);
             MI.setDesc(TII->get(SMovOp));
             break;
           }
           TII->moveToVALU(MI);
+        } else if (isSGPRToVGPRCopy(SrcRC, DstRC, *TRI)) {
+          tryChangeVGPRtoSGPRinCopy(MI, TRI, TII);
         }
 
         break;
       }
       case AMDGPU::PHI: {
         unsigned Reg = MI.getOperand(0).getReg();
         if (!TRI->isSGPRClass(MRI.getRegClass(Reg)))
           break;

test/CodeGen/AMDGPU/llvm.amdgcn.mqsad.pk.u16.u8.ll

 ; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 ; RUN: llc -march=amdgcn -mcpu=fiji -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 declare i64 @llvm.amdgcn.mqsad.pk.u16.u8(i64, i32, i64) #0
 
 ; GCN-LABEL: {{^}}v_mqsad_pk_u16_u8:
-; GCN: v_mqsad_pk_u16_u8 v[0:1], v[4:5], s{{[0-9]+}}, v[{{[0-9]+:[0-9]+}}]
+; GCN: v_mqsad_pk_u16_u8 v[0:1], v[4:5], v{{[0-9]+}}, s[{{[0-9]+:[0-9]+}}]
 ; GCN-DAG: v_mov_b32_e32 v5, v1
 ; GCN-DAG: v_mov_b32_e32 v4, v0
 define amdgpu_kernel void @v_mqsad_pk_u16_u8(i64 addrspace(1)* %out, i64 %src) {
   %tmp = call i64 asm "v_lsrlrev_b64 $0, $1, 1", "={v[4:5]},v"(i64 %src) #0
   %tmp1 = call i64 @llvm.amdgcn.mqsad.pk.u16.u8(i64 %tmp, i32 100, i64 100) #0
   %tmp2 = call i64 asm ";; force constraint", "=v,{v[4:5]}"(i64 %tmp1) #0
   store i64 %tmp2, i64 addrspace(1)* %out, align 4
   ret void

test/CodeGen/AMDGPU/llvm.amdgcn.qsad.pk.u16.u8.ll

 ; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 ; RUN: llc -march=amdgcn -mcpu=fiji -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
 declare i64 @llvm.amdgcn.qsad.pk.u16.u8(i64, i32, i64) #0
 
 ; GCN-LABEL: {{^}}v_qsad_pk_u16_u8:
-; GCN: v_qsad_pk_u16_u8 v[0:1], v[4:5], s{{[0-9]+}}, v[{{[0-9]+:[0-9]+}}]
+; GCN: v_qsad_pk_u16_u8 v[0:1], v[4:5], v{{[0-9]+}}, s[{{[0-9]+:[0-9]+}}]
 ; GCN-DAG: v_mov_b32_e32 v5, v1
 ; GCN-DAG: v_mov_b32_e32 v4, v0
 define amdgpu_kernel void @v_qsad_pk_u16_u8(i64 addrspace(1)* %out, i64 %src) {
   %tmp = call i64 asm "v_lsrlrev_b64 $0, $1, 1", "={v[4:5]},v"(i64 %src) #0
   %tmp1 = call i64 @llvm.amdgcn.qsad.pk.u16.u8(i64 %tmp, i32 100, i64 100) #0
   %tmp2 = call i64 asm ";; force constraint", "=v,{v[4:5]}"(i64 %tmp1) #0
   store i64 %tmp2, i64 addrspace(1)* %out, align 4
   ret void

test/CodeGen/AMDGPU/opt-sgpr-to-vgrp-copy.ll

+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+
+; Check that constant is in SGPR registers
+; GCN-LABEL: {{^}}const_to_sgpr:
+; GCN-DAG: s_mov_b32 s[[HI:[0-9]+]], 0
+; GCN-DAG: s_mov_b32 s[[LO:[0-9]+]], 0x100000
+; GCN:     v_cmp_gt_u64_e32 vcc, s{{\[}}[[LO]]:[[HI]]], v
+
+define amdgpu_kernel void @const_to_sgpr(i32 addrspace(1)* nocapture %arg, i64 %id) {
+bb:
+  %tid = tail call i32 @llvm.amdgcn.workitem.id.x() #0
+  %tid64 = zext i32 %tid to i64
+  %x = add i64 %tid64, %id
+  %cmp = icmp ugt i64 %x, 1048575
+  br i1 %cmp, label %bb2, label %bb1
+
+bb1:                                             ; preds = %bb
+  %gep = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 %tid64
+  store i32 0, i32 addrspace(1)* %gep, align 4
+  br label %bb2
+
+bb2:                                             ; preds = %bb1, %bb
+  ret void
+}
+
+declare i32 @llvm.amdgcn.workitem.id.x() #0
+
+attributes #0 = { nounwind readnone speculatable }

test/CodeGen/AMDGPU/sdwa-peephole.ll

 store_label:
   store <2 x i16> %add, <2 x i16> addrspace(1)* %out, align 4
   ret void
 }
 
 
 ; Check that "pulling out" SDWA operands works correctly.
 ; GCN-LABEL: {{^}}pulled_out_test:
-; NOSDWA-DAG: v_and_b32_e32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
+; NOSDWA-DAG: v_and_b32_e32 v{{[0-9]+}}, s{{[0-9]+}}, v{{[0-9]+}}
 ; NOSDWA-DAG: v_lshlrev_b16_e32 v{{[0-9]+}}, 8, v{{[0-9]+}}
-; NOSDWA-DAG: v_and_b32_e32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
+; NOSDWA-DAG: v_and_b32_e32 v{{[0-9]+}}, s{{[0-9]+}}, v{{[0-9]+}}
 ; NOSDWA-DAG: v_lshlrev_b16_e32 v{{[0-9]+}}, 8, v{{[0-9]+}}
 ; NOSDWA: v_or_b32_e32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
 ; NOSDWA-NOT: v_and_b32_sdwa
 ; NOSDWA-NOT: v_or_b32_sdwa
 
 ; SDWA-DAG: v_and_b32_sdwa v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD
 ; SDWA-DAG: v_lshlrev_b16_e32 v{{[0-9]+}}, 8, v{{[0-9]+}}
 ; SDWA-DAG: v_and_b32_sdwa v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:WORD_1 src1_sel:DWORD

test/CodeGen/AMDGPU/sint_to_fp.i64.ll

 ; GCN: v_xor_b32
 
 ; GCN: v_ffbh_u32
 ; GCN: v_ffbh_u32
 ; GCN: v_cndmask
 ; GCN: v_cndmask
 
 ; GCN-DAG: v_cmp_eq_u64
-; GCN-DAG: v_cmp_lt_u64
+; GCN-DAG: v_cmp_gt_u64
 
 ; GCN: v_xor_b32_e32 v{{[0-9]+}}, 0x80000000, v{{[0-9]+}}
 ; GCN: v_cndmask_b32_e{{32|64}} [[SIGN_SEL:v[0-9]+]],
 ; GCN: v_cvt_f16_f32_e32 [[SIGN_SEL_F16:v[0-9]+]], [[SIGN_SEL]]
 ; GCN: {{buffer|flat}}_store_short {{.*}}[[SIGN_SEL_F16]]
 define amdgpu_kernel void @v_sint_to_fp_i64_to_f16(half addrspace(1)* %out, i64 addrspace(1)* %in) #0 {
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   %in.gep = getelementptr i64, i64 addrspace(1)* %in, i32 %tid

 ; GCN: v_xor_b32
 
 ; GCN: v_ffbh_u32
 ; GCN: v_ffbh_u32
 ; GCN: v_cndmask
 ; GCN: v_cndmask
 
 ; GCN-DAG: v_cmp_eq_u64
-; GCN-DAG: v_cmp_lt_u64
+; GCN-DAG: v_cmp_gt_u64
 
 ; GCN: v_xor_b32_e32 v{{[0-9]+}}, 0x80000000, v{{[0-9]+}}
 ; GCN: v_cndmask_b32_e{{32|64}} [[SIGN_SEL:v[0-9]+]],
 ; GCN: {{buffer|flat}}_store_dword {{.*}}[[SIGN_SEL]]
 define amdgpu_kernel void @v_sint_to_fp_i64_to_f32(float addrspace(1)* %out, i64 addrspace(1)* %in) #0 {
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   %in.gep = getelementptr i64, i64 addrspace(1)* %in, i32 %tid
   %out.gep = getelementptr float, float addrspace(1)* %out, i32 %tid

test/CodeGen/AMDGPU/uint_to_fp.i64.ll

 ; GCN: {{buffer|flat}}_load_dwordx2
 
 ; GCN: v_ffbh_u32
 ; GCN: v_ffbh_u32
 ; GCN: v_cndmask
 ; GCN: v_cndmask
 
 ; GCN-DAG: v_cmp_eq_u64
-; GCN-DAG: v_cmp_lt_u64
+; GCN-DAG: v_cmp_gt_u64
 
 ; GCN: v_add_i32_e32 [[VR:v[0-9]+]]
 ; GCN: v_cvt_f16_f32_e32 [[VR_F16:v[0-9]+]], [[VR]]
 ; GCN: {{buffer|flat}}_store_short {{.*}}[[VR_F16]]
 define amdgpu_kernel void @v_uint_to_fp_i64_to_f16(half addrspace(1)* %out, i64 addrspace(1)* %in) #0 {
   %tid = call i32 @llvm.r600.read.tidig.x()
   %in.gep = getelementptr i64, i64 addrspace(1)* %in, i32 %tid
   %out.gep = getelementptr half, half addrspace(1)* %out, i32 %tid

 ; GCN: {{buffer|flat}}_load_dwordx2
 
 ; GCN: v_ffbh_u32
 ; GCN: v_ffbh_u32
 ; GCN: v_cndmask
 ; GCN: v_cndmask
 
 ; GCN-DAG: v_cmp_eq_u64
-; GCN-DAG: v_cmp_lt_u64
+; GCN-DAG: v_cmp_gt_u64
 
 ; GCN: v_add_i32_e32 [[VR:v[0-9]+]]
 ; GCN: {{buffer|flat}}_store_dword {{.*}}[[VR]]
 define amdgpu_kernel void @v_uint_to_fp_i64_to_f32(float addrspace(1)* %out, i64 addrspace(1)* %in) #0 {
   %tid = call i32 @llvm.r600.read.tidig.x()
   %in.gep = getelementptr i64, i64 addrspace(1)* %in, i32 %tid
   %out.gep = getelementptr float, float addrspace(1)* %out, i32 %tid
   %val = load i64, i64 addrspace(1)* %in.gep