[AMDGPU][MIR] Serialize SpillPhysVGPRs #113129

optimisan · 2024-10-21T06:15:58Z

No description provided.

llvmbot · 2024-10-21T06:22:01Z

@llvm/pr-subscribers-backend-amdgpu

Author: Akshat Oke (optimisan)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/113129.diff

4 Files Affected:

(modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+7)
(modified) llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp (+3)
(modified) llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h (+3)
(added) llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir (+57)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index e4cc522194f2a9..d2a24c921094db 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -1718,6 +1718,13 @@ bool GCNTargetMachine::parseMachineFunctionInfo(
     MFI->reserveWWMRegister(ParsedReg);
   }
 
+  for (const auto &YamlRegStr : YamlMFI.SpillPhysVGPRS) {
+    Register ParsedReg;
+    if (parseRegister(YamlRegStr, ParsedReg))
+      return true;
+    MFI->SpillPhysVGPRs.push_back(ParsedReg);
+  }
+
   auto parseAndCheckArgument = [&](const std::optional<yaml::SIArgument> &A,
                                    const TargetRegisterClass &RC,
                                    ArgDescriptor &Arg, unsigned UserSGPRs,
diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
index e59dd724b94f8b..1e43d2727a00da 100644
--- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
@@ -711,6 +711,9 @@ yaml::SIMachineFunctionInfo::SIMachineFunctionInfo(
       PSInputAddr(MFI.getPSInputAddr()),
       PSInputEnable(MFI.getPSInputEnable()),
       Mode(MFI.getMode()) {
+  for (Register Reg : MFI.getSGPRSpillPhysVGPRs())
+    SpillPhysVGPRS.push_back(regToString(Reg, TRI));
+
   for (Register Reg : MFI.getWWMReservedRegs())
     WWMReservedRegs.push_back(regToString(Reg, TRI));
 
diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h
index c8c305e24c7101..018322eaa18665 100644
--- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h
@@ -275,6 +275,7 @@ struct SIMachineFunctionInfo final : public yaml::MachineFunctionInfo {
   // TODO: 10 may be a better default since it's the maximum.
   unsigned Occupancy = 0;
 
+  SmallVector<StringValue, 2> SpillPhysVGPRS;
   SmallVector<StringValue> WWMReservedRegs;
 
   StringValue ScratchRSrcReg = "$private_rsrc_reg";
@@ -336,6 +337,7 @@ template <> struct MappingTraits<SIMachineFunctionInfo> {
     YamlIO.mapOptional("highBitsOf32BitAddress",
                        MFI.HighBitsOf32BitAddress, 0u);
     YamlIO.mapOptional("occupancy", MFI.Occupancy, 0);
+    YamlIO.mapOptional("spillPhysVGPRs", MFI.SpillPhysVGPRS);
     YamlIO.mapOptional("wwmReservedRegs", MFI.WWMReservedRegs);
     YamlIO.mapOptional("scavengeFI", MFI.ScavengeFI);
     YamlIO.mapOptional("vgprForAGPRCopy", MFI.VGPRForAGPRCopy,
@@ -610,6 +612,7 @@ class SIMachineFunctionInfo final : public AMDGPUMachineFunction,
   }
 
   ArrayRef<Register> getSGPRSpillVGPRs() const { return SpillVGPRs; }
+  ArrayRef<Register> getSGPRSpillPhysVGPRs() const { return SpillPhysVGPRs; }
 
   const WWMSpillsMap &getWWMSpills() const { return WWMSpills; }
   const ReservedRegSet &getWWMReservedRegs() const { return WWMReservedRegs; }
diff --git a/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir b/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir
new file mode 100644
index 00000000000000..083ebcd9fa5683
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir
@@ -0,0 +1,57 @@
+# RUN: llc -mtriple=amdgcn-amd-amdhsa --start-before=si-lower-sgpr-spills --stop-after=prologepilog -o - %s | FileCheck %s
+
+# CHECK: csr_sgpr_spill
+# CHECK: spillPhysVGPRs:
+# CHECK-NEXT: - '$vgpr0'
+---
+name: csr_sgpr_spill
+tracksRegLiveness: true
+body: |
+  bb.0:
+    S_NOP 0
+  bb.1:
+    $sgpr40 = S_MOV_B32 0
+    $sgpr41 = S_MOV_B32 1
+
+...
+
+# CHECK-LABEL: name: parse_none
+# CHECK: machineFunctionInfo:
+# CHECK-NOT: spillPhysVGPRs:
+---
+name: parse_none
+machineFunctionInfo:
+  spillPhysVGPRs: []
+body: |
+  bb.0:
+    S_ENDPGM 0
+
+...
+
+# CHECK-LABEL: name: parse_one
+# CHECK: machineFunctionInfo:
+# CHECK: spillPhysVGPRs:
+# CHECK-NEXT: - '$vgpr0'
+---
+name: parse_one
+machineFunctionInfo:
+  spillPhysVGPRs: ['$vgpr0']
+body: |
+  bb.0:
+    S_ENDPGM 0
+
+...
+
+# CHECK-LABEL: name: parse_one
+# CHECK: machineFunctionInfo:
+# CHECK: spillPhysVGPRs:
+# CHECK-NEXT: - '$vgpr0'
+---
+name: parse_one
+machineFunctionInfo:
+  spillPhysVGPRs: ['$vgpr0']
+body: |
+  bb.0:
+    S_ENDPGM 0
+
+...

llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

arsenm · 2024-10-21T21:07:19Z

llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h

@@ -275,6 +275,7 @@ struct SIMachineFunctionInfo final : public yaml::MachineFunctionInfo {
  // TODO: 10 may be a better default since it's the maximum.
  unsigned Occupancy = 0;

+  SmallVector<StringValue, 2> SpillPhysVGPRS;


I see the name was copied from the existing MFI field, but this is not a good name. We ought to rename this to reflect the special usage; this isn't just any spilled physical vgprs

iinw these are VGPRs used for CSR or FP/BP SGPR spills. This is also related to SGPRSpillToPhysicalVGPRLanes map.

Would PhysVGPRsForSGPRSpills or SGPRSpillPhysVGPRS still be a bit general?

WWMPhysVGPRs?

llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir

arsenm

lgtm with nit

llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs-invalid.mir

llvm-ci · 2024-11-05T08:39:49Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/8250

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
RUN: at line 1: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn-amd-amdhsa --start-before=si-lower-sgpr-spills --stop-after=prologepilog -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir | /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=amdgcn-amd-amdhsa --start-before=si-lower-sgpr-spills --stop-after=prologepilog -o - /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs.mir

# After Prologue/Epilogue Insertion & Frame Finalization
# Machine code for function csr_sgpr_spill: NoPHIs, TracksLiveness, NoVRegs, TracksDebugUserValues
Frame Objects:
  fi#0: dead
  fi#1: dead
  fi#2: size=4, align=4, at location [SP]
  fi#3: size=4, align=4, at location [SP+4]

bb.0:
  successors: %bb.1(0x80000000); %bb.1(100.00%)
  liveins: $sgpr40, $sgpr41
  $sgpr0_sgpr1 = S_XOR_SAVEEXEC_B64 -1, implicit-def $exec, implicit-def dead $scc, implicit $exec
  BUFFER_STORE_DWORD_OFFSET killed $vgpr0, $private_rsrc_reg, $sp_reg, 0, 0, 0, implicit $exec :: (store (s32) into %stack.2, addrspace 5)
  $exec = S_MOV_B64 killed $sgpr0_sgpr1
  $vgpr0 = SI_SPILL_S32_TO_VGPR $sgpr40, 0, $vgpr0(tied-def 0)
  $vgpr0 = SI_SPILL_S32_TO_VGPR $sgpr41, 1, $vgpr0(tied-def 0)
  S_NOP 0

bb.1:
; predecessors: %bb.0

  $sgpr40 = S_MOV_B32 0
  $sgpr41 = S_MOV_B32 1

# End machine code for function csr_sgpr_spill.

*** Bad machine code: Operand has incorrect register class. ***
- function:    csr_sgpr_spill
- basic block: %bb.0  (0xa020ea8)
- instruction: BUFFER_STORE_DWORD_OFFSET killed $vgpr0, $private_rsrc_reg, $sp_reg, 0, 0, 0, implicit $exec :: (store (s32) into %stack.2, addrspace 5)

*** Bad machine code: Illegal physical register for instruction ***
- function:    csr_sgpr_spill
- basic block: %bb.0  (0xa020ea8)
- instruction: BUFFER_STORE_DWORD_OFFSET killed $vgpr0, $private_rsrc_reg, $sp_reg, 0, 0, 0, implicit $exec :: (store (s32) into %stack.2, addrspace 5)
- operand 1:   $private_rsrc_reg
$private_rsrc_reg is not a SReg_128 register.

*** Bad machine code: Illegal physical register for instruction ***
- function:    csr_sgpr_spill
- basic block: %bb.0  (0xa020ea8)
...

Needed to specify scratchRSrcReg and spreg in order to stop after prologepilog. - Fixes #113129 test failure

Needed to specify scratchRSrcReg and spreg in order to stop after prologepilog. - Fixes llvm#113129 test failure

optimisan force-pushed the serialize-spill-phys-vgprs branch 2 times, most recently from dcbeba6 to a221286 Compare October 21, 2024 06:18

optimisan marked this pull request as ready for review October 21, 2024 06:21

optimisan requested a review from arsenm October 21, 2024 06:21

llvmbot added the backend:AMDGPU label Oct 21, 2024

optimisan requested review from Pierre-vh and cdevadas October 21, 2024 06:21

arsenm reviewed Oct 21, 2024

View reviewed changes

optimisan force-pushed the serialize-spill-phys-vgprs branch from c023b4b to 63447a0 Compare October 22, 2024 09:50

arsenm approved these changes Oct 29, 2024

View reviewed changes

llvm/test/CodeGen/MIR/AMDGPU/spill-phys-vgprs-invalid.mir Show resolved Hide resolved

optimisan added 4 commits November 5, 2024 06:23

[AMDGPU][MIR] Serialize SpillPhysVGPRs

cc81489

AS

86b0016

Add reg parse failure test

616f534

add not a reg test

aa76832

optimisan force-pushed the serialize-spill-phys-vgprs branch from 63447a0 to aa76832 Compare November 5, 2024 06:53

optimisan merged commit 3495d04 into llvm:main Nov 5, 2024
6 of 7 checks passed

optimisan mentioned this pull request Nov 5, 2024

[AMDGPU] Fix 3495d04 MIR test #114963

Merged

optimisan added a commit that referenced this pull request Nov 5, 2024

[AMDGPU] Fix 3495d04 MIR test (#114963)

e76d921

Needed to specify scratchRSrcReg and spreg in order to stop after prologepilog. - Fixes #113129 test failure

PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024

[AMDGPU][MIR] Serialize SpillPhysVGPRs (llvm#113129)

41793b5

PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024

[AMDGPU] Fix 3495d04 MIR test (llvm#114963)

37d810b

Needed to specify scratchRSrcReg and spreg in order to stop after prologepilog. - Fixes llvm#113129 test failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AMDGPU][MIR] Serialize SpillPhysVGPRs #113129

[AMDGPU][MIR] Serialize SpillPhysVGPRs #113129

Uh oh!

optimisan commented Oct 21, 2024

Uh oh!

llvmbot commented Oct 21, 2024

Uh oh!

Uh oh!

Uh oh!

arsenm Oct 21, 2024

Uh oh!

optimisan Oct 22, 2024 •

edited

Loading

Uh oh!

arsenm Oct 29, 2024

Uh oh!

optimisan Oct 29, 2024

Uh oh!

Uh oh!

arsenm left a comment

Uh oh!

Uh oh!

Uh oh!

llvm-ci commented Nov 5, 2024

Uh oh!

Uh oh!

[AMDGPU][MIR] Serialize SpillPhysVGPRs #113129

[AMDGPU][MIR] Serialize SpillPhysVGPRs #113129

Uh oh!

Conversation

optimisan commented Oct 21, 2024

Uh oh!

llvmbot commented Oct 21, 2024

Uh oh!

Uh oh!

Uh oh!

arsenm Oct 21, 2024

Choose a reason for hiding this comment

Uh oh!

optimisan Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arsenm Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

optimisan Oct 29, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvm-ci commented Nov 5, 2024

Uh oh!

Uh oh!

optimisan Oct 22, 2024 •

edited

Loading