MCRegisterInfo: Merge getLLVMRegNum and getLLVMRegNumFromEH

[labath]

Summary

The functions different in two ways:

• getLLVMRegNum could return both "eh" and "other" dwarf register numbers, while getLLVMRegNumFromEH only returned the "eh" number.
• getLLVMRegNum asserted if the register was not found, while the second function returned -1.

The second distinction was pretty important, but it was very hard to
infer that from the function name. Aditionally, for the use case of
dumping dwarf expressions, we needed a function which can work with both
kinds of number, but does not assert.

This patch solves both of these issues by merging the two functions into
one, returning an Optional<unsigned> value. While the same thing could
be achieved by adding an "IsEH" argument to the (renamed)
getLLVMRegNumFromEH function, it seemed better to avoid the confusion of
two functions and put the choice of asserting into the hands of the
caller -- if he checks the Optional value, he can safely process
"untrusted" input, and if he blindly dereferences the Optional, he gets
the assertion.

I've updated all call sites to the new API, choosing between the two
options according to the function they were calling originally, except
that I've updated the usage in DWARFExpression.cpp to use the "safe"
method instead, and added a test case which would have previously
triggered an assertion failure when processing (incorrect?) dwarf
expressions.

[JDevlieghere]

This makes sense to me.

llvm/trunk/include/llvm/MC/MCRegisterInfo.h

public:
   }
 
   /// Map a target register to an equivalent dwarf register
   /// 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;
 
-  /// Map a dwarf register back to a target register.
-  int getLLVMRegNum(unsigned RegNum, bool isEH) const;
-
-  /// Map a DWARF EH register back to a target register (same as
-  /// getLLVMRegNum(RegNum, true)) but return -1 if there is no mapping,
-  /// rather than asserting that there must be one.
-  int getLLVMRegNumFromEH(unsigned RegNum) const;
+  /// Map a dwarf register back to a target register. Returns None is there is
+  /// no mapping.
+  Optional<unsigned> getLLVMRegNum(unsigned RegNum, bool isEH) const;
 
   /// Map a target EH register number to an equivalent DWARF register
   /// number.
   int getDwarfRegNumFromDwarfEHRegNum(unsigned RegNum) const;
 
   /// Map a target register to an equivalent SEH register
   /// number.  Returns LLVM register number if there is no equivalent value.
   int getSEHRegNum(MCRegister RegNum) const;

