[llvm] facb3ac - [GlobalISel][DebugInfo] salvageDebugInfo analogue for gMIR

Kristina Bessonova via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 02:15:23 PDT 2022


Author: Vladislav Dzhidzhoev
Date: 2022-08-01T11:14:53+02:00
New Revision: facb3ac385c3c5fc6775ea320ed2fcfeb2f595f6

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

LOG: [GlobalISel][DebugInfo] salvageDebugInfo analogue for gMIR

Salvage debug info of instruction that is about to be deleted as dead in
Combiner pass. Currently supported instructions are COPY and G_TRUNC.

It allows to salvage debug info of some dead arguments of functions, by putting
DWARF expression corresponding to the instruction being deleted into related
DBG_VALUE instruction.

Here is an example of missing variables location https://godbolt.org/z/K48osb9dK.
We see that arguments x, y of function foo are not available in debugger, and
corresponding DBG_VALUE instructions have undefined register operand instead of
variables locaton after Aarch64PreLegalizerCombiner pass. The reason is that
registers where variables are located are removed as dead (with instruction
G_TRUNC). We can use salvageDebugInfo analogue for gMIR to preserve debug
locations of dead variables.

Statistics of llvm object files built with vs without this commit on -O2
optimization level (CMAKE_BUILD_TYPE=RelWithDebInfo, -fglobal-isel) on Aarch64 (macOS):

Number of variables with 100% of parent scope covered by DW_AT_location has been increased by 7,9%.
Number of variables with 0% coverage of parent scope has been decreased by 1,2%.
Number of variables processed by location statistics has been increased by 2,9%.
Average PC ranges coverage has been increased by 1,8 percentage points.

Coverage can be improved by supporting more instructions, or by calling
salvageDebugInfo for instructions that are deleted during Combiner rules exection.

Reviewed By: aprantl

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

Added: 
    llvm/test/CodeGen/AArch64/GlobalISel/salvage-debug-info-dead.mir

Modified: 
    llvm/include/llvm/CodeGen/CodeGenCommonISel.h
    llvm/include/llvm/CodeGen/GlobalISel/Utils.h
    llvm/lib/CodeGen/CodeGenCommonISel.cpp
    llvm/lib/CodeGen/GlobalISel/Combiner.cpp
    llvm/lib/CodeGen/GlobalISel/Utils.cpp
    llvm/test/DebugInfo/AArch64/debug-reg-bank.ll
    llvm/test/DebugInfo/X86/debug-reg-bank.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/CodeGenCommonISel.h b/llvm/include/llvm/CodeGen/CodeGenCommonISel.h
index ce278468dffca..3b11c840256d2 100644
--- a/llvm/include/llvm/CodeGen/CodeGenCommonISel.h
+++ b/llvm/include/llvm/CodeGen/CodeGenCommonISel.h
@@ -220,6 +220,12 @@ findSplitPointForStackProtector(MachineBasicBlock *BB,
 /// test.
 unsigned getInvertedFPClassTest(unsigned Test);
 
+/// Assuming the instruction \p MI is going to be deleted, attempt to salvage
+/// debug users of \p MI by writing the effect of \p MI in a DIExpression.
+void salvageDebugInfoForDbgValue(const MachineRegisterInfo &MRI,
+                                 MachineInstr &MI,
+                                 ArrayRef<MachineOperand *> DbgUsers);
+
 } // namespace llvm
 
 #endif // LLVM_CODEGEN_CODEGENCOMMONISEL_H

diff  --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index 31f3d5d84186a..68f5003710b96 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -506,5 +506,9 @@ void eraseInstrs(ArrayRef<MachineInstr *> DeadInstrs, MachineRegisterInfo &MRI,
 void eraseInstr(MachineInstr &MI, MachineRegisterInfo &MRI,
                 LostDebugLocObserver *LocObserver = nullptr);
 
+/// Assuming the instruction \p MI is going to be deleted, attempt to salvage
+/// debug users of \p MI by writing the effect of \p MI in a DIExpression.
+void salvageDebugInfo(const MachineRegisterInfo &MRI, MachineInstr &MI);
+
 } // End namespace llvm.
 #endif

