[AMDGPU][NFC] Rename ReserveTrapVGPRs -> ReserveRegs

[kzhuravl]

Emit error if debug info is not present

[kzhuravl]

Ping

[arsenm]

lib/Target/AMDGPU/SIMachineFunctionInfo.cpp

140–143 ↗

(On Diff #56750)

I don't see why this would be an error, particularly a backend error. Talking about -g here doesn't make sense

[kzhuravl]

lib/Target/AMDGPU/SIMachineFunctionInfo.cpp

140–143 ↗

(On Diff #56750)

We want to restrict additional debugger features (register reservation, nops insertion, etc.) to be only used with debug info present and -O0 opt level.

I can change the error message to say "only allowed with debug info present".

Should we use assert instead? What are your thoughts?

[arsenm]

lib/Target/AMDGPU/SIMachineFunctionInfo.cpp

140–143 ↗

(On Diff #56750)

There should be no error. It should proceed and emit the program. There's no reason the registers can't be reserved with optimizations. The nop insertion will be less useful with optimizations, but should still work. Telling the user that they want to disable optimizations with debugging is a frontend question

[kzhuravl]

Address Matt's comments

[kzhuravl]

Ping

[kzhuravl]

Ping

[arsenm]

LGTM

[kzhuravl]

Commit rL270594

llvm/trunk/lib/Target/AMDGPU/AMDGPU.td

 
 def FeatureDebuggerInsertNops : SubtargetFeature<
   "amdgpu-debugger-insert-nops",
   "DebuggerInsertNops",
   "true",
   "Insert one nop instruction for each high level source statement"
 >;
 
-def FeatureDebuggerReserveTrapRegs : SubtargetFeature<
-  "amdgpu-debugger-reserve-trap-regs",
-  "DebuggerReserveTrapVGPRs",
+def FeatureDebuggerReserveRegs : SubtargetFeature<
+  "amdgpu-debugger-reserve-regs",
+  "DebuggerReserveRegs",
   "true",
-  "Reserve VGPRs for trap handler usage"
+  "Reserve registers for debugger usage"
 >;
 
 //===----------------------------------------------------------------------===//
 
 def AMDGPUInstrInfo : InstrInfo {
   let guessInstructionProperties = 1;
   let noNamedPositionallyEncodedOperands = 1;
 }

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

if (STM.isXNACKEnabled())
       ExtraSGPRs = 4;
 
     if (FlatUsed)
       ExtraSGPRs = 6;
   }
 
   MaxSGPR += ExtraSGPRs;
 
-  // Update necessary Reserved* fields and max VGPRs used if
-  // "amdgpu-debugger-reserve-trap-regs" attribute was specified.
-  if (STM.debuggerReserveTrapVGPRs()) {
+  // Record first reserved register and reserved register count fields, and
+  // update max register counts if "amdgpu-debugger-reserve-regs" attribute was
+  // specified.
+  if (STM.debuggerReserveRegs()) {
     ProgInfo.ReservedVGPRFirst = MaxVGPR + 1;
-    ProgInfo.ReservedVGPRCount = MFI->getDebuggerReserveTrapVGPRCount();
-    MaxVGPR += MFI->getDebuggerReserveTrapVGPRCount();
+    ProgInfo.ReservedVGPRCount = MFI->getDebuggerReservedVGPRCount();
+    MaxVGPR += MFI->getDebuggerReservedVGPRCount();
   }
 
   // We found the maximum register index. They start at 0, so add one to get the
   // number of registers.
   ProgInfo.NumVGPR = MaxVGPR + 1;
   ProgInfo.NumSGPR = MaxSGPR + 1;
 
   if (STM.hasSGPRInitBug()) {

llvm/trunk/lib/Target/AMDGPU/AMDGPUSubtarget.h

private:
   bool CIInsts;
   bool HasSMemRealTime;
   bool Has16BitInsts;
   bool FeatureDisable;
   int LDSBankCount;
   unsigned IsaVersion;
   bool EnableSIScheduler;
   bool DebuggerInsertNops;
-  bool DebuggerReserveTrapVGPRs;
+  bool DebuggerReserveRegs;
 
   std::unique_ptr<AMDGPUFrameLowering> FrameLowering;
   std::unique_ptr<AMDGPUTargetLowering> TLInfo;
   std::unique_ptr<AMDGPUInstrInfo> InstrInfo;
   std::unique_ptr<GISelAccessor> GISel;
   InstrItineraryData InstrItins;
   Triple TargetTriple;
 
public:
   bool enableSIScheduler() const {
     return EnableSIScheduler;
   }
 
   bool debuggerInsertNops() const {
     return DebuggerInsertNops;
   }
 
-  bool debuggerReserveTrapVGPRs() const {
-    return DebuggerReserveTrapVGPRs;
+  bool debuggerReserveRegs() const {
+    return DebuggerReserveRegs;
   }
 
   bool dumpCode() const {
     return DumpCode;
   }
   bool r600ALUEncoding() const {
     return R600ALUInst;
   }

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

: AMDGPUGenSubtargetInfo(TT, GPU, FS),
       WavefrontSize(0), CFALUBug(false),
       LocalMemorySize(0), MaxPrivateElementSize(0),
       EnableVGPRSpilling(false), SGPRInitBug(false), IsGCN(false),
       GCN1Encoding(false), GCN3Encoding(false), CIInsts(false),
       HasSMemRealTime(false), Has16BitInsts(false),
       LDSBankCount(0),
       IsaVersion(ISAVersion0_0_0),
       EnableSIScheduler(false),
-      DebuggerInsertNops(false), DebuggerReserveTrapVGPRs(false),
+      DebuggerInsertNops(false), DebuggerReserveRegs(false),
       FrameLowering(nullptr),
       GISel(),
       InstrItins(getInstrItineraryForCPU(GPU)), TargetTriple(TT) {
 
   initializeSubtargetDependencies(TT, GPU, FS);
 
   const unsigned MaxStackAlign = 64 * 16; // Maximum stack alignment (long16)
 

llvm/trunk/lib/Target/AMDGPU/SIMachineFunctionInfo.h

class SIMachineFunctionInfo final : public AMDGPUMachineFunction {
   unsigned PrivateSegmentWaveByteOffsetSystemSGPR;
 
   // Graphics info.
   unsigned PSInputAddr;
   bool ReturnsVoid;
 
   unsigned MaximumWorkGroupSize;
 
-  // Number of reserved VGPRs for trap handler usage.
-  unsigned DebuggerReserveTrapVGPRCount;
+  // Number of reserved VGPRs for debugger usage.
+  unsigned DebuggerReservedVGPRCount;
 
 public:
   // FIXME: Make private
   unsigned LDSWaveSpillSize;
   unsigned PSInputEna;
   std::map<unsigned, unsigned> LaneVGPRs;
   unsigned ScratchOffsetReg;
   unsigned NumUserSGPRs;
public:
   bool returnsVoid() const {
     return ReturnsVoid;
   }
 
   void setIfReturnsVoid(bool Value) {
     ReturnsVoid = Value;
   }
 
-  unsigned getDebuggerReserveTrapVGPRCount() const {
-    return DebuggerReserveTrapVGPRCount;
+  /// \returns Number of reserved VGPRs for debugger usage.
+  unsigned getDebuggerReservedVGPRCount() const {
+    return DebuggerReservedVGPRCount;
   }
 
   unsigned getMaximumWorkGroupSize(const MachineFunction &MF) const;
 };
 
 } // End namespace llvm
 
 #endif

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

: AMDGPUMachineFunction(MF),
     WorkGroupIDXSystemSGPR(AMDGPU::NoRegister),
     WorkGroupIDYSystemSGPR(AMDGPU::NoRegister),
     WorkGroupIDZSystemSGPR(AMDGPU::NoRegister),
     WorkGroupInfoSystemSGPR(AMDGPU::NoRegister),
     PrivateSegmentWaveByteOffsetSystemSGPR(AMDGPU::NoRegister),
     PSInputAddr(0),
     ReturnsVoid(true),
     MaximumWorkGroupSize(0),
-    DebuggerReserveTrapVGPRCount(0),
+    DebuggerReservedVGPRCount(0),
     LDSWaveSpillSize(0),
     PSInputEna(0),
     NumUserSGPRs(0),
     NumSystemSGPRs(0),
     HasSpilledSGPRs(false),
     HasSpilledVGPRs(false),
     HasNonSpillStackObjects(false),
     HasFlatInstructions(false),
if (HasStackObjects && ST.getGeneration() >= AMDGPUSubtarget::SEA_ISLANDS &&
       ST.isAmdHsaOS())
     FlatScratchInit = true;
 
   if (AMDGPU::isCompute(F->getCallingConv()))
     MaximumWorkGroupSize = AMDGPU::getMaximumWorkGroupSize(*F);
   else
     MaximumWorkGroupSize = ST.getWavefrontSize();
 
-  if (ST.debuggerReserveTrapVGPRs())
-    DebuggerReserveTrapVGPRCount = 4;
+  if (ST.debuggerReserveRegs())
+    DebuggerReservedVGPRCount = 4;
 }
 
 unsigned SIMachineFunctionInfo::addPrivateSegmentBuffer(
   const SIRegisterInfo &TRI) {
   PrivateSegmentBufferUserSGPR = TRI.getMatchingSuperReg(
     getNextUserSGPR(), AMDGPU::sub0, &AMDGPU::SReg_128RegClass);
   NumUserSGPRs += 4;
   return PrivateSegmentBufferUserSGPR;

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

BitVector SIRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   if (ScratchRSrcReg != AMDGPU::NoRegister) {
     // Reserve 4 SGPRs for the scratch buffer resource descriptor in case we need
     // to spill.
     // TODO: May need to reserve a VGPR if doing LDS spilling.
     reserveRegisterTuples(Reserved, ScratchRSrcReg);
     assert(!isSubRegister(ScratchRSrcReg, ScratchWaveOffsetReg));
   }
 
-  // Reserve VGPRs for trap handler usage if "amdgpu-debugger-reserve-trap-regs"
+  // Reserve registers for debugger usage if "amdgpu-debugger-reserve-trap-regs"
   // attribute was specified.
   const AMDGPUSubtarget &ST = MF.getSubtarget<AMDGPUSubtarget>();
-  if (ST.debuggerReserveTrapVGPRs()) {
+  if (ST.debuggerReserveRegs()) {
     unsigned ReservedVGPRFirst =
-      MaxWorkGroupVGPRCount - MFI->getDebuggerReserveTrapVGPRCount();
+      MaxWorkGroupVGPRCount - MFI->getDebuggerReservedVGPRCount();
     for (unsigned i = ReservedVGPRFirst; i < MaxWorkGroupVGPRCount; ++i) {
       unsigned Reg = AMDGPU::VGPR_32RegClass.getRegister(i);
       reserveRegisterTuples(Reserved, Reg);
     }
   }
 
   return Reserved;
 }

llvm/trunk/test/CodeGen/AMDGPU/debugger-reserve-regs.ll

-; RUN: llc -mtriple=amdgcn--amdhsa -mcpu=fiji -mattr=+amdgpu-debugger-reserve-trap-regs -verify-machineinstrs < %s | FileCheck %s
-
+; RUN: llc -O0 -mtriple=amdgcn--amdhsa -mcpu=fiji -mattr=+amdgpu-debugger-reserve-regs -verify-machineinstrs < %s | FileCheck %s
 ; CHECK: reserved_vgpr_first = {{[0-9]+}}
 ; CHECK-NEXT: reserved_vgpr_count = 4
-
 ; CHECK: ReservedVGPRFirst: {{[0-9]+}}
 ; CHECK-NEXT: ReservedVGPRCount: 4
 
 ; Function Attrs: nounwind
 define void @test(i32 addrspace(1)* %A) #0 !dbg !12 {
 entry:
   %A.addr = alloca i32 addrspace(1)*, align 4
   store i32 addrspace(1)* %A, i32 addrspace(1)** %A.addr, align 4