llvm/trunk/lib/CodeGen/MachineOperand.cpp

 
 static void printCFIRegister(unsigned DwarfReg, raw_ostream &OS,
                              const TargetRegisterInfo *TRI) {
   if (!TRI) {
     OS << "%dwarfreg." << DwarfReg;
     return;
   }
 
-  int Reg = TRI->getLLVMRegNum(DwarfReg, true);
-  if (Reg == -1) {
+  if (Optional<unsigned> Reg = TRI->getLLVMRegNum(DwarfReg, true))
+    OS << printReg(*Reg, TRI);
+  else
     OS << "<badreg>";
-    return;
-  }
-  OS << printReg(Reg, TRI);
 }
 
 static void printIRBlockReference(raw_ostream &OS, const BasicBlock &BB,
                                   ModuleSlotTracker &MST) {
   OS << "%ir-block.";
   if (BB.hasName()) {
     printLLVMNameWithoutPrefix(OS, BB.getName());
     return;

llvm/trunk/lib/CodeGen/StackMaps.cpp

if (MOI->isReg()) {
 
     assert(Register::isPhysicalRegister(MOI->getReg()) &&
            "Virtreg operands should have been rewritten before now.");
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(MOI->getReg());
     assert(!MOI->getSubReg() && "Physical subreg still around.");
 
     unsigned Offset = 0;
     unsigned DwarfRegNum = getDwarfRegNum(MOI->getReg(), TRI);
-    unsigned LLVMRegNum = TRI->getLLVMRegNum(DwarfRegNum, false);
+    unsigned LLVMRegNum = *TRI->getLLVMRegNum(DwarfRegNum, false);
     unsigned SubRegIdx = TRI->getSubRegIndex(LLVMRegNum, MOI->getReg());
     if (SubRegIdx)
       Offset = TRI->getSubRegIdxOffset(SubRegIdx);
 
     Locs.emplace_back(Location::Register, TRI->getSpillSize(*RC),
                       DwarfRegNum, Offset);
     return ++MOI;
   }

llvm/trunk/lib/DebugInfo/DWARF/DWARFExpression.cpp

static bool prettyPrintRegisterOp(raw_ostream &OS, uint8_t Opcode,
 
   if (Opcode == DW_OP_bregx || Opcode == DW_OP_regx)
     DwarfRegNum = Operands[OpNum++];
   else if (Opcode >= DW_OP_breg0 && Opcode < DW_OP_bregx)
     DwarfRegNum = Opcode - DW_OP_breg0;
   else
     DwarfRegNum = Opcode - DW_OP_reg0;
 
-  int LLVMRegNum = MRI->getLLVMRegNum(DwarfRegNum, isEH);
-  if (LLVMRegNum >= 0) {
-    if (const char *RegName = MRI->getName(LLVMRegNum)) {
+  if (Optional<unsigned> LLVMRegNum = MRI->getLLVMRegNum(DwarfRegNum, isEH)) {
+    if (const char *RegName = MRI->getName(*LLVMRegNum)) {
       if ((Opcode >= DW_OP_breg0 && Opcode <= DW_OP_breg31) ||
           Opcode == DW_OP_bregx)
         OS << format(" %s%+" PRId64, RegName, Operands[OpNum]);
       else
         OS << ' ' << RegName;
       return true;
     }
   }

llvm/trunk/lib/MC/MCAsmStreamer.cpp

 }
 
 void MCAsmStreamer::EmitRegisterName(int64_t Register) {
   if (!MAI->useDwarfRegNumForCFI()) {
     // User .cfi_* directives can use arbitrary DWARF register numbers, not
     // just ones that map to LLVM register numbers and have known names.
     // Fall back to using the original number directly if no name is known.
     const MCRegisterInfo *MRI = getContext().getRegisterInfo();
-    int LLVMRegister = MRI->getLLVMRegNumFromEH(Register);
-    if (LLVMRegister != -1) {
-      InstPrinter->printRegName(OS, LLVMRegister);
+    if (Optional<unsigned> LLVMRegister = MRI->getLLVMRegNum(Register, true)) {
+      InstPrinter->printRegName(OS, *LLVMRegister);
       return;
     }
   }
   OS << Register;
 }
 
 void MCAsmStreamer::EmitCFIDefCfa(int64_t Register, int64_t Offset) {
   MCStreamer::EmitCFIDefCfa(Register, Offset);

llvm/trunk/lib/MC/MCRegisterInfo.cpp

if (!M)
     return -1;
   DwarfLLVMRegPair Key = { RegNum, 0 };
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
   if (I == M+Size || I->FromReg != RegNum)
     return -1;
   return I->ToReg;
 }
 
-int MCRegisterInfo::getLLVMRegNum(unsigned RegNum, bool isEH) const {
+Optional<unsigned> MCRegisterInfo::getLLVMRegNum(unsigned RegNum,
+                                                 bool isEH) const {
   const DwarfLLVMRegPair *M = isEH ? EHDwarf2LRegs : Dwarf2LRegs;
   unsigned Size = isEH ? EHDwarf2LRegsSize : Dwarf2LRegsSize;
 
   if (!M)
-    return -1;
+    return None;
   DwarfLLVMRegPair Key = { RegNum, 0 };
   const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
-  assert(I != M+Size && I->FromReg == RegNum && "Invalid RegNum");
-  return I->ToReg;
-}
-
-int MCRegisterInfo::getLLVMRegNumFromEH(unsigned RegNum) const {
-  const DwarfLLVMRegPair *M = EHDwarf2LRegs;
-  unsigned Size = EHDwarf2LRegsSize;
-
-  if (!M)
-    return -1;
-  DwarfLLVMRegPair Key = { RegNum, 0 };
-  const DwarfLLVMRegPair *I = std::lower_bound(M, M+Size, Key);
-  if (I == M+Size || I->FromReg != RegNum)
-    return -1;
+  if (I != M + Size && I->FromReg == RegNum)
     return I->ToReg;
+  return None;
 }
 
 int MCRegisterInfo::getDwarfRegNumFromDwarfEHRegNum(unsigned 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
   // register names and should generate exactly what the assembly code
   // asked for, so there might be DWARF/EH register numbers that don't have
   // a corresponding LLVM register number at all.  So if we can't map the
   // EH register number to an LLVM register number, assume it's just a
   // valid DWARF register number as is.
-  int LRegNum = getLLVMRegNumFromEH(RegNum);
-  if (LRegNum != -1)
-    return getDwarfRegNum(LRegNum, false);
+  if (Optional<unsigned> LRegNum = getLLVMRegNum(RegNum, true))
+    return getDwarfRegNum(*LRegNum, false);
   return RegNum;
 }
 
 int MCRegisterInfo::getSEHRegNum(MCRegister RegNum) const {
   const DenseMap<MCRegister, int>::const_iterator I = L2SEHRegs.find(RegNum);
   if (I == L2SEHRegs.end()) return (int)RegNum;
   return I->second;
 }

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

for (size_t i = 0, e = Instrs.size(); i != e; ++i) {
 
       switch (Inst.getOperation()) {
       default:
         // Cannot handle this directive:  bail out.
         return CU::UNWIND_ARM64_MODE_DWARF;
       case MCCFIInstruction::OpDefCfa: {
         // Defines a frame pointer.
         unsigned XReg =
-            getXRegFromWReg(MRI.getLLVMRegNum(Inst.getRegister(), true));
+            getXRegFromWReg(*MRI.getLLVMRegNum(Inst.getRegister(), true));
 
         // Other CFA registers than FP are not supported by compact unwind.
         // Fallback on DWARF.
         // FIXME: When opt-remarks are supported in MC, add a remark to notify
         // the user.
         if (XReg != AArch64::FP)
           return CU::UNWIND_ARM64_MODE_DWARF;
 
         assert(XReg == AArch64::FP && "Invalid frame pointer!");
         assert(i + 2 < e && "Insufficient CFI instructions to define a frame!");
 
         const MCCFIInstruction &LRPush = Instrs[++i];
         assert(LRPush.getOperation() == MCCFIInstruction::OpOffset &&
                "Link register not pushed!");
         const MCCFIInstruction &FPPush = Instrs[++i];
         assert(FPPush.getOperation() == MCCFIInstruction::OpOffset &&
                "Frame pointer not pushed!");
 
-        unsigned LRReg = MRI.getLLVMRegNum(LRPush.getRegister(), true);
-        unsigned FPReg = MRI.getLLVMRegNum(FPPush.getRegister(), true);
+        unsigned LRReg = *MRI.getLLVMRegNum(LRPush.getRegister(), true);
+        unsigned FPReg = *MRI.getLLVMRegNum(FPPush.getRegister(), true);
 
         LRReg = getXRegFromWReg(LRReg);
         FPReg = getXRegFromWReg(FPReg);
 
         assert(LRReg == AArch64::LR && FPReg == AArch64::FP &&
                "Pushing invalid registers for frame!");
 
         // Indicate that the function has a frame.
         CompactUnwindEncoding |= CU::UNWIND_ARM64_MODE_FRAME;
         HasFP = true;
         break;
       }
       case MCCFIInstruction::OpDefCfaOffset: {
         assert(StackSize == 0 && "We already have the CFA offset!");
         StackSize = std::abs(Inst.getOffset());
         break;
       }
       case MCCFIInstruction::OpOffset: {
         // Registers are saved in pairs. We expect there to be two consecutive
         // `.cfi_offset' instructions with the appropriate registers specified.
-        unsigned Reg1 = MRI.getLLVMRegNum(Inst.getRegister(), true);
+        unsigned Reg1 = *MRI.getLLVMRegNum(Inst.getRegister(), true);
         if (i + 1 == e)
           return CU::UNWIND_ARM64_MODE_DWARF;
 
         const MCCFIInstruction &Inst2 = Instrs[++i];
         if (Inst2.getOperation() != MCCFIInstruction::OpOffset)
           return CU::UNWIND_ARM64_MODE_DWARF;
-        unsigned Reg2 = MRI.getLLVMRegNum(Inst2.getRegister(), true);
+        unsigned Reg2 = *MRI.getLLVMRegNum(Inst2.getRegister(), true);
 
         // N.B. The encodings must be in register number order, and the X
         // registers before the D registers.
 
         // X19/X20 pair = 0x00000001,
         // X21/X22 pair = 0x00000002,
         // X23/X24 pair = 0x00000004,
         // X25/X26 pair = 0x00000008,

llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

uint32_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
   DEBUG_WITH_TYPE("compact-unwind", llvm::dbgs() << "generateCU()\n");
   // Only armv7k uses CFI based unwinding.
   if (Subtype != MachO::CPU_SUBTYPE_ARM_V7K)
     return 0;
   // No .cfi directives means no frame.
   if (Instrs.empty())
     return 0;
   // Start off assuming CFA is at SP+0.
-  int CFARegister = ARM::SP;
+  unsigned CFARegister = ARM::SP;
   int CFARegisterOffset = 0;
   // Mark savable registers as initially unsaved
   DenseMap<unsigned, int> RegOffsets;
   int FloatRegCount = 0;
   // Process each .cfi directive and build up compact unwind info.
   for (size_t i = 0, e = Instrs.size(); i != e; ++i) {
-    int Reg;
+    unsigned Reg;
     const MCCFIInstruction &Inst = Instrs[i];
     switch (Inst.getOperation()) {
     case MCCFIInstruction::OpDefCfa: // DW_CFA_def_cfa
       CFARegisterOffset = -Inst.getOffset();
-      CFARegister = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      CFARegister = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       break;
     case MCCFIInstruction::OpDefCfaOffset: // DW_CFA_def_cfa_offset
       CFARegisterOffset = -Inst.getOffset();
       break;
     case MCCFIInstruction::OpDefCfaRegister: // DW_CFA_def_cfa_register
-      CFARegister = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      CFARegister = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       break;
     case MCCFIInstruction::OpOffset: // DW_CFA_offset
-      Reg = MRI.getLLVMRegNum(Inst.getRegister(), true);
+      Reg = *MRI.getLLVMRegNum(Inst.getRegister(), true);
       if (ARMMCRegisterClasses[ARM::GPRRegClassID].contains(Reg))
         RegOffsets[Reg] = Inst.getOffset();
       else if (ARMMCRegisterClasses[ARM::DPRRegClassID].contains(Reg)) {
         RegOffsets[Reg] = Inst.getOffset();
         ++FloatRegCount;
       } else {
         DEBUG_WITH_TYPE("compact-unwind",
                         llvm::dbgs() << ".cfi_offset on unknown register="

llvm/trunk/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

for (unsigned i = 0, e = Instrs.size(); i != e; ++i) {
         //     movq %rsp, %rbp
         //  L0:
         //     .cfi_def_cfa_register %rbp
         //
         HasFP = true;
 
         // If the frame pointer is other than esp/rsp, we do not have a way to
         // generate a compact unwinding representation, so bail out.
-        if (MRI.getLLVMRegNum(Inst.getRegister(), true) !=
+        if (*MRI.getLLVMRegNum(Inst.getRegister(), true) !=
             (Is64Bit ? X86::RBP : X86::EBP))
           return 0;
 
         // Reset the counts.
         memset(SavedRegs, 0, sizeof(SavedRegs));
         StackAdjust = 0;
         SavedRegIdx = 0;
         InstrOffset += MoveInstrSize;
for (unsigned i = 0, e = Instrs.size(); i != e; ++i) {
         //     .cfi_offset %r14, -32
         //     .cfi_offset %r15, -24
         //
         if (SavedRegIdx == CU_NUM_SAVED_REGS)
           // If there are too many saved registers, we cannot use a compact
           // unwind encoding.
           return CU::UNWIND_MODE_DWARF;
 
-        unsigned Reg = MRI.getLLVMRegNum(Inst.getRegister(), true);
+        unsigned Reg = *MRI.getLLVMRegNum(Inst.getRegister(), true);
         SavedRegs[SavedRegIdx++] = Reg;
         StackAdjust += OffsetSize;
         InstrOffset += PushInstrSize(Reg);
         break;
       }
       }
     }
 

llvm/trunk/test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s

 # RUN: llvm-dwarfdump -debug-loc %t4.o 2>&1 | FileCheck %s
 
 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE5=0 -o %t5.o
 # RUN: llvm-dwarfdump -debug-loc %t5.o 2>&1 | FileCheck %s
 
 # RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE6=0 -o %t6.o
 # RUN: llvm-dwarfdump -debug-loc %t6.o 2>&1 | FileCheck %s
 
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE7=0 -o %t7.o
+# RUN: llvm-dwarfdump -debug-loc %t7.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-REG
+
 # CHECK: error: unexpected end of data
 
+# UNKNOWN-REG: [0x0000000000000000,  0x0000000000000001): DW_OP_regx 0xdeadbeef
+
 .section  .debug_loc,"",@progbits
 .ifdef CASE1
   .byte  1                       # bogus
 .endif
 .ifdef CASE2
   .quad  0                       # starting offset
 .endif
 .ifdef CASE3
.ifdef CASE5
   .word  0                       # Loc expr size
   .quad  0                       # starting offset
 .endif
 .ifdef CASE6
   .quad  0                       # starting offset
   .quad  1                       # ending offset
   .word  0xffff                  # Loc expr size
 .endif
+.ifdef CASE7
+  .quad  0                       # starting offset
+  .quad  1                       # ending offset
+  .word  2f-1f                   # Loc expr size
+1:
+  .byte  0x90                    # DW_OP_regx
+  .uleb128 0xdeadbeef
+2:
+  .quad  0                       # starting offset
+  .quad  0                       # ending offset
+.endif
 
 # A minimal compile unit is needed to deduce the address size of the location
 # lists
 .section  .debug_info,"",@progbits
   .long  .Lcu_end0-.Lcu_begin0   # Length of Unit
 .Lcu_begin0:
   .short  4                      # DWARF version number
   .long  0                       # Offset Into Abbrev. Section
   .byte  8                       # Address Size (in bytes)
   .byte  0                       # End Of Children Mark
 .Lcu_end0: