Skip to content

[AMDGPU] Remove Dwarf encodings for subregisters #117891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 6, 2025
Merged

Conversation

epilk
Copy link
Member

@epilk epilk commented Nov 27, 2024

Previously, registers and subregisters mapped to the same Dwarf encoding. We don't really have any way to refer to subregisters directly from Dwarf, the expression emitter should instead use DW_OPs to stencil out the subregister from the whole register. This was also confusing tools that need to map back to the llvm reg (e.g. dwarfdump), since getLLVMRegNum() would arbitrarily return the _LO16 register.

Previously, registers and subregisters mapped to the same Dwarf
encoding. We don't really have any way to refer to subregisters directly
from Dwarf, the expression emitter should instead use DW_OPs to stencil
out the subregister from the whole register. This was also confusing
tools that need to map back to the llvm reg (e.g. dwarfdump), since
getLLVMRegNum() would arbitrarily return the _LO16 register.
@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Emma Pilkington (epilk)

Changes

Previously, registers and subregisters mapped to the same Dwarf encoding. We don't really have any way to refer to subregisters directly from Dwarf, the expression emitter should instead use DW_OPs to stencil out the subregister from the whole register. This was also confusing tools that need to map back to the llvm reg (e.g. dwarfdump), since getLLVMRegNum() would arbitrarily return the _LO16 register.


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

4 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIRegisterInfo.td (+16-12)
  • (modified) llvm/test/CodeGen/AMDGPU/waitcnt-meta-instructions.mir (+1-1)
  • (modified) llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp (+10-8)
  • (modified) llvm/unittests/Target/AMDGPU/DwarfRegMappings.cpp (+34-8)
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
index 6a349d2bf06ea2..815dc56848738b 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
@@ -153,14 +153,16 @@ class SIRegisterClass <string n, list<ValueType> rTypes, int Align, dag rList>
 }
 
 multiclass SIRegLoHi16 <string n, bits<8> regIdx, bit ArtificialHigh = 1,
-                        bit isVGPR = 0, bit isAGPR = 0> {
+                        bit isVGPR = 0, bit isAGPR = 0,
+                        list<int> DwarfEncodings = [-1, -1]> {
   def _LO16 : SIReg<n#".l", regIdx, isVGPR, isAGPR>;
   def _HI16 : SIReg<!if(ArtificialHigh, "", n#".h"), regIdx, isVGPR, isAGPR,
                     /* isHi16 */ 1> {
     let isArtificial = ArtificialHigh;
   }
   def "" : RegisterWithSubRegs<n, [!cast<Register>(NAME#"_LO16"),
-                                   !cast<Register>(NAME#"_HI16")]> {
+                                   !cast<Register>(NAME#"_HI16")]>,
+           DwarfRegNum<DwarfEncodings> {
     let Namespace = "AMDGPU";
     let SubRegIndices = [lo16, hi16];
     let CoveredBySubRegs = !not(ArtificialHigh);
@@ -197,7 +199,8 @@ def VCC : RegisterWithSubRegs<"vcc", [VCC_LO, VCC_HI]> {
   let HWEncoding = VCC_LO.HWEncoding;
 }
 
-defm EXEC_LO : SIRegLoHi16<"exec_lo", 126>, DwarfRegNum<[1, 1]>;
+defm EXEC_LO : SIRegLoHi16<"exec_lo", 126, /*ArtificialHigh=*/1, /*isVGPR=*/0,
+                           /*isAGPR=*/0, /*DwarfEncodings=*/[1, 1]>;
 defm EXEC_HI : SIRegLoHi16<"exec_hi", 127>;
 
 def EXEC : RegisterWithSubRegs<"exec", [EXEC_LO, EXEC_HI]>, DwarfRegNum<[17, 1]> {
@@ -337,25 +340,26 @@ def FLAT_SCR : FlatReg<FLAT_SCR_LO, FLAT_SCR_HI, 0>;
 // SGPR registers
 foreach Index = 0...105 in {
   defm SGPR#Index :
-     SIRegLoHi16 <"s"#Index, Index>,
-     DwarfRegNum<[!if(!le(Index, 63), !add(Index, 32), !add(Index, 1024)),
-                  !if(!le(Index, 63), !add(Index, 32), !add(Index, 1024))]>;
+     SIRegLoHi16 <"s"#Index, Index, /*ArtificialHigh=*/1,
+                  /*isVGPR=*/0, /*isAGPR=*/0, /*DwarfEncodings=*/
+                  [!if(!le(Index, 63), !add(Index, 32), !add(Index, 1024)),
+                   !if(!le(Index, 63), !add(Index, 32), !add(Index, 1024))]>;
 }
 
 // VGPR registers
 foreach Index = 0...255 in {
   defm VGPR#Index :
-    SIRegLoHi16 <"v"#Index, Index, /* ArtificialHigh= */ 0,
-                 /* isVGPR= */ 1, /* isAGPR= */ 0>,
-    DwarfRegNum<[!add(Index, 2560), !add(Index, 1536)]>;
+    SIRegLoHi16 <"v"#Index, Index, /*ArtificialHigh=*/ 0,
+                 /*isVGPR=*/ 1, /*isAGPR=*/ 0, /*DwarfEncodings=*/
+                 [!add(Index, 2560), !add(Index, 1536)]>;
 }
 
 // AccVGPR registers
 foreach Index = 0...255 in {
   defm AGPR#Index :
-      SIRegLoHi16 <"a"#Index, Index, /* ArtificialHigh= */ 1,
-                   /* isVGPR= */ 0, /* isAGPR= */ 1>,
-      DwarfRegNum<[!add(Index, 3072), !add(Index, 2048)]>;
+      SIRegLoHi16 <"a"#Index, Index, /*ArtificialHigh=*/ 1,
+                   /*isVGPR=*/ 0, /*isAGPR=*/ 1, /*DwarfEncodings=*/
+                   [!add(Index, 3072), !add(Index, 2048)]>;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/CodeGen/AMDGPU/waitcnt-meta-instructions.mir b/llvm/test/CodeGen/AMDGPU/waitcnt-meta-instructions.mir
index ad4ad6df73e7f6..b663acb8ce3fde 100644
--- a/llvm/test/CodeGen/AMDGPU/waitcnt-meta-instructions.mir
+++ b/llvm/test/CodeGen/AMDGPU/waitcnt-meta-instructions.mir
@@ -67,7 +67,7 @@ body:             |
     ; GCN-NEXT: {{  $}}
     ; GCN-NEXT: S_WAITCNT 0
     ; GCN-NEXT: $vgpr0 = GLOBAL_LOAD_DWORD $vgpr0_vgpr1, 0, 0, implicit $exec
-    ; GCN-NEXT: CFI_INSTRUCTION offset $vgpr0_lo16, 16
+    ; GCN-NEXT: CFI_INSTRUCTION offset $vgpr0, 16
     $vgpr0 = GLOBAL_LOAD_DWORD $vgpr0_vgpr1, 0, 0, implicit $exec
     CFI_INSTRUCTION offset $vgpr0, 16
 
diff --git a/llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp b/llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp
index a0c843711a2197..259d68ed95b208 100644
--- a/llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp
+++ b/llvm/unittests/MC/AMDGPU/DwarfRegMappings.cpp
@@ -51,10 +51,11 @@ TEST(AMDGPUDwarfRegMappingTests, TestWave64DwarfRegMapping) {
       // PC_64 => 16, EXEC_MASK_64 => 17, S0 => 32, S63 => 95,
       // S64 => 1088, S105 => 1129, V0 => 2560, V255 => 2815,
       // A0 => 3072, A255 => 3327
-      for (int llvmReg : {16, 17, 32, 95, 1088, 1129, 2560, 2815, 3072, 3327}) {
-        MCRegister PCReg(*MRI->getLLVMRegNum(llvmReg, false));
-        EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, false));
-        EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, true));
+      for (int DwarfEncoding :
+           {16, 17, 32, 95, 1088, 1129, 2560, 2815, 3072, 3327}) {
+        MCRegister Reg = *MRI->getLLVMRegNum(DwarfEncoding, false);
+        EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, false));
+        EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, true));
       }
     }
   }
@@ -70,10 +71,11 @@ TEST(AMDGPUDwarfRegMappingTests, TestWave32DwarfRegMapping) {
       // PC_64 => 16, EXEC_MASK_32 => 1, S0 => 32, S63 => 95,
       // S64 => 1088, S105 => 1129, V0 => 1536, V255 => 1791,
       // A0 => 2048, A255 => 2303
-      for (int llvmReg : {16, 1, 32, 95, 1088, 1129, 1536, 1791, 2048, 2303}) {
-        MCRegister PCReg(*MRI->getLLVMRegNum(llvmReg, false));
-        EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, false));
-        EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, true));
+      for (int DwarfEncoding :
+           {16, 1, 32, 95, 1088, 1129, 1536, 1791, 2048, 2303}) {
+        MCRegister Reg = *MRI->getLLVMRegNum(DwarfEncoding, false);
+        EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, false));
+        EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, true));
       }
     }
   }
diff --git a/llvm/unittests/Target/AMDGPU/DwarfRegMappings.cpp b/llvm/unittests/Target/AMDGPU/DwarfRegMappings.cpp
index 56da4ce7b43af0..00c9593eafed07 100644
--- a/llvm/unittests/Target/AMDGPU/DwarfRegMappings.cpp
+++ b/llvm/unittests/Target/AMDGPU/DwarfRegMappings.cpp
@@ -25,11 +25,24 @@ TEST(AMDGPU, TestWave64DwarfRegMapping) {
         // PC_64 => 16, EXEC_MASK_64 => 17, S0 => 32, S63 => 95,
         // S64 => 1088, S105 => 1129, V0 => 2560, V255 => 2815,
         // A0 => 3072, A255 => 3327
-        for (int llvmReg :
+        for (int DwarfEncoding :
              {16, 17, 32, 95, 1088, 1129, 2560, 2815, 3072, 3327}) {
-          MCRegister PCReg(*MRI->getLLVMRegNum(llvmReg, false));
-          EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, false));
-          EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, true));
+          MCRegister Reg = *MRI->getLLVMRegNum(DwarfEncoding, false);
+          EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, false));
+          EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, true));
+        }
+
+        // We should get the correct LLVM register when round tripping through
+        // the dwarf encoding.
+        for (MCRegister LLReg : {AMDGPU::VGPR1, AMDGPU::AGPR2, AMDGPU::SGPR3}) {
+          int DwarfEncoding = MRI->getDwarfRegNum(LLReg, false);
+          EXPECT_EQ(LLReg, MRI->getLLVMRegNum(DwarfEncoding, false));
+        }
+
+        // Verify that subregisters have no dwarf encoding.
+        for (MCRegister LLSubReg :
+             {AMDGPU::VGPR1_LO16, AMDGPU::AGPR1_HI16, AMDGPU::SGPR1_HI16}) {
+          EXPECT_EQ(MRI->getDwarfRegNum(LLSubReg, false), -1);
         }
       }
     }
@@ -49,11 +62,24 @@ TEST(AMDGPU, TestWave32DwarfRegMapping) {
         // PC_64 => 16, EXEC_MASK_32 => 1, S0 => 32, S63 => 95,
         // S64 => 1088, S105 => 1129, V0 => 1536, V255 => 1791,
         // A0 => 2048, A255 => 2303
-        for (int llvmReg :
+        for (int DwarfEncoding :
              {16, 1, 32, 95, 1088, 1129, 1536, 1791, 2048, 2303}) {
-          MCRegister PCReg(*MRI->getLLVMRegNum(llvmReg, false));
-          EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, false));
-          EXPECT_EQ(llvmReg, MRI->getDwarfRegNum(PCReg, true));
+          MCRegister Reg = *MRI->getLLVMRegNum(DwarfEncoding, false);
+          EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, false));
+          EXPECT_EQ(DwarfEncoding, MRI->getDwarfRegNum(Reg, true));
+        }
+
+        // We should get the correct LLVM register when round tripping through
+        // the dwarf encoding.
+        for (MCRegister LLReg : {AMDGPU::VGPR1, AMDGPU::AGPR2, AMDGPU::SGPR3}) {
+          int DwarfEncoding = MRI->getDwarfRegNum(LLReg, false);
+          EXPECT_EQ(LLReg, MRI->getLLVMRegNum(DwarfEncoding, false));
+        }
+
+        // Verify that subregisters have no dwarf encoding.
+        for (MCRegister LLSubReg :
+             {AMDGPU::VGPR1_LO16, AMDGPU::AGPR1_HI16, AMDGPU::SGPR1_HI16}) {
+          EXPECT_EQ(MRI->getDwarfRegNum(LLSubReg, false), -1);
         }
       }
     }

@epilk epilk merged commit dc0e258 into llvm:main Jan 6, 2025
8 checks passed
@epilk epilk deleted the dwarf-subregs branch January 6, 2025 19:51
yxsamliu pushed a commit to yxsamliu/llvm-project that referenced this pull request Mar 1, 2025
Previously, registers and subregisters mapped to the same Dwarf
encoding. We don't really have any way to refer to subregisters directly
from Dwarf, the expression emitter should instead use DW_OPs to stencil
out the subregister from the whole register. This was also confusing
tools that need to map back to the llvm reg (e.g. dwarfdump), since
getLLVMRegNum() would arbitrarily return the _LO16 register.

This is a cherry-pick of github.com/llvm/pull/117891 with
test fixes.

Change-Id: I155bce592c7d556c01a7e3048bb8b251109dd51d
jrbyrnes pushed a commit to jrbyrnes/llvm-project that referenced this pull request Apr 29, 2025
Squashed commit, fixes SWDEV-463925.

This is a combination of 5 commits.
This is the 1st commit message:

[AMDGPU] Remove Dwarf encodings for subregisters

Previously, registers and subregisters mapped to the same Dwarf
encoding. We don't really have any way to refer to subregisters directly
from Dwarf, the expression emitter should instead use DW_OPs to stencil
out the subregister from the whole register. This was also confusing
tools that need to map back to the llvm reg (e.g. dwarfdump), since
getLLVMRegNum() would arbitrarily return the _LO16 register.

This is a cherry-pick of github.com/llvm/pull/117891 with
test fixes.

Change-Id: I155bce592c7d556c01a7e3048bb8b251109dd51d

This is the commit message #2:

[HeterogeneousDwarf] Support poisoned fragments in DIExpression

This improves fragment emission, since we previously had to assume that
a poisoned expression clobbered all other live variable fragments. Now,
it only clobbers the region described by it's fragment.

This commit also canonicalizes any DIExpression containing a poison to
only the ops DW_OP_LLVM_poisoned and (optionally) DW_OP_LLVM_fragment.
The other ops don't really serve any purpose since we can't rely on any
invariants (e.g. number of location ops) of a poisoned expression. This
also cleans up the dwarf output for posioned exprs.

Change-Id: I97bd9513a81b30f290ef70f958dcc8ca4c79e489

This is the commit message #3:

[HeterogeneousDwarf] Support Dwarf register emission of subregs/sequences

This commit adds support for emitting subregisters and register
sequences. This is needed for debugging -O1, since DIExpressions can now
refer to these registers.

Change-Id: Ic7b468a01855d3f8dc675dce4b2280625bf68574

This is the commit message #4:

[SelectionDAG] Add debug info salvaging for bitcast operations

This is needed to retain debug info for 64 bit kernel parameters.

This is the commit message #5:

[SROA] Fix DIOp-DIExpression fragment handling

Change-Id: If1f73e941c227d74ebe563c2857f5df2d60e9225

Change-Id: Id3914345ef7a434355f36e45d3ea0bf6c7ae29aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants