[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109494

willghatch · 2024-09-20T23:33:24Z

The register encoding used by NVPTX and cuda-gdb basically use strings encoded as numbers. They are always within 64-bits, but typically outside of 32-bits, since they often need at least 5 characters.

This patch changes the signature of MCRegisterInfo::getDwarfRegNum and some related data structures to use 64-bit numbers to accommodate encodings like this.

Additionally, MCRegisterInfo::getDwarfRegNum is marked as virtual, so that targets with peculiar dwarf register mapping schemes (such as NVPTX) can override its behavior.

I originally tried to do a broader switch to 64-bit types for registers, but it caused many problems. There are various places in code generation where registers are not just treated as 32-bit numbers, but also treat certain bit offsets as flags. So I limited the change as much as possible to just the output of getDwarfRegNum. Keeping the types used by DwarfLLVMRegPair as unsigned preserves the current behaviors. The only way to give a 64-bit output from getDwarfRegNum that actually needs more than 32-bits is to override getDwarfRegNum and provide an implementation that sidesteps the use of the DwarfLLVMRegPair maps defined in tablegen files.

First layer of stack supporting: #109495

llvmbot · 2024-09-20T23:34:02Z

@llvm/pr-subscribers-debuginfo

Author: William G Hatch (willghatch)

Changes

The register encoding used by NVPTX and cuda-gdb basically use strings encoded as numbers. They are always within 64-bits, but typically outside of 32-bits, since they often need at least 5 characters.

This patch changes the signature of MCRegisterInfo::getDwarfRegNum and some related data structures to use 64-bit numbers to accommodate encodings like this.

Additionally, the MCRegisterInfo::getDwarfRegNum is marked as virtual, so that targets with peculiar dwarf register mapping schemes (such as NVPTX) can override its behavior.

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

6 Files Affected:

(modified) llvm/include/llvm/MC/MCRegisterInfo.h (+3-3)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+2-2)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (+4-4)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h (+6-6)
(modified) llvm/lib/MC/MCRegisterInfo.cpp (+10-6)
(modified) llvm/lib/Target/Lanai/LanaiRegisterInfo.h (-2)