diff  --git a/llvm/lib/CodeGen/CodeGenCommonISel.cpp b/llvm/lib/CodeGen/CodeGenCommonISel.cpp
index 8f185a161bd0b..44cdd8275beda 100644
--- a/llvm/lib/CodeGen/CodeGenCommonISel.cpp
+++ b/llvm/lib/CodeGen/CodeGenCommonISel.cpp
@@ -17,6 +17,9 @@
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetOpcodes.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+
+#define DEBUG_TYPE "codegen-common"
 
 using namespace llvm;
 
@@ -197,3 +200,88 @@ unsigned llvm::getInvertedFPClassTest(unsigned Test) {
   }
   return 0;
 }
+
+static MachineOperand *getSalvageOpsForCopy(const MachineRegisterInfo &MRI,
+                                            MachineInstr &Copy) {
+  assert(Copy.getOpcode() == TargetOpcode::COPY && "Must be a COPY");
+
+  return &Copy.getOperand(1);
+}
+
+static MachineOperand *getSalvageOpsForTrunc(const MachineRegisterInfo &MRI,
+                                            MachineInstr &Trunc,
+                                            SmallVectorImpl<uint64_t> &Ops) {
+  assert(Trunc.getOpcode() == TargetOpcode::G_TRUNC && "Must be a G_TRUNC");
+
+  const auto FromLLT = MRI.getType(Trunc.getOperand(1).getReg());
+  const auto ToLLT = MRI.getType(Trunc.defs().begin()->getReg());
+
+  // TODO: Support non-scalar types.
+  if (!FromLLT.isScalar()) {
+    return nullptr;
+  }
+
+  auto ExtOps = DIExpression::getExtOps(FromLLT.getSizeInBits(),
+                                        ToLLT.getSizeInBits(), false);
+  Ops.append(ExtOps.begin(), ExtOps.end());
+  return &Trunc.getOperand(1);
+}
+
+static MachineOperand *salvageDebugInfoImpl(const MachineRegisterInfo &MRI,
+                                            MachineInstr &MI,
+                                            SmallVectorImpl<uint64_t> &Ops) {
+  switch (MI.getOpcode()) {
+  case TargetOpcode::G_TRUNC:
+    return getSalvageOpsForTrunc(MRI, MI, Ops);
+  case TargetOpcode::COPY:
+    return getSalvageOpsForCopy(MRI, MI);
+  default:
+    return nullptr;
+  }
+}
+
+void llvm::salvageDebugInfoForDbgValue(const MachineRegisterInfo &MRI,
+                                       MachineInstr &MI,
+                                       ArrayRef<MachineOperand *> DbgUsers) {
+  // These are arbitrary chosen limits on the maximum number of values and the
+  // maximum size of a debug expression we can salvage up to, used for
+  // performance reasons.
+  const unsigned MaxExpressionSize = 128;
+
+  for (auto *DefMO : DbgUsers) {
+    MachineInstr *DbgMI = DefMO->getParent();
+    if (DbgMI->isIndirectDebugValue()) {
+      continue;
+    }
+
+    int UseMOIdx = DbgMI->findRegisterUseOperandIdx(DefMO->getReg());
+    assert(UseMOIdx != -1 && DbgMI->hasDebugOperandForReg(DefMO->getReg()) &&
+           "Must use salvaged instruction as its location");
+
+    // TODO: Support DBG_VALUE_LIST.
+    if (DbgMI->getOpcode() != TargetOpcode::DBG_VALUE) {
+      assert(DbgMI->getOpcode() == TargetOpcode::DBG_VALUE_LIST &&
+             "Must be either DBG_VALUE or DBG_VALUE_LIST");
+      continue;
+    }
+
+    const DIExpression *SalvagedExpr = DbgMI->getDebugExpression();
+
+    SmallVector<uint64_t, 16> Ops;
+    auto Op0 = salvageDebugInfoImpl(MRI, MI, Ops);
+    if (!Op0)
+      continue;
+    SalvagedExpr = DIExpression::appendOpsToArg(SalvagedExpr, Ops, 0, true);
+
+    bool IsValidSalvageExpr =
+        SalvagedExpr->getNumElements() <= MaxExpressionSize;
+    if (IsValidSalvageExpr) {
+      auto &UseMO = DbgMI->getOperand(UseMOIdx);
+      UseMO.setReg(Op0->getReg());
+      UseMO.setSubReg(Op0->getSubReg());
+      DbgMI->getDebugExpressionOp().setMetadata(SalvagedExpr);
+
+      LLVM_DEBUG(dbgs() << "SALVAGE: " << *DbgMI << '\n');
+    }
+  }
+}

diff  --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
index 1a5fe3e84c17f..5b1f20af168f7 100644
--- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -132,6 +132,7 @@ bool Combiner::combineMachineInstrs(MachineFunction &MF,
         // Erase dead insts before even adding to the list.
         if (isTriviallyDead(CurMI, *MRI)) {
           LLVM_DEBUG(dbgs() << CurMI << "Is dead; erasing.\n");
+          llvm::salvageDebugInfo(*MRI, CurMI);
           CurMI.eraseFromParent();
           continue;
         }

diff  --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 013c8700e8ae3..e0bcc2dd6a4e8 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -13,6 +13,7 @@
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/CodeGen/CodeGenCommonISel.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/GISelKnownBits.h"
 #include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
@@ -1335,3 +1336,22 @@ void llvm::eraseInstr(MachineInstr &MI, MachineRegisterInfo &MRI,
                       LostDebugLocObserver *LocObserver) {
   return eraseInstrs({&MI}, MRI, LocObserver);
 }
+
+void llvm::salvageDebugInfo(const MachineRegisterInfo &MRI, MachineInstr &MI) {
+  for (auto &Def : MI.defs()) {
+    assert(Def.isReg() && "Must be a reg");
+
+    SmallVector<MachineOperand *, 16> DbgUsers;
+    for (auto &MOUse : MRI.use_operands(Def.getReg())) {
+      MachineInstr *DbgValue = MOUse.getParent();
+      // Ignore partially formed DBG_VALUEs.
+      if (DbgValue->isNonListDebugValue() && DbgValue->getNumOperands() == 4) {
+        DbgUsers.push_back(&MOUse);
+      }
+    }
+
+    if (!DbgUsers.empty()) {
+      salvageDebugInfoForDbgValue(MRI, MI, DbgUsers);
+    }
+  }
+}

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/salvage-debug-info-dead.mir b/llvm/test/CodeGen/AArch64/GlobalISel/salvage-debug-info-dead.mir
new file mode 100644
index 0000000000000..fb8a5c1e0fd3a
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/salvage-debug-info-dead.mir
@@ -0,0 +1,72 @@
+# RUN: llc -global-isel=1 -O0 -run-pass=aarch64-O0-prelegalizer-combiner %s -o - -verify-machineinstrs | FileCheck %s --check-prefix=CHECK
+
+--- |
+  ; ModuleID = 'salvage-debug-info-dead.mir'
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+
+  @A = global i8 1234
+
+  declare external i128 @foo()
+
+  define void @main() !dbg !6 {
+    ret void
+  }
+
+  !llvm.dbg.cu = !{!2}
+  !llvm.module.flags = !{!10, !11, !12}
+  !llvm.ident = !{!20}
+
+  !1 = distinct !DIGlobalVariable(name: "A", scope: !2, file: !3, line: 1, type: !7, isLocal: false, isDefinition: true)
+  !2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+  !3 = !DIFile(filename: "test.ll", directory: "/")
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !DISubroutineType(types: !23)
+  !9 = distinct !DIGlobalVariable(name: "C", scope: !2, file: !3, line: 3, type: !7, isLocal: false, isDefinition: true)
+  !10 = !{i32 7, !"Dwarf Version", i32 4}
+  !11 = !{i32 2, !"Debug Info Version", i32 3}
+  !12 = !{i32 1, !"wchar_size", i32 4}
+  !20 = !{!"clang"}
+  !6 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 5, type: !8, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !24)
+  !23 = !{!7}
+  !24 = !{}
+  !25 = !DILocalVariable(name: "X", scope: !6, file: !3, line: 6, type: !7)
+  !42 = !DILocalVariable(name: "Y", scope: !6, file: !3, line: 12, type: !7)
+  !52 = !DILocalVariable(name: "Z", scope: !6, file: !3, line: 16, type: !7)
+  !56 = !DILocalVariable(name: "W", scope: !6, file: !3, line: 18, type: !7)
+  !57 = !DILocalVariable(name: "P", scope: !6, file: !3, line: 18, type: !7)
+...
+---
+# Check that we salvage debug information for instructions
+# deleted as dead in Combiner pass
+name: main
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: main
+  ; CHECK: bb.0:
+  ; CHECK:   liveins: $x0
+  ; CHECK-NEXT: {{   $}}
+  ; CHECK-NEXT:   DBG_VALUE $x0, $noreg, {{.*}}, !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value), debug-location !DILocation(line: 6, column: 7, scope: !6)
+  ; CHECK-NEXT:   DBG_VALUE $x0, $noreg, {{.*}}, !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_LLVM_convert, 16, DW_ATE_unsigned, DW_OP_stack_value), debug-location !DILocation(line: 7, column: 7, scope: !6)
+  ; CHECK-NEXT:   DBG_VALUE $x0, $noreg, {{.*}}, !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_LLVM_convert, 16, DW_ATE_unsigned, DW_OP_LLVM_convert, 16, DW_ATE_unsigned, DW_OP_LLVM_convert, 8, DW_ATE_unsigned, DW_OP_stack_value), debug-location !DILocation(line: 8, column: 7, scope: !6)
+  ; CHECK: bb.1:
+  ; CHECK-NEXT:   DBG_VALUE $x0, $noreg, {{.*}}, !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value), debug-location !DILocation(line: 9, column: 7, scope: !6)
+  ; CHECK-NEXT:   DBG_VALUE %4:_(s32), 0, {{.*}}, !DIExpression(), debug-location !DILocation(line: 9, column: 7, scope: !6)
+  bb.0:
+    liveins: $x0
+    %0:_(s64) = COPY $x0
+    %1:_(s32) = G_TRUNC %0:_(s64)
+    DBG_VALUE %1:_(s32), $noreg, !25, !DIExpression(), debug-location !DILocation(line: 6, column: 7, scope: !6)
+
+    %2:_(s16) = G_TRUNC %1:_(s32)
+    DBG_VALUE %2:_(s16), $noreg, !42, !DIExpression(), debug-location !DILocation(line: 7, column: 7, scope: !6)
+
+    %3:_(s8) = G_TRUNC %2:_(s16)
+    DBG_VALUE %3:_(s8), $noreg, !52, !DIExpression(), debug-location !DILocation(line: 8, column: 7, scope: !6)
+
+  bb.1:
+    %4:_(s32) = G_TRUNC %0:_(s64)
+    DBG_VALUE %4:_(s32), $noreg, !56, !DIExpression(), debug-location !DILocation(line: 9, column: 7, scope: !6)
+    DBG_VALUE %4:_(s32), 0, !57, !DIExpression(), debug-location !DILocation(line: 9, column: 7, scope: !6)
+
+...

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

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


        


More information about the llvm-commits mailing list