[llvm] [RemoveDIs][DebugInfo] Verifier and printing fixes for DPLabel (PR #83242)

Stephen Tozer via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 28 01:58:40 PST 2024


https://github.com/SLTozer created https://github.com/llvm/llvm-project/pull/83242

`DPLabel`, the RemoveDI version of `llvm.dbg.label`, was landed recently at the same time the RemoveDIs IR printing and verifier patches were landing. The patches were updated to not miscompile, but did not have full-featured and correct support for DPLabel built in; this patch makes the remaining set of changes to enable DPLabel support.

>From 85b695a4cc4557f6aa9f540a3225a56ccce65807 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Wed, 28 Feb 2024 09:17:59 +0000
Subject: [PATCH] Verifier and printing fixes for DPLabel

---
 .../include/llvm/IR/DebugProgramInstruction.h | 12 ++--
 llvm/lib/IR/AsmWriter.cpp                     |  2 +
 llvm/lib/IR/DebugProgramInstruction.cpp       | 12 +++-
 llvm/lib/IR/Verifier.cpp                      | 62 +++++++++++++++----
 .../print-non-instruction-debug-info.ll       |  4 +-
 5 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 2dd546ce709c76..3b35c7bd45631a 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -174,13 +174,10 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DbgRecord &R) {
 /// llvm.dbg.label intrinsic.
 /// FIXME: Rename DbgLabelRecord when DPValue is renamed to DbgVariableRecord.
 class DPLabel : public DbgRecord {
-  DILabel *Label;
+  TrackingMDNodeRef Label;
 
 public:
-  DPLabel(DILabel *Label, DebugLoc DL)
-      : DbgRecord(LabelKind, DL), Label(Label) {
-    assert(Label && "Unexpected nullptr");
-  }
+  DPLabel(DILabel *Label, DebugLoc DL);
 
   DPLabel *clone() const;
   void print(raw_ostream &O, bool IsForDebug = false) const;
@@ -188,8 +185,9 @@ class DPLabel : public DbgRecord {
   DbgLabelInst *createDebugIntrinsic(Module *M,
                                      Instruction *InsertBefore) const;
 
-  void setLabel(DILabel *NewLabel) { Label = NewLabel; }
-  DILabel *getLabel() const { return Label; }
+  void setLabel(DILabel *NewLabel);
+  DILabel *getLabel() const;
+  MDNode *getRawLabel() const { return Label; };
 
   /// Support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const DbgRecord *E) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 4e1e48b4ad4a35..9c209c7fb4d46e 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -4660,6 +4660,8 @@ void AssemblyWriter::printDPLabel(const DPLabel &Label) {
   auto WriterCtx = getContext();
   Out << "#dbg_label(";
   WriteAsOperandInternal(Out, Label.getLabel(), WriterCtx, true);
+  Out << ", ";
+  WriteAsOperandInternal(Out, Label.getDebugLoc(), WriterCtx, true);
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 3a8b94a87bbcf0..987f1c8f636e6a 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -123,6 +123,14 @@ DbgRecord::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
   llvm_unreachable("unsupported DbgRecord kind");
 }
 
+DPLabel::DPLabel(DILabel *Label, DebugLoc DL)
+    : DbgRecord(LabelKind, DL), Label(Label) {
+  assert(Label && "Unexpected nullptr");
+}
+
+void DPLabel::setLabel(DILabel *NewLabel) { Label.reset(NewLabel); }
+DILabel *DPLabel::getLabel() const { return cast<DILabel>(Label); }
+
 DPValue *DPValue::createDPValue(Value *Location, DILocalVariable *DV,
                                 DIExpression *Expr, const DILocation *DI) {
   return new DPValue(ValueAsMetadata::get(Location), DV, Expr, DI,
@@ -337,7 +345,9 @@ DbgRecord *DbgRecord::clone() const {
 
 DPValue *DPValue::clone() const { return new DPValue(*this); }
 
-DPLabel *DPLabel::clone() const { return new DPLabel(Label, getDebugLoc()); }
+DPLabel *DPLabel::clone() const {
+  return new DPLabel(getLabel(), getDebugLoc());
+}
 
 DbgVariableIntrinsic *
 DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 3741e5deaa4cd1..2a93e47fb9c7df 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -547,6 +547,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
 
   void visitTemplateParams(const MDNode &N, const Metadata &RawParams);
 
+  void visit(DPLabel &DPL);
   void visit(DPValue &DPV);
   // InstVisitor overrides...
   using InstVisitor<Verifier>::visit;
@@ -683,10 +684,17 @@ void Verifier::visitDbgRecords(Instruction &I) {
           &I);
   CheckDI(!isa<PHINode>(&I) || !I.hasDbgValues(),
           "PHI Node must not have any attached DbgRecords", &I);
-  for (DPValue &DPV : DPValue::filter(I.getDbgValueRange())) {
-    CheckDI(DPV.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
-            &I, &DPV);
-    visit(DPV);
+  for (DbgRecord &DR : I.getDbgValueRange()) {
+    CheckDI(DR.getMarker() == I.DbgMarker, "DbgRecord had invalid DbgMarker",
+            &I, &DR);
+    if (MDNode *N = DR.getDebugLoc().getAsMDNode()) {
+      CheckDI(isa<DILocation>(N), "invalid #dbg record location", &DR, N);
+      visitDILocation(*cast<DILocation>(N));
+    }
+    if (auto *DPV = dyn_cast<DPValue>(&DR))
+      visit(*DPV);
+    else if (auto *DPL = dyn_cast<DPLabel>(&DR))
+      visit(*DPL);
   }
 }
 
@@ -6187,6 +6195,34 @@ static DISubprogram *getSubprogram(Metadata *LocalScope) {
   return nullptr;
 }
 
+void Verifier::visit(DPLabel &DPL) {
+  CheckDI(isa<DILabel>(DPL.getRawLabel()),
+          "invalid #dbg_label intrinsic variable", &DPL, DPL.getRawLabel());
+
+  // Ignore broken !dbg attachments; they're checked elsewhere.
+  if (MDNode *N = DPL.getDebugLoc().getAsMDNode())
+    if (!isa<DILocation>(N))
+      return;
+
+  BasicBlock *BB = DPL.getParent();
+  Function *F = BB ? BB->getParent() : nullptr;
+
+  // The scopes for variables and !dbg attachments must agree.
+  DILabel *Label = DPL.getLabel();
+  DILocation *Loc = DPL.getDebugLoc();
+  Check(Loc, "#dbg_label record requires a !dbg attachment", &DPL, BB, F);
+
+  DISubprogram *LabelSP = getSubprogram(Label->getRawScope());
+  DISubprogram *LocSP = getSubprogram(Loc->getRawScope());
+  if (!LabelSP || !LocSP)
+    return;
+
+  CheckDI(LabelSP == LocSP,
+          "mismatched subprogram between #dbg_label label and !dbg attachment",
+          &DPL, BB, F, Label, Label->getScope()->getSubprogram(), Loc,
+          Loc->getScope()->getSubprogram());
+}
+
 void Verifier::visit(DPValue &DPV) {
   CheckDI(DPV.getType() == DPValue::LocationType::Value ||
               DPV.getType() == DPValue::LocationType::Declare ||
@@ -6223,16 +6259,20 @@ void Verifier::visit(DPValue &DPV) {
               "inst not in same function as #dbg_assign", I, &DPV);
   }
 
-  if (MDNode *N = DPV.getDebugLoc().getAsMDNode()) {
-    CheckDI(isa<DILocation>(N), "invalid #dbg record location", &DPV, N);
-    visitDILocation(*cast<DILocation>(N));
-  }
+  // This check is redundant with one in visitLocalVariable().
+  DILocalVariable *Var = DPV.getVariable();
+  CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
+          Var->getRawType());
+
+  // Ignore broken !dbg attachments; they're checked elsewhere.
+  if (MDNode *N = DPV.getDebugLoc().getAsMDNode())
+    if (!isa<DILocation>(N))
+      return;
 
   BasicBlock *BB = DPV.getParent();
   Function *F = BB ? BB->getParent() : nullptr;
 
   // The scopes for variables and !dbg attachments must agree.
-  DILocalVariable *Var = DPV.getVariable();
   DILocation *Loc = DPV.getDebugLoc();
   CheckDI(Loc, "missing #dbg record DILocation", &DPV, BB, F);
 
@@ -6245,10 +6285,6 @@ void Verifier::visit(DPValue &DPV) {
           "mismatched subprogram between #dbg record variable and DILocation",
           &DPV, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
           Loc->getScope()->getSubprogram());
-
-  // This check is redundant with one in visitLocalVariable().
-  CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
-          Var->getRawType());
 }
 
 void Verifier::visitVPIntrinsic(VPIntrinsic &VPI) {
diff --git a/llvm/test/DebugInfo/print-non-instruction-debug-info.ll b/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
index f8271df146fe96..2e765619fcb896 100644
--- a/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
+++ b/llvm/test/DebugInfo/print-non-instruction-debug-info.ll
@@ -21,8 +21,8 @@
 ; CHECK-NEXT: {{^}}  %[[VAL_ADD:[0-9a-zA-Z]+]] = add i32 %[[VAL_A]], 5
 ; OLDDBG-NEXT: call void @llvm.dbg.value(metadata !DIArgList(i32 %[[VAL_A]], i32 %[[VAL_ADD]]), metadata ![[VAR_A]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg ![[LOC_3:[0-9]+]]
 ; NEWDBG-NEXT: {{^}}    #dbg_value(!DIArgList(i32 %[[VAL_A]], i32 %[[VAL_ADD]]), ![[VAR_A]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus), ![[LOC_3:[0-9]+]])
-; OLDDBG-NEXT: call void @llvm.dbg.label(metadata ![[LABEL_ID:[0-9]+]])
-; NEWDBG-NEXT: {{^}}    #dbg_label(![[LABEL_ID:[0-9]+]])
+; OLDDBG-NEXT: call void @llvm.dbg.label(metadata ![[LABEL_ID:[0-9]+]]), !dbg ![[LOC_3]]
+; NEWDBG-NEXT: {{^}}    #dbg_label(![[LABEL_ID:[0-9]+]], ![[LOC_3]])
 ; CHECK-NEXT: {{^}}  store i32 %[[VAL_ADD]]{{.+}}, !DIAssignID ![[ASSIGNID:[0-9]+]]
 ; OLDDBG-NEXT: call void @llvm.dbg.assign(metadata i32 %[[VAL_ADD]], metadata ![[VAR_B]], metadata !DIExpression(), metadata ![[ASSIGNID]], metadata ptr %[[VAL_B]], metadata !DIExpression()), !dbg ![[LOC_4:[0-9]+]]
 ; NEWDBG-NEXT: {{^}}    #dbg_assign(i32 %[[VAL_ADD]], ![[VAR_B]], !DIExpression(), ![[ASSIGNID]], ptr %[[VAL_B]], !DIExpression(), ![[LOC_4:[0-9]+]])



More information about the llvm-commits mailing list