[llvm] aaf6c7b - [globalisel] Select register bank for DBG_VALUE

via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 8 22:14:09 PDT 2022


Author: Luo, Yuanke
Date: 2022-08-09T13:11:51+08:00
New Revision: aaf6c7b05c9d155c8d34ec06a2f76ebdef8d21b9

URL: https://github.com/llvm/llvm-project/commit/aaf6c7b05c9d155c8d34ec06a2f76ebdef8d21b9
DIFF: https://github.com/llvm/llvm-project/commit/aaf6c7b05c9d155c8d34ec06a2f76ebdef8d21b9.diff

LOG: [globalisel] Select register bank for DBG_VALUE

The register operand of DBG_VALUE is not selected to a proper register
bank in both AArch64 and X86. This would cause getRegClass crash after
global ISel. After discussion, we think the MIR should assume all
vritual register should be set proper register class after global ISel,
so this patch is to fix the gap of DBG_VALUE for AArch64 and X86.

Differential Revision: https://reviews.llvm.org/D129037

Added: 
    

Modified: 
    llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
    llvm/lib/CodeGen/RegisterBankInfo.cpp
    llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
    llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
    llvm/lib/Target/X86/X86InstructionSelector.cpp
    llvm/lib/Target/X86/X86RegisterBankInfo.cpp
    llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir
    llvm/test/CodeGen/AArch64/GlobalISel/select-hint.mir
    llvm/test/DebugInfo/AArch64/debug-reg-bank.ll
    llvm/test/DebugInfo/X86/debug-reg-bank.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp b/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
index bce850ee212cf..07eece77143fe 100644
--- a/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
@@ -458,6 +458,7 @@ RegBankSelect::MappingCost RegBankSelect::computeMapping(
     LLVM_DEBUG(dbgs() << "Mapping is too expensive from the start\n");
     return Cost;
   }
+  const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
 
   // Moreover, to realize this mapping, the register bank of each operand must
   // match this mapping. In other words, we may need to locally reassign the
@@ -471,6 +472,10 @@ RegBankSelect::MappingCost RegBankSelect::computeMapping(
     Register Reg = MO.getReg();
     if (!Reg)
       continue;
+    LLT Ty = MRI.getType(Reg);
+    if (!Ty.isValid())
+      continue;
+
     LLVM_DEBUG(dbgs() << "Opd" << OpIdx << '\n');
     const RegisterBankInfo::ValueMapping &ValMapping =
         InstrMapping.getOperandMapping(OpIdx);
@@ -603,6 +608,9 @@ bool RegBankSelect::applyMapping(
       MRI->setRegBank(Reg, *ValMapping.BreakDown[0].RegBank);
       break;
     case RepairingPlacement::Insert:
+      // Don't insert additional instruction for debug instruction.
+      if (MI.isDebugInstr())
+        break;
       OpdMapper.createVRegs(OpIdx);
       if (!repairReg(MO, ValMapping, RepairPt, OpdMapper.getVRegs(OpIdx)))
         return false;
@@ -716,10 +724,6 @@ bool RegBankSelect::runOnMachineFunction(MachineFunction &MF) {
       if (MI.isInlineAsm())
         continue;
 
-      // Ignore debug info.
-      if (MI.isDebugInstr())
-        continue;
-
       // Ignore IMPLICIT_DEF which must have a regclass.
       if (MI.isImplicitDef())
         continue;

diff  --git a/llvm/lib/CodeGen/RegisterBankInfo.cpp b/llvm/lib/CodeGen/RegisterBankInfo.cpp
index de851ffc7fdca..22d8309e598ba 100644
--- a/llvm/lib/CodeGen/RegisterBankInfo.cpp
+++ b/llvm/lib/CodeGen/RegisterBankInfo.cpp
@@ -449,6 +449,9 @@ void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
       LLVM_DEBUG(dbgs() << " is $noreg, nothing to be done\n");
       continue;
     }
+    LLT Ty = MRI.getType(MO.getReg());
+    if (!Ty.isValid())
+      continue;
     assert(OpdMapper.getInstrMapping().getOperandMapping(OpIdx).NumBreakDowns !=
                0 &&
            "Invalid mapping");
@@ -601,6 +604,7 @@ bool RegisterBankInfo::InstructionMapping::verify(
   const MachineFunction &MF = *MI.getMF();
   const RegisterBankInfo *RBI = MF.getSubtarget().getRegBankInfo();
   (void)RBI;
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
 
   for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     const MachineOperand &MO = MI.getOperand(Idx);
@@ -612,6 +616,9 @@ bool RegisterBankInfo::InstructionMapping::verify(
     Register Reg = MO.getReg();
     if (!Reg)
       continue;
+    LLT Ty = MRI.getType(Reg);
+    if (!Ty.isValid())
+      continue;
     assert(getOperandMapping(Idx).isValid() &&
            "We must have a mapping for reg operands");
     const RegisterBankInfo::ValueMapping &MOMapping = getOperandMapping(Idx);

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 870c8e27f9949..ad053cdde0538 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -907,6 +907,38 @@ getRegClassesForCopy(MachineInstr &I, const TargetInstrInfo &TII,
           getMinClassForRegBank(DstRegBank, DstSize, true)};
 }
 
+// FIXME: We need some sort of API in RBI/TRI to allow generic code to
+// constrain operands of simple instructions given a TargetRegisterClass
+// and LLT
+static bool selectDebugInstr(MachineInstr &I, MachineRegisterInfo &MRI,
+                             const RegisterBankInfo &RBI) {
+  for (MachineOperand &MO : I.operands()) {
+    if (!MO.isReg())
+      continue;
+    Register Reg = MO.getReg();
+    if (!Reg)
+      continue;
+    if (Reg.isPhysical())
+      continue;
+    LLT Ty = MRI.getType(Reg);
+    const RegClassOrRegBank &RegClassOrBank = MRI.getRegClassOrRegBank(Reg);
+    const TargetRegisterClass *RC =
+        RegClassOrBank.dyn_cast<const TargetRegisterClass *>();
+    if (!RC) {
+      const RegisterBank &RB = *RegClassOrBank.get<const RegisterBank *>();
+      RC = getRegClassForTypeOnBank(Ty, RB);
+      if (!RC) {
+        LLVM_DEBUG(
+            dbgs() << "Warning: DBG_VALUE operand has unexpected size/bank\n");
+        break;
+      }
+    }
+    RBI.constrainGenericRegister(Reg, *RC, MRI);
+  }
+
+  return true;
+}
+
 static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
                        MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
                        const RegisterBankInfo &RBI) {
@@ -2377,6 +2409,9 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
     if (I.isCopy())
       return selectCopy(I, TII, MRI, TRI, RBI);
 
+    if (I.isDebugInstr())
+      return selectDebugInstr(I, MRI, RBI);
+
     return true;
   }
 

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index c48ab31a429f9..47785ef73cced 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -681,6 +681,8 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
       continue;
 
     LLT Ty = MRI.getType(MO.getReg());
+    if (!Ty.isValid())
+      continue;
     OpSize[Idx] = Ty.getSizeInBits();
 
     // As a top-level guess, vectors go in FPRs, scalars and pointers in GPRs.
@@ -989,6 +991,9 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   SmallVector<const ValueMapping *, 8> OpdsMapping(NumOperands);
   for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     if (MI.getOperand(Idx).isReg() && MI.getOperand(Idx).getReg()) {
+      LLT Ty = MRI.getType(MI.getOperand(Idx).getReg());
+      if (!Ty.isValid())
+        continue;
       auto Mapping = getValueMapping(OpRegBankIdx[Idx], OpSize[Idx]);
       if (!Mapping->isValid())
         return getInvalidInstructionMapping();

diff  --git a/llvm/lib/Target/X86/X86InstructionSelector.cpp b/llvm/lib/Target/X86/X86InstructionSelector.cpp
index ff701159b95ea..0f95e5c142f98 100644
--- a/llvm/lib/Target/X86/X86InstructionSelector.cpp
+++ b/llvm/lib/Target/X86/X86InstructionSelector.cpp
@@ -94,6 +94,7 @@ class X86InstructionSelector : public InstructionSelector {
                   MachineFunction &MF) const;
   bool selectUadde(MachineInstr &I, MachineRegisterInfo &MRI,
                    MachineFunction &MF) const;
+  bool selectDebugInstr(MachineInstr &I, MachineRegisterInfo &MRI) const;
   bool selectCopy(MachineInstr &I, MachineRegisterInfo &MRI) const;
   bool selectUnmergeValues(MachineInstr &I, MachineRegisterInfo &MRI,
                            MachineFunction &MF);
@@ -230,6 +231,38 @@ static const TargetRegisterClass *getRegClassFromGRPhysReg(Register Reg) {
   llvm_unreachable("Unknown RegClass for PhysReg!");
 }
 
+// FIXME: We need some sort of API in RBI/TRI to allow generic code to
+// constrain operands of simple instructions given a TargetRegisterClass
+// and LLT
+bool X86InstructionSelector::selectDebugInstr(MachineInstr &I,
+                                              MachineRegisterInfo &MRI) const {
+  for (MachineOperand &MO : I.operands()) {
+    if (!MO.isReg())
+      continue;
+    Register Reg = MO.getReg();
+    if (!Reg)
+      continue;
+    if (Reg.isPhysical())
+      continue;
+    LLT Ty = MRI.getType(Reg);
+    const RegClassOrRegBank &RegClassOrBank = MRI.getRegClassOrRegBank(Reg);
+    const TargetRegisterClass *RC =
+        RegClassOrBank.dyn_cast<const TargetRegisterClass *>();
+    if (!RC) {
+      const RegisterBank &RB = *RegClassOrBank.get<const RegisterBank *>();
+      RC = getRegClass(Ty, RB);
+      if (!RC) {
+        LLVM_DEBUG(
+            dbgs() << "Warning: DBG_VALUE operand has unexpected size/bank\n");
+        break;
+      }
+    }
+    RBI.constrainGenericRegister(Reg, *RC, MRI);
+  }
+
+  return true;
+}
+
 // Set X86 Opcode and constrain DestReg.
 bool X86InstructionSelector::selectCopy(MachineInstr &I,
                                         MachineRegisterInfo &MRI) const {
@@ -326,6 +359,9 @@ bool X86InstructionSelector::select(MachineInstr &I) {
     if (I.isCopy())
       return selectCopy(I, MRI);
 
+    if (I.isDebugInstr())
+      return selectDebugInstr(I, MRI);
+
     return true;
   }
 

diff  --git a/llvm/lib/Target/X86/X86RegisterBankInfo.cpp b/llvm/lib/Target/X86/X86RegisterBankInfo.cpp
index c49fc458eab3e..733db70f14a2e 100644
--- a/llvm/lib/Target/X86/X86RegisterBankInfo.cpp
+++ b/llvm/lib/Target/X86/X86RegisterBankInfo.cpp
@@ -114,7 +114,7 @@ void X86RegisterBankInfo::getInstrPartialMappingIdxs(
   unsigned NumOperands = MI.getNumOperands();
   for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     auto &MO = MI.getOperand(Idx);
-    if (!MO.isReg())
+    if (!MO.isReg() || !MO.getReg())
       OpRegBankIdx[Idx] = PMI_None;
     else
       OpRegBankIdx[Idx] = getPartialMappingIdx(MRI.getType(MO.getReg()), isFP);
@@ -130,6 +130,8 @@ bool X86RegisterBankInfo::getInstrValueMapping(
   for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     if (!MI.getOperand(Idx).isReg())
       continue;
+    if (!MI.getOperand(Idx).getReg())
+      continue;
 
     auto Mapping = getValueMapping(OpRegBankIdx[Idx], 1);
     if (!Mapping->isValid())

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir b/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir
index 0df73e81d600f..316cd580994c8 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir
@@ -65,7 +65,7 @@ body: |
     ; CHECK-LABEL: name: test_dbg_value_dead
     ; CHECK: liveins: $w0
     ; CHECK-NEXT: {{  $}}
-    ; CHECK-NEXT: DBG_VALUE %0:gpr, $noreg, !7, !DIExpression(), debug-location !9
+    ; CHECK-NEXT: DBG_VALUE %0:gpr32, $noreg, !7, !DIExpression(), debug-location !9
     %0:gpr(s32) = COPY $w0
     DBG_VALUE %0(s32), $noreg, !7, !DIExpression(), debug-location !9
 ...

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/select-hint.mir b/llvm/test/CodeGen/AArch64/GlobalISel/select-hint.mir
index a584310cec4f9..99e864a287f2b 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/select-hint.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/select-hint.mir
@@ -16,7 +16,7 @@ body:             |
     ; CHECK-LABEL: name: assert_zext_gpr
     ; CHECK: liveins: $w0, $w1
     ; CHECK-NEXT: {{  $}}
-    ; CHECK-NEXT: %copy:gpr32all = COPY $w0
+    ; CHECK-NEXT: %copy:gpr32 = COPY $w0
     ; CHECK-NEXT: $w1 = COPY %copy
     ; CHECK-NEXT: RET_ReallyLR implicit $w1
     %copy:gpr(s32) = COPY $w0
@@ -104,7 +104,7 @@ body:             |
     ; CHECK-LABEL: name: assert_sext_gpr
     ; CHECK: liveins: $w0, $w1
     ; CHECK-NEXT: {{  $}}
-    ; CHECK-NEXT: %copy:gpr32all = COPY $w0
+    ; CHECK-NEXT: %copy:gpr32 = COPY $w0
     ; CHECK-NEXT: $w1 = COPY %copy
     ; CHECK-NEXT: RET_ReallyLR implicit $w1
     %copy:gpr(s32) = COPY $w0

diff  --git a/llvm/test/DebugInfo/AArch64/debug-reg-bank.ll b/llvm/test/DebugInfo/AArch64/debug-reg-bank.ll
index ffd88821fd802..19c33b34f4a39 100644
--- a/llvm/test/DebugInfo/AArch64/debug-reg-bank.ll
+++ b/llvm/test/DebugInfo/AArch64/debug-reg-bank.ll
@@ -6,7 +6,7 @@ define void @foo(i32 %n) !dbg !7 {
   ; CHECK: bb.1.entry:
   ; CHECK-NEXT:   liveins: $w0
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT:   DBG_VALUE %{{[0-9]+}}:_, $noreg, !12, !DIExpression(), debug-location !19
+  ; CHECK-NEXT:   DBG_VALUE %2:gpr32, $noreg, !12, !DIExpression(), debug-location !19
   ; CHECK-NEXT:   RET_ReallyLR debug-location !20
 entry:
   %m = mul i32 %n, 13

diff  --git a/llvm/test/DebugInfo/X86/debug-reg-bank.ll b/llvm/test/DebugInfo/X86/debug-reg-bank.ll
index ed2da7a61f80a..8d3bd95720a6b 100644
--- a/llvm/test/DebugInfo/X86/debug-reg-bank.ll
+++ b/llvm/test/DebugInfo/X86/debug-reg-bank.ll
@@ -6,7 +6,7 @@ define void @foo(i32 %n) !dbg !7 {
   ; CHECK: bb.1.entry:
   ; CHECK-NEXT:   liveins: $edi
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT:   DBG_VALUE %{{[0-9]+}}:_, $noreg, !12, !DIExpression(), debug-location !19
+  ; CHECK-NEXT:   DBG_VALUE %2:gr32, $noreg, !12, !DIExpression(), debug-location !19
   ; CHECK-NEXT:   RET 0, debug-location !20
 entry:
   %m = mul i32 %n, 13


        


More information about the llvm-commits mailing list