diff --git a/llvm/include/llvm/MC/MCRegisterInfo.h b/llvm/include/llvm/MC/MCRegisterInfo.h
index a617ddecd38a2b..8a6f9fce97e30c 100644
--- a/llvm/include/llvm/MC/MCRegisterInfo.h
+++ b/llvm/include/llvm/MC/MCRegisterInfo.h
@@ -418,15 +418,15 @@ class MCRegisterInfo {
   /// number.  Returns -1 if there is no equivalent value.  The second
   /// parameter allows targets to use different numberings for EH info and
   /// debugging info.
-  int getDwarfRegNum(MCRegister RegNum, bool isEH) const;
+  virtual int64_t getDwarfRegNum(MCRegister RegNum, bool isEH) const;
 
   /// Map a dwarf register back to a target register. Returns std::nullopt if
   /// there is no mapping.
-  std::optional<MCRegister> getLLVMRegNum(unsigned RegNum, bool isEH) const;
+  std::optional<MCRegister> getLLVMRegNum(uint64_t RegNum, bool isEH) const;
 
   /// Map a target EH register number to an equivalent DWARF register
   /// number.
-  int getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const;
+  int64_t getDwarfRegNumFromDwarfEHRegNum(uint64_t RegNum) const;
 
   /// Map a target register to an equivalent SEH register
   /// number.  Returns LLVM register number if there is no equivalent value.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index e9649f9ff81658..8ab0b9c9253700 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -570,7 +570,7 @@ void DwarfDebug::constructAbstractSubprogramScopeDIE(DwarfCompileUnit &SrcCU,
 /// debug expression to a register in the forwarded register worklist.
 struct FwdRegParamInfo {
   /// The described parameter register.
-  unsigned ParamReg;
+  uint64_t ParamReg;
 
   /// Debug expression that has been built up when walking through the
   /// instruction chain that produces the parameter's value.
@@ -578,7 +578,7 @@ struct FwdRegParamInfo {
 };
 
 /// Register worklist for finding call site values.
-using FwdRegWorklist = MapVector<unsigned, SmallVector<FwdRegParamInfo, 2>>;
+using FwdRegWorklist = MapVector<uint64_t, SmallVector<FwdRegParamInfo, 2>>;
 /// Container for the set of registers known to be clobbered on the path to a
 /// call site.
 using ClobberedRegSet = SmallSet<Register, 16>;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
index 9d6e1bb367bc85..08c762485b6527 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -40,7 +40,7 @@ void DwarfExpression::emitConstu(uint64_t Value) {
   }
 }
 
-void DwarfExpression::addReg(int DwarfReg, const char *Comment) {
+void DwarfExpression::addReg(int64_t DwarfReg, const char *Comment) {
   assert(DwarfReg >= 0 && "invalid negative dwarf register number");
   assert((isUnknownLocation() || isRegisterLocation()) &&
          "location description already locked down");
@@ -53,7 +53,7 @@ void DwarfExpression::addReg(int DwarfReg, const char *Comment) {
   }
 }
 
-void DwarfExpression::addBReg(int DwarfReg, int Offset) {
+void DwarfExpression::addBReg(int64_t DwarfReg, int64_t Offset) {
   assert(DwarfReg >= 0 && "invalid negative dwarf register number");
   assert(!isRegisterLocation() && "location description already locked down");
   if (DwarfReg < 32) {
@@ -65,7 +65,7 @@ void DwarfExpression::addBReg(int DwarfReg, int Offset) {
   emitSigned(Offset);
 }
 
-void DwarfExpression::addFBReg(int Offset) {
+void DwarfExpression::addFBReg(int64_t Offset) {
   emitOp(dwarf::DW_OP_fbreg);
   emitSigned(Offset);
 }
@@ -108,7 +108,7 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
     return false;
   }
 
-  int Reg = TRI.getDwarfRegNum(MachineReg, false);
+  int64_t Reg = TRI.getDwarfRegNum(MachineReg, false);
 
   // If this is a valid register number, emit it.
   if (Reg >= 0) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
index 4daa78b15b8e29..06809ab2638754 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -45,17 +45,17 @@ class DwarfExpression {
 protected:
   /// Holds information about all subregisters comprising a register location.
   struct Register {
-    int DwarfRegNo;
+    int64_t DwarfRegNo;
     unsigned SubRegSize;
     const char *Comment;
 
     /// Create a full register, no extra DW_OP_piece operators necessary.
-    static Register createRegister(int RegNo, const char *Comment) {
+    static Register createRegister(int64_t RegNo, const char *Comment) {
       return {RegNo, 0, Comment};
     }
 
     /// Create a subregister that needs a DW_OP_piece operator with SizeInBits.
-    static Register createSubRegister(int RegNo, unsigned SizeInBits,
+    static Register createSubRegister(int64_t RegNo, unsigned SizeInBits,
                                       const char *Comment) {
       return {RegNo, SizeInBits, Comment};
     }
@@ -161,13 +161,13 @@ class DwarfExpression {
 
   /// Emit a DW_OP_reg operation. Note that this is only legal inside a DWARF
   /// register location description.
-  void addReg(int DwarfReg, const char *Comment = nullptr);
+  void addReg(int64_t DwarfReg, const char *Comment = nullptr);
 
   /// Emit a DW_OP_breg operation.
-  void addBReg(int DwarfReg, int Offset);
+  void addBReg(int64_t DwarfReg, int64_t Offset);
 
   /// Emit DW_OP_fbreg <Offset>.
-  void addFBReg(int Offset);
+  void addFBReg(int64_t Offset);
 
   /// Emit a partial DWARF register operation.
   ///
diff --git a/llvm/lib/MC/MCRegisterInfo.cpp b/llvm/lib/MC/MCRegisterInfo.cpp
index a5de02abce667e..7d269308b02b5e 100644
--- a/llvm/lib/MC/MCRegisterInfo.cpp
+++ b/llvm/lib/MC/MCRegisterInfo.cpp
@@ -141,7 +141,7 @@ unsigned MCRegisterInfo::getSubRegIndex(MCRegister Reg,
   return 0;
 }
 
-int MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
+int64_t MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHL2DwarfRegs : L2DwarfRegs;
   unsigned Size = isEH ? EHL2DwarfRegsSize : L2DwarfRegsSize;
 
@@ -151,24 +151,28 @@ int MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
   if (I == M+Size || I->FromReg != RegNum)
     return -1;
-  return I->ToReg;
+  // Consumers need to be able to detect -1 and -2, but at various points
+  // the numbers move between unsigned and signed representations, as well as
+  // between 32- and 64-bit representations. We need to convert first to int
+  // before int64_t for proper sign handling.
+  return int64_t(int(I->ToReg));
 }
 
-std::optional<MCRegister> MCRegisterInfo::getLLVMRegNum(unsigned RegNum,
-                                                        bool isEH) const {
+std::optional<MCRegister> MCRegisterInfo::getLLVMRegNum(uint64_t RegNum,
+                                                      bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHDwarf2LRegs : Dwarf2LRegs;
   unsigned Size = isEH ? EHDwarf2LRegsSize : Dwarf2LRegsSize;
 
   if (!M)
     return std::nullopt;
-  DwarfLLVMRegPair Key = { RegNum, 0 };
+  DwarfLLVMRegPair Key = { unsigned(RegNum), 0 };
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
   if (I != M + Size && I->FromReg == RegNum)
     return MCRegister::from(I->ToReg);
   return std::nullopt;
 }
 
-int MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const {
+int64_t MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(uint64_t RegNum) const {
   // On ELF platforms, DWARF EH register numbers are the same as DWARF
   // other register numbers.  On Darwin x86, they differ and so need to be
   // mapped.  The .cfi_* directives accept integer literals as well as
diff --git a/llvm/lib/Target/Lanai/LanaiRegisterInfo.h b/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
index 5168dddd93019d..4ff74c5f4eb1e3 100644
--- a/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
+++ b/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
@@ -43,8 +43,6 @@ struct LanaiRegisterInfo : public LanaiGenRegisterInfo {
   Register getFrameRegister(const MachineFunction &MF) const override;
   Register getBaseRegister() const;
   bool hasBasePointer(const MachineFunction &MF) const;
-
-  int getDwarfRegNum(unsigned RegNum, bool IsEH) const;
 };
 
 } // end namespace llvm

llvmbot · 2024-09-20T23:34:03Z

@llvm/pr-subscribers-mc

Author: William G Hatch (willghatch)

Changes

The register encoding used by NVPTX and cuda-gdb basically use strings encoded as numbers. They are always within 64-bits, but typically outside of 32-bits, since they often need at least 5 characters.

This patch changes the signature of MCRegisterInfo::getDwarfRegNum and some related data structures to use 64-bit numbers to accommodate encodings like this.

Additionally, the MCRegisterInfo::getDwarfRegNum is marked as virtual, so that targets with peculiar dwarf register mapping schemes (such as NVPTX) can override its behavior.

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

6 Files Affected:

(modified) llvm/include/llvm/MC/MCRegisterInfo.h (+3-3)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+2-2)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (+4-4)
(modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h (+6-6)
(modified) llvm/lib/MC/MCRegisterInfo.cpp (+10-6)
(modified) llvm/lib/Target/Lanai/LanaiRegisterInfo.h (-2)

diff --git a/llvm/include/llvm/MC/MCRegisterInfo.h b/llvm/include/llvm/MC/MCRegisterInfo.h
index a617ddecd38a2b..8a6f9fce97e30c 100644
--- a/llvm/include/llvm/MC/MCRegisterInfo.h
+++ b/llvm/include/llvm/MC/MCRegisterInfo.h
@@ -418,15 +418,15 @@ class MCRegisterInfo {
   /// number.  Returns -1 if there is no equivalent value.  The second
   /// parameter allows targets to use different numberings for EH info and
   /// debugging info.
-  int getDwarfRegNum(MCRegister RegNum, bool isEH) const;
+  virtual int64_t getDwarfRegNum(MCRegister RegNum, bool isEH) const;
 
   /// Map a dwarf register back to a target register. Returns std::nullopt if
   /// there is no mapping.
-  std::optional<MCRegister> getLLVMRegNum(unsigned RegNum, bool isEH) const;
+  std::optional<MCRegister> getLLVMRegNum(uint64_t RegNum, bool isEH) const;
 
   /// Map a target EH register number to an equivalent DWARF register
   /// number.
-  int getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const;
+  int64_t getDwarfRegNumFromDwarfEHRegNum(uint64_t RegNum) const;
 
   /// Map a target register to an equivalent SEH register
   /// number.  Returns LLVM register number if there is no equivalent value.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index e9649f9ff81658..8ab0b9c9253700 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -570,7 +570,7 @@ void DwarfDebug::constructAbstractSubprogramScopeDIE(DwarfCompileUnit &SrcCU,
 /// debug expression to a register in the forwarded register worklist.
 struct FwdRegParamInfo {
   /// The described parameter register.
-  unsigned ParamReg;
+  uint64_t ParamReg;
 
   /// Debug expression that has been built up when walking through the
   /// instruction chain that produces the parameter's value.
@@ -578,7 +578,7 @@ struct FwdRegParamInfo {
 };
 
 /// Register worklist for finding call site values.
-using FwdRegWorklist = MapVector<unsigned, SmallVector<FwdRegParamInfo, 2>>;
+using FwdRegWorklist = MapVector<uint64_t, SmallVector<FwdRegParamInfo, 2>>;
 /// Container for the set of registers known to be clobbered on the path to a
 /// call site.
 using ClobberedRegSet = SmallSet<Register, 16>;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
index 9d6e1bb367bc85..08c762485b6527 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -40,7 +40,7 @@ void DwarfExpression::emitConstu(uint64_t Value) {
   }
 }
 
-void DwarfExpression::addReg(int DwarfReg, const char *Comment) {
+void DwarfExpression::addReg(int64_t DwarfReg, const char *Comment) {
   assert(DwarfReg >= 0 && "invalid negative dwarf register number");
   assert((isUnknownLocation() || isRegisterLocation()) &&
          "location description already locked down");
@@ -53,7 +53,7 @@ void DwarfExpression::addReg(int DwarfReg, const char *Comment) {
   }
 }
 
-void DwarfExpression::addBReg(int DwarfReg, int Offset) {
+void DwarfExpression::addBReg(int64_t DwarfReg, int64_t Offset) {
   assert(DwarfReg >= 0 && "invalid negative dwarf register number");
   assert(!isRegisterLocation() && "location description already locked down");
   if (DwarfReg < 32) {
@@ -65,7 +65,7 @@ void DwarfExpression::addBReg(int DwarfReg, int Offset) {
   emitSigned(Offset);
 }
 
-void DwarfExpression::addFBReg(int Offset) {
+void DwarfExpression::addFBReg(int64_t Offset) {
   emitOp(dwarf::DW_OP_fbreg);
   emitSigned(Offset);
 }
@@ -108,7 +108,7 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
     return false;
   }
 
-  int Reg = TRI.getDwarfRegNum(MachineReg, false);
+  int64_t Reg = TRI.getDwarfRegNum(MachineReg, false);
 
   // If this is a valid register number, emit it.
   if (Reg >= 0) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
index 4daa78b15b8e29..06809ab2638754 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -45,17 +45,17 @@ class DwarfExpression {
 protected:
   /// Holds information about all subregisters comprising a register location.
   struct Register {
-    int DwarfRegNo;
+    int64_t DwarfRegNo;
     unsigned SubRegSize;
     const char *Comment;
 
     /// Create a full register, no extra DW_OP_piece operators necessary.
-    static Register createRegister(int RegNo, const char *Comment) {
+    static Register createRegister(int64_t RegNo, const char *Comment) {
       return {RegNo, 0, Comment};
     }
 
     /// Create a subregister that needs a DW_OP_piece operator with SizeInBits.
-    static Register createSubRegister(int RegNo, unsigned SizeInBits,
+    static Register createSubRegister(int64_t RegNo, unsigned SizeInBits,
                                       const char *Comment) {
       return {RegNo, SizeInBits, Comment};
     }
@@ -161,13 +161,13 @@ class DwarfExpression {
 
   /// Emit a DW_OP_reg operation. Note that this is only legal inside a DWARF
   /// register location description.
-  void addReg(int DwarfReg, const char *Comment = nullptr);
+  void addReg(int64_t DwarfReg, const char *Comment = nullptr);
 
   /// Emit a DW_OP_breg operation.
-  void addBReg(int DwarfReg, int Offset);
+  void addBReg(int64_t DwarfReg, int64_t Offset);
 
   /// Emit DW_OP_fbreg <Offset>.
-  void addFBReg(int Offset);
+  void addFBReg(int64_t Offset);
 
   /// Emit a partial DWARF register operation.
   ///
diff --git a/llvm/lib/MC/MCRegisterInfo.cpp b/llvm/lib/MC/MCRegisterInfo.cpp
index a5de02abce667e..7d269308b02b5e 100644
--- a/llvm/lib/MC/MCRegisterInfo.cpp
+++ b/llvm/lib/MC/MCRegisterInfo.cpp
@@ -141,7 +141,7 @@ unsigned MCRegisterInfo::getSubRegIndex(MCRegister Reg,
   return 0;
 }
 
-int MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
+int64_t MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHL2DwarfRegs : L2DwarfRegs;
   unsigned Size = isEH ? EHL2DwarfRegsSize : L2DwarfRegsSize;
 
@@ -151,24 +151,28 @@ int MCRegisterInfo::getDwarfRegNum(MCRegister RegNum, bool isEH) const {
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
   if (I == M+Size || I->FromReg != RegNum)
     return -1;
-  return I->ToReg;
+  // Consumers need to be able to detect -1 and -2, but at various points
+  // the numbers move between unsigned and signed representations, as well as
+  // between 32- and 64-bit representations. We need to convert first to int
+  // before int64_t for proper sign handling.
+  return int64_t(int(I->ToReg));
 }
 
-std::optional<MCRegister> MCRegisterInfo::getLLVMRegNum(unsigned RegNum,
-                                                        bool isEH) const {
+std::optional<MCRegister> MCRegisterInfo::getLLVMRegNum(uint64_t RegNum,
+                                                      bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHDwarf2LRegs : Dwarf2LRegs;
   unsigned Size = isEH ? EHDwarf2LRegsSize : Dwarf2LRegsSize;
 
   if (!M)
     return std::nullopt;
-  DwarfLLVMRegPair Key = { RegNum, 0 };
+  DwarfLLVMRegPair Key = { unsigned(RegNum), 0 };
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
   if (I != M + Size && I->FromReg == RegNum)
     return MCRegister::from(I->ToReg);
   return std::nullopt;
 }
 
-int MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const {
+int64_t MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(uint64_t RegNum) const {
   // On ELF platforms, DWARF EH register numbers are the same as DWARF
   // other register numbers.  On Darwin x86, they differ and so need to be
   // mapped.  The .cfi_* directives accept integer literals as well as
diff --git a/llvm/lib/Target/Lanai/LanaiRegisterInfo.h b/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
index 5168dddd93019d..4ff74c5f4eb1e3 100644
--- a/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
+++ b/llvm/lib/Target/Lanai/LanaiRegisterInfo.h
@@ -43,8 +43,6 @@ struct LanaiRegisterInfo : public LanaiGenRegisterInfo {
   Register getFrameRegister(const MachineFunction &MF) const override;
   Register getBaseRegister() const;
   bool hasBasePointer(const MachineFunction &MF) const;
-
-  int getDwarfRegNum(unsigned RegNum, bool IsEH) const;
 };
 
 } // end namespace llvm

github-actions · 2024-09-20T23:36:48Z

✅ With the latest revision this PR passed the C/C++ code formatter.

The register encoding used by NVPTX and cuda-gdb basically use strings encoded as numbers. They are always within 64-bits, but typically outside of 32-bits, since they often need at least 5 characters. This patch changes the signature of MCRegisterInfo::getDwarfRegNum and some related data structures to use 64-bit numbers to accommodate encodings like this. Additionally, the MCRegisterInfo::getDwarfRegNum is marked as virtual, so that targets with peculiar dwarf register mapping schemes (such as NVPTX) can override its behavior.

walter-erquinigo · 2024-09-22T20:20:13Z

llvm/lib/MC/MCRegisterInfo.cpp

+  // the numbers move between unsigned and signed representations, as well as
+  // between 32- and 64-bit representations. We need to convert first to int
+  // before int64_t for proper sign handling.
+  return int64_t(int(I->ToReg));


what about doing the following:

auto reg = I->ToReg; switch (reg) { case -1: return -1; case -2: return -2; default: return reg; }

That might be good. I'm not certain there are not other negative values currently being used as signal values, but if that switch passes the tests then it's probably fine. I'm not certain that's necessarily better, though. But I'm fine with either implementation.

Then, what about int64_t(int32_t(I->ToReg))?

All of the existing code dealing with these register numbers uses the types int and unsigned. The int64_t(int(I->ToReg)) conversion seems more consistent with other usage than int64_t(int32_t(I->ToReg)).

walter-erquinigo · 2024-09-22T20:20:31Z

llvm/lib/Target/Lanai/LanaiRegisterInfo.h

-
-  int getDwarfRegNum(unsigned RegNum, bool IsEH) const;


why did you remove this?

It caused errors when I switched getDwarfRegNum to be virtual, and there didn't seem to be a reason for it to be there. No tests failed from removing it. I believe it is vestigial, but I'm open to someone telling or showing me why it should be there.

No worries. What you say makes sense.

walter-erquinigo · 2024-09-22T20:21:12Z

llvm/lib/MC/MCRegisterInfo.cpp

                                                        bool isEH) const {
  const DwarfLLVMRegPair *M = isEH ? EHDwarf2LRegs : Dwarf2LRegs;
  unsigned Size = isEH ? EHDwarf2LRegsSize : Dwarf2LRegsSize;

  if (!M)
    return std::nullopt;
-  DwarfLLVMRegPair Key = { RegNum, 0 };
+  DwarfLLVMRegPair Key = {unsigned(RegNum), 0};


wouldn't it be better to have DwarfLLVMRegPair support int64_t? Or is there something that makes you sure that the current conversion is safe?

I originally tried to do a broader switch to 64-bit types for registers, but it caused many problems. There are various places in code generation where registers are not just treated as 32-bit numbers, but also treat certain bit offsets as flags. So I limited the change as much as possible to just the output of getDwarfRegNum. Keeping the types used by DwarfLLVMRegPair as unsigned preserves the current behaviors. The only way to give a 64-bit output from getDwarfRegNum that actually needs more than 32-bits is to override getDwarfRegNum and provide an implementation that sidesteps these maps. These maps are generated from tablegen register definitions, and don't make sense for an implementation that needs to use a weird encoding that requires 64 bits anyway.

Oof. Got it. Please mention this in the commit description for posterity.

This patch adds support for encoding PTX registers for DWARF, using the encoding supported by nvcc and cuda-gcc. There are some other features still needed for proper register debugging that this patch does not address, such as DW_AT_address_class. This PR is stacked on: #109494

The register encoding used by NVPTX and cuda-gdb basically use strings encoded as numbers. They are always within 64-bits, but typically outside of 32-bits, since they often need at least 5 characters. This patch changes the signature of `MCRegisterInfo::getDwarfRegNum` and some related data structures to use 64-bit numbers to accommodate encodings like this. Additionally, `MCRegisterInfo::getDwarfRegNum` is marked as virtual, so that targets with peculiar dwarf register mapping schemes (such as NVPTX) can override its behavior. I originally tried to do a broader switch to 64-bit types for registers, but it caused many problems. There are various places in code generation where registers are not just treated as 32-bit numbers, but also treat certain bit offsets as flags. So I limited the change as much as possible to just the output of `getDwarfRegNum`. Keeping the types used by `DwarfLLVMRegPair` as unsigned preserves the current behaviors. The only way to give a 64-bit output from `getDwarfRegNum` that actually needs more than 32-bits is to override `getDwarfRegNum` and provide an implementation that sidesteps the use of the `DwarfLLVMRegPair` maps defined in tablegen files. First layer of stack supporting: llvm#109495

This patch adds support for encoding PTX registers for DWARF, using the encoding supported by nvcc and cuda-gcc. There are some other features still needed for proper register debugging that this patch does not address, such as DW_AT_address_class. This PR is stacked on: llvm#109494

This patch adds support for encoding PTX registers for DWARF, using the encoding supported by nvcc and cuda-gcc. There are some other features still needed for proper register debugging that this patch does not address, such as DW_AT_address_class. This PR is stacked on: llvm/llvm-project#109494

llvmbot added debuginfo mc Machine (object) code labels Sep 20, 2024

This was referenced Sep 20, 2024

[NVPTX] add support for encoding PTX registers for DWARF #109495

Merged

[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109492

Closed

willghatch force-pushed the users/willghatch/width-changes-for-ptx-encoding branch from c8fbd58 to 9c5d67b Compare September 20, 2024 23:40

willghatch requested review from walter-erquinigo and zyx-billy September 20, 2024 23:43

walter-erquinigo requested a review from adrian-prantl September 22, 2024 16:46

walter-erquinigo reviewed Sep 22, 2024

View reviewed changes

walter-erquinigo approved these changes Sep 26, 2024

View reviewed changes

willghatch merged commit 4822e9d into main Sep 26, 2024
8 checks passed

willghatch deleted the users/willghatch/width-changes-for-ptx-encoding branch September 26, 2024 18:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109494

[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109494

Uh oh!

willghatch commented Sep 20, 2024 •

edited

Loading

Uh oh!

llvmbot commented Sep 20, 2024

Uh oh!

llvmbot commented Sep 20, 2024

Uh oh!

github-actions bot commented Sep 20, 2024 •

edited

Loading

Uh oh!

walter-erquinigo Sep 22, 2024

Uh oh!

willghatch Sep 23, 2024 •

edited

Loading

Uh oh!

walter-erquinigo Sep 24, 2024

Uh oh!

willghatch Sep 26, 2024

Uh oh!

walter-erquinigo Sep 22, 2024

Uh oh!

willghatch Sep 23, 2024

Uh oh!

walter-erquinigo Sep 24, 2024

Uh oh!

walter-erquinigo Sep 22, 2024

Uh oh!

willghatch Sep 23, 2024

Uh oh!

walter-erquinigo Sep 24, 2024

Uh oh!

Uh oh!

Uh oh!

[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109494

[llvm] use 64-bit types for result of getDwarfRegNum (NFC) #109494

Uh oh!

Conversation

willghatch commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 20, 2024

Uh oh!

llvmbot commented Sep 20, 2024

Uh oh!

github-actions bot commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willghatch Sep 23, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

willghatch commented Sep 20, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited

Loading

willghatch Sep 23, 2024 •

edited

Loading