[AMDGPU] Reserve all AGPRs on targets which do not have them

[kzhuravl]

LGTM

[bjope]

llvm/trunk/test/CodeGen/AMDGPU/spill-vgpr-to-agpr.ll

2 ↗

(On Diff #212410)

What is the idea behind this test. This single RUN-line takes more than 50 seconds to run, just to discover a failure?

A little bit annoying when llvm-lit test/CodeGen/AMDGPU/ only takes 11s when removing this particular RUN-line.

[arsenm]

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

223 ↗

(On Diff #212410)

Do we really need to do this? If there are no AGPRs, no AGPRs virtual registers should be created for this to come up

[rampitec]

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

223 ↗

(On Diff #212410)

It is just a safety net. Somebody can use an inline asm with agprs (and sometimes do).

[rampitec]

llvm/trunk/test/CodeGen/AMDGPU/spill-vgpr-to-agpr.ll

2 ↗

(On Diff #212410)

I just have tried it. This run took 4.5 seconds with the debug build. It is still fairly long, but nothing close to 50s.

[bjope]

llvm/trunk/test/CodeGen/AMDGPU/spill-vgpr-to-agpr.ll

2 ↗

(On Diff #212410)

Ehm, ok, sorry! Seem to be something downstream.

Using latest version from upstream master is fast (0.6s).
But on our development track it seem to spend an awful lot of time in "SI post-RA bundler". It must be some modification that we have done downstream, so I'll try to debug that myself.

[arsenm]

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

223 ↗

(On Diff #212410)

This would need a deliberate attempt to diagnose it. This won't stop you from using AGPRs since it still won't stop you from using physical AGPRs. I think this would just break regalloc in an unpredicatable way.

These loops over all registers aren't exactly cheap either, and getReservedRegs is called a few too many times. This should probably be removed

[rampitec]

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

223 ↗

(On Diff #212410)

Well, maybe. It does not give 100% guarantee anyway.

lib/Target/AMDGPU/SIISelLowering.cpp

case 'v':
         RC = &AMDGPU::VReg_256RegClass;
         break;
       case 512:
         RC = &AMDGPU::VReg_512RegClass;
         break;
       }
       break;
     case 'a':
+      if (!Subtarget->hasMAIInsts())
+        break;
       switch (VT.getSizeInBits()) {
       default:
         return std::make_pair(0U, nullptr);
       case 32:
       case 16:
         RC = &AMDGPU::AGPR_32RegClass;
         break;
       case 64:

lib/Target/AMDGPU/SIRegisterInfo.cpp

BitVector SIRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   unsigned TotalNumVGPRs = AMDGPU::VGPR_32RegClass.getNumRegs();
   for (unsigned i = MaxNumVGPRs; i < TotalNumVGPRs; ++i) {
     unsigned Reg = AMDGPU::VGPR_32RegClass.getRegister(i);
     reserveRegisterTuples(Reserved, Reg);
     Reg = AMDGPU::AGPR_32RegClass.getRegister(i);
     reserveRegisterTuples(Reserved, Reg);
   }
 
+  // Reserve all the rest AGPRs if there are no instructions to use it.
+  if (!ST.hasMAIInsts()) {
+    for (unsigned i = 0; i < MaxNumVGPRs; ++i) {
+      unsigned Reg = AMDGPU::AGPR_32RegClass.getRegister(i);
+      reserveRegisterTuples(Reserved, Reg);
+    }
+  }
+
   const SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
 
   unsigned ScratchWaveOffsetReg = MFI->getScratchWaveOffsetReg();
   if (ScratchWaveOffsetReg != AMDGPU::NoRegister) {
     // Reserve 1 SGPR for scratch wave offset in case we need to spill.
     reserveRegisterTuples(Reserved, ScratchWaveOffsetReg);
   }
 

test/CodeGen/AMDGPU/spill-vgpr-to-agpr.ll

 ; RUN: llc -march=amdgcn -mcpu=gfx908 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX908 %s
-; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX900 %s
+; RUN: not llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefixes=GCN,GFX900 %s
 
 ; GCN-LABEL: {{^}}max_10_vgprs:
 ; GFX900-DAG: s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD0
 ; GFX900-DAG: s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD1
 ; GFX908-NOT: SCRATCH_RSRC
 ; GFX908-DAG: v_accvgpr_write_b32 a0, v{{[0-9]}}
 ; GFX908-DAG: v_accvgpr_write_b32 a1, v{{[0-9]}}
 ; GFX900:     buffer_store_dword v{{[0-9]}},
define amdgpu_kernel void @max_10_vgprs(i32 addrspace(1)* %p) #0 {
   store volatile i32 %v7, i32 addrspace(1)* undef
   store volatile i32 %v8, i32 addrspace(1)* undef
   store volatile i32 %v9, i32 addrspace(1)* undef
   store volatile i32 %v10, i32 addrspace(1)* undef
   ret void
 }
 
 ; GCN-LABEL: {{^}}max_10_vgprs_used_9a:
-; GCN-DAG:    s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD0
-; GCN-DAG:    s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD1
+; GFX908-DAG: s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD0
+; GFX908-DAG: s_mov_b32 s{{[0-9]+}}, SCRATCH_RSRC_DWORD1
 ; GFX908:     v_accvgpr_write_b32 a9, v{{[0-9]}}
-; GCN:        buffer_store_dword v{{[0-9]}},
-; GFX900:     buffer_store_dword v{{[0-9]}},
-; GFX900:     buffer_load_dword v{{[0-9]}},
-; GFX900:     buffer_load_dword v{{[0-9]}},
+; GFX908:     buffer_store_dword v{{[0-9]}},
 ; GFX908-NOT: buffer_
 ; GFX908:     v_accvgpr_read_b32 v{{[0-9]}}, a9
 ; GFX908:     buffer_load_dword v{{[0-9]}},
 ; GFX908-NOT: buffer_
 
-; GCN:    NumVgprs: 10
-; GFX900: ScratchSize: 12
+; GFX900:     couldn't allocate input reg for constraint 'a'
+
+; GFX908: NumVgprs: 10
 ; GFX908: ScratchSize: 8
-; GCN:    VGPRBlocks: 2
-; GCN:    NumVGPRsForWavesPerEU: 10
+; GFX908: VGPRBlocks: 2
+; GFX908: NumVGPRsForWavesPerEU: 10
 define amdgpu_kernel void @max_10_vgprs_used_9a(i32 addrspace(1)* %p) #0 {
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   call void asm sideeffect "", "a,a,a,a,a,a,a,a,a"(i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9)
   %p1 = getelementptr inbounds i32, i32 addrspace(1)* %p, i32 %tid
   %p2 = getelementptr inbounds i32, i32 addrspace(1)* %p1, i32 4
   %p3 = getelementptr inbounds i32, i32 addrspace(1)* %p2, i32 8
   %p4 = getelementptr inbounds i32, i32 addrspace(1)* %p3, i32 12
   %p5 = getelementptr inbounds i32, i32 addrspace(1)* %p4, i32 16