[PPC] Add DwarfRegAlias for VSRPair

[syzaara]

Add DwarfRegAlias for VSRPair as it shares dwarfRegNum with the VR registers.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

Author: Zaara Syeda (syzaara)

Changes

In getDwarfRegNum we currently only check the super-register chain until we hit a valid dwarf register number. Registers may also share dwarf register number with their subregs. For example with Power10, the VSRPair registers share dwarf register number with the VR subregs.

---

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

3 Files Affected:

• (modified) llvm/lib/CodeGen/StackMaps.cpp (+9-1)
• (modified) llvm/lib/Target/PowerPC/PPCRegisterInfo.td (+1-1)
• (modified) llvm/test/CodeGen/PowerPC/ppc64-anyregcc.ll (+1-1)

diff --git a/llvm/lib/CodeGen/StackMaps.cpp b/llvm/lib/CodeGen/StackMaps.cpp
index 90aa93e442cf3..1f33fd3c09994 100644
--- a/llvm/lib/CodeGen/StackMaps.cpp
+++ b/llvm/lib/CodeGen/StackMaps.cpp
@@ -191,7 +191,8 @@ unsigned StackMaps::getNextMetaArgIdx(const MachineInstr *MI, unsigned CurIdx) {
   return CurIdx;
 }
 
-/// Go up the super-register chain until we hit a valid dwarf register number.
+/// Go up the super-register and sub-register chain until we hit a valid dwarf
+/// register number.
 static unsigned getDwarfRegNum(unsigned Reg, const TargetRegisterInfo *TRI) {
   int RegNum;
   for (MCPhysReg SR : TRI->superregs_inclusive(Reg)) {
@@ -199,6 +200,13 @@ static unsigned getDwarfRegNum(unsigned Reg, const TargetRegisterInfo *TRI) {
     if (RegNum >= 0)
       break;
   }
+  if (RegNum < 0) {
+    for (MCPhysReg SR : TRI->subregs_inclusive(Reg)) {
+      RegNum = TRI->getDwarfRegNum(SR, false);
+      if (RegNum >= 0)
+        break;
+    }
+  }
 
   assert(RegNum >= 0 && isUInt<16>(RegNum) && "Invalid Dwarf register number.");
   return (unsigned)RegNum;
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfo.td b/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
index 8a37e40414eee..fdbdc14736c86 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
@@ -199,7 +199,7 @@ let SubRegIndices = [sub_vsx0, sub_vsx1] in {
     def VSRp#!add(!srl(Index, 1), 16) :
       VSRPair<!add(!srl(Index, 1), 16), "vsp"#!add(Index, 32),
               [!cast<VR>("V"#Index), !cast<VR>("V"#!add(Index, 1))]>,
-      DwarfRegNum<[-1, -1]>;
+              DwarfRegAlias<!cast<VR>("V"#Index)>;
   }
 }
 
diff --git a/llvm/test/CodeGen/PowerPC/ppc64-anyregcc.ll b/llvm/test/CodeGen/PowerPC/ppc64-anyregcc.ll
index 9b48fb72dc7cf..4bf222e898488 100644
--- a/llvm/test/CodeGen/PowerPC/ppc64-anyregcc.ll
+++ b/llvm/test/CodeGen/PowerPC/ppc64-anyregcc.ll
@@ -1,4 +1,4 @@
-; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mcpu=pwr10 -verify-machineinstrs < %s | FileCheck %s
 target datalayout = "E-m:e-i64:64-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 

[bzEq]

Is this change still necessary after using DwarfRegAlias? Looks we have similar register structure as ddffa0e.

[chenzheng1030]

I think for this issue, adding DwarfRegAlias should be enough to solve the missing DWARF number assertion for PPC pair vsr registers.

Maybe we can get a more general solution here. This is a static getDwarfRegNum() in StackMap.cpp. There is another implementation for getDwarfRegNum() in MCRegisterInfo.h.

• We should try to replace this static implementation with the one in MCRegisterInfo.h?
• Do we really need the DwarfRegAlias in td files? For the current implementation of getDwarfRegNum() in MCRegisterInfo.h, yes, we need DwarfRegAlias. But if we change its implementation to also check the sub/super registers, I don't think we still need them. The implementation seems make sense at least on PPC. For PPC registers, if registers overlap physically, they will share same DWARF register number. (For now, most usages of DwarfRegAlias are for super/sub registers, several usages should be able to replace with DwarfRegNum, or register Aliases). For example:

PPC:

// The representation of r0 when treated as the constant 0.
let isConstant = true in {
def ZERO  : GPR<0, "0">,    DwarfRegAlias<R0>; 
def ZERO8 : GP8<ZERO, "0">, DwarfRegAlias<X0>;
} // isConstant = true

Can be replaced with:

// The representation of r0 when treated as the constant 0.
let isConstant = true in {
def ZERO  : GPR<0, "0"> {
  let Aliases = [R0]
} 
def ZERO8 : GP8<ZERO, "0"> {
  let Aliases = [X0]
}
} // isConstant = true

AArch64:

let isConstant = true in
def WZR   : AArch64Reg<31, "wzr">, DwarfRegAlias<WSP>;  //can be replaced with DwarfRegNum, can it be replaced with register Aliases?

One thing is, the register with DWARF number in td files may not be the direct super/sub register of the register we want to get the DWARF number for.

@arsenm @efriedma-quic any thoughts for this proposal?

[arsenm]

Definitely should try to just use the MCRegisterInfo version instead of reconstructing it.

Do we really need the DwarfRegAlias in td files?

I don't know, but the rest of your comment implies to me it only works if you can't address all parts of the register. e.g. if you have sub0_sub1, a direct sub1 use should not have the same number as sub0_sub1 (but maybe sub0 could have the same as sub0_sub1)? I haven't really looked at subregisters in dwarf before

[chenzheng1030]

OK. Thanks for comment @arsenm

I am fine if we solve PPC's issue by just adding DwarfRegAlias in PPC td files. We can do the refactor for the two versions getDwarfRegNum() in another patch.

[chenzheng1030]

LGTM.

I created #96129 for the follow ups related to refactor of getDwarfRegNum()