Skip to content

[DebugInfo][RemoveDIs] Eliminate another debug-info variation flag #133917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 9, 2025

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Apr 1, 2025

The "preserve input debug-info format" flag allowed some tooling to opt into not seeing the new debug records yet, and to not autoupgrade. This was good at the time, but un-necessary now that we'll be ditching intrinsics shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag to on, and as a result it hasn't seen debug records before. Thus, we missed a uselistorder variation: constant-expressions such as GEPs can be contained within debug records and completely isolated from the value hierachy, see the metadata-use-uselistorder.ll test. These Values didn't get ordered, but were legitimate uses of constants like "i64 0", and we now run into difficulty handling that. The patch to AsmWriter seeks Values to order even through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we can just delete, such as one in llvm-reduce and another few in the LocalTest unit tests. For the fast-isel test, it was added in https://reviews.llvm.org/D67703 explicitly for checking the size of blocks without debug-info and in 1525abb the codepath it tests moved towards being sunsetted. It'll be totally redundant once RemoveDIs is on permanently.

Note that there's now no explicit test for the textual-IR autoupgrade path. I submit that we can rely on the thousands of .ll files where we've only been bothered to update the outputs, not the inputs, to debug records.

The "preserve input debug-info format" flag allowed some tooling to opt
into not seeing the new debug records yet, and to not autoupgrade. This was
good at the time, but un-necessary now that we'll be ditching intrinsics
shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag to
on, and as a result it hasn't seen debug records before. Thus, we missed a
uselistorder variation: constant-expressions such as GEPs can be contained
within debug records and completely isolated from the value hierachy, see
the metadata-use-uselistorder.ll test. These Values didn't get ordered, but
were legitimate uses of constants like "i64 0", and we now run into
difficulty handling that. The patch to AsmWriter seeks Values to order even
through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we can
just delete, such as one in llvm-reduce and another few in the LocalTest
unit tests. For the fast-isel test, it was added in
https://reviews.llvm.org/D67703 explicitly for checking the size of blocks
without debug-info and in 1525abb the codepath it tests moved towards
being sunsetted. It'll be totally redundant once RemoveDIs is on
permanently.

Note that there's now no explicit test for the textual-IR autoupgrade path.
I submit that we can rely on the thousands of .ll files where we've only
been bothered to update the outputs, not the inputs, to debug records.
@jmorse jmorse requested review from SLTozer and OCHyams April 1, 2025 14:21
@jmorse
Copy link
Member Author

jmorse commented Apr 1, 2025

This patch could in theory be split into three: removing tests, fixing the verify-uselistorder thing, and removing the flag. However, there's some subtle connection between verify-uselistorder having an autoupgrade happening under its feet and the flag being removed; and the test-deletions should really land at the same time as the flag deletion. Sorry I couldn't make it any smaller!

Copy link

github-actions bot commented Apr 1, 2025

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

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of nits, otherwise LGTM.

Comment on lines 183 to 184
for (const Value *V : VariableRecord->location_ops())
orderValue(V, OM);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to check if it's an assign-kind and do the same for the address? (note to self: we still haven't added an "iterator over all the values" iterator).

ASSERT_TRUE(DVI);
It = std::next(It);
Instruction *RetInst = &*It;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this second half of the test still useful? Should we just delete the intrinsic part of the test, and change the intrisic to a DbgRecord in the IR?

@llvmbot llvmbot added backend:AArch64 llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Apr 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2025

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Jeremy Morse (jmorse)

Changes

The "preserve input debug-info format" flag allowed some tooling to opt into not seeing the new debug records yet, and to not autoupgrade. This was good at the time, but un-necessary now that we'll be ditching intrinsics shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag to on, and as a result it hasn't seen debug records before. Thus, we missed a uselistorder variation: constant-expressions such as GEPs can be contained within debug records and completely isolated from the value hierachy, see the metadata-use-uselistorder.ll test. These Values didn't get ordered, but were legitimate uses of constants like "i64 0", and we now run into difficulty handling that. The patch to AsmWriter seeks Values to order even through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we can just delete, such as one in llvm-reduce and another few in the LocalTest unit tests. For the fast-isel test, it was added in https://reviews.llvm.org/D67703 explicitly for checking the size of blocks without debug-info and in 1525abb the codepath it tests moved towards being sunsetted. It'll be totally redundant once RemoveDIs is on permanently.

Note that there's now no explicit test for the textual-IR autoupgrade path. I submit that we can rely on the thousands of .ll files where we've only been bothered to update the outputs, not the inputs, to debug records.


Patch is 30.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133917.diff

18 Files Affected:

  • (modified) llvm/lib/AsmParser/LLParser.cpp (+1-7)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2-35)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+24)
  • (modified) llvm/lib/IR/BasicBlock.cpp (-7)
  • (modified) llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll (+1-1)
  • (modified) llvm/test/Assembler/DIDefaultTemplateParam.ll (+2-2)
  • (modified) llvm/test/Assembler/metadata-use-uselistorder.ll (+10-6)
  • (modified) llvm/test/Bitcode/dbg-record-roundtrip.ll (+17-18)
  • (removed) llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll (-45)
  • (modified) llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll (+1-1)
  • (removed) llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll (-11)
  • (modified) llvm/tools/llvm-link/llvm-link.cpp (-6)
  • (modified) llvm/tools/llvm-lto/llvm-lto.cpp (-7)
  • (modified) llvm/tools/llvm-lto2/llvm-lto2.cpp (-7)
  • (modified) llvm/tools/llvm-reduce/llvm-reduce.cpp (-3)
  • (modified) llvm/tools/verify-uselistorder/verify-uselistorder.cpp (-3)
  • (modified) llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp (-1)
  • (modified) llvm/unittests/Transforms/Utils/LocalTest.cpp (+7-78)
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index ff2cdea6e8ee6..af0422f09bc4f 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -64,7 +64,6 @@ static cl::opt<bool> AllowIncompleteIR(
         "metadata will be dropped)"));
 
 extern llvm::cl::opt<bool> UseNewDbgInfoFormat;
-extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
 static std::string getTypeString(Type *T) {
   std::string Result;
@@ -209,10 +208,6 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
   // records in this IR.
   assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) &&
          "Mixed debug intrinsics/records seen without a parsing error?");
-  if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
-    UseNewDbgInfoFormat = SeenNewDbgInfoFormat;
-    M->setNewDbgInfoFormatFlag(SeenNewDbgInfoFormat);
-  }
 
   // Handle any function attribute group forward references.
   for (const auto &RAG : ForwardRefAttrGroups) {
@@ -447,8 +442,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
   UpgradeNVVMAnnotations(*M);
   UpgradeSectionAttributes(*M);
 
-  if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE)
-    M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);
+  M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);
 
   if (!Slots)
     return false;
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 4de3c4f085ca7..5c62ef4ad8e4e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -103,7 +103,6 @@ static cl::opt<bool> ExpandConstantExprs(
         "Expand constant expressions to instructions for testing purposes"));
 
 extern cl::opt<bool> UseNewDbgInfoFormat;
-extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
 namespace {
 
@@ -3953,11 +3952,7 @@ Error BitcodeReader::globalCleanup() {
   for (Function &F : *TheModule) {
     MDLoader->upgradeDebugIntrinsics(F);
     Function *NewFn;
-    // If PreserveInputDbgFormat=true, then we don't know whether we want
-    // intrinsics or records, and we won't perform any conversions in either
-    // case, so don't upgrade intrinsics to records.
-    if (UpgradeIntrinsicFunction(
-            &F, NewFn, PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE))
+    if (UpgradeIntrinsicFunction(&F, NewFn))
       UpgradedIntrinsics[&F] = NewFn;
     // Look for functions that rely on old function attribute behavior.
     UpgradeFunctionAttributes(F);
@@ -7002,37 +6997,9 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
   F->setIsMaterializable(false);
 
   // All parsed Functions should load into the debug info format dictated by the
-  // Module, unless we're attempting to preserve the input debug info format.
+  // Module.
   if (SeenDebugIntrinsic && SeenDebugRecord)
     return error("Mixed debug intrinsics and debug records in bitcode module!");
-  if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
-    bool SeenAnyDebugInfo = SeenDebugIntrinsic || SeenDebugRecord;
-    bool NewDbgInfoFormatDesired =
-        SeenAnyDebugInfo ? SeenDebugRecord : F->getParent()->IsNewDbgInfoFormat;
-    if (SeenAnyDebugInfo) {
-      UseNewDbgInfoFormat = SeenDebugRecord;
-    }
-    // If the module's debug info format doesn't match the observed input
-    // format, then set its format now; we don't need to call the conversion
-    // function because there must be no existing intrinsics to convert.
-    // Otherwise, just set the format on this function now.
-    if (NewDbgInfoFormatDesired != F->getParent()->IsNewDbgInfoFormat)
-      F->getParent()->setNewDbgInfoFormatFlag(NewDbgInfoFormatDesired);
-    else
-      F->setNewDbgInfoFormatFlag(NewDbgInfoFormatDesired);
-  } else {
-    // If we aren't preserving formats, we use the Module flag to get our
-    // desired format instead of reading flags, in case we are lazy-loading and
-    // the format of the module has been changed since it was set by the flags.
-    // We only need to convert debug info here if we have debug records but
-    // desire the intrinsic format; everything else is a no-op or handled by the
-    // autoupgrader.
-    bool ModuleIsNewDbgInfoFormat = F->getParent()->IsNewDbgInfoFormat;
-    if (ModuleIsNewDbgInfoFormat || !SeenDebugRecord)
-      F->setNewDbgInfoFormatFlag(ModuleIsNewDbgInfoFormat);
-    else
-      F->setIsNewDbgInfoFormat(ModuleIsNewDbgInfoFormat);
-  }
 
   if (StripDebugInfo)
     stripDebugInfo(*F);
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 5f0a9cdfb941a..ab7f32a70c517 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -140,6 +140,20 @@ static void orderValue(const Value *V, OrderMap &OM) {
 static OrderMap orderModule(const Module *M) {
   OrderMap OM;
 
+  auto orderConstantValue = [&OM](const Value *V) {
+    if (isa<Constant>(V) || isa<InlineAsm>(V))
+      orderValue(V, OM);
+  };
+
+  auto OrderConstantFromMetadata = [&](Metadata *MD) {
+      if (const auto *VAM = dyn_cast<ValueAsMetadata>(MD)) {
+        orderConstantValue(VAM->getValue());
+      } else if (const auto *AL = dyn_cast<DIArgList>(MD)) {
+        for (const auto *VAM : AL->getArgs())
+          orderConstantValue(VAM->getValue());
+      }
+    };
+
   for (const GlobalVariable &G : M->globals()) {
     if (G.hasInitializer())
       if (!isa<GlobalValue>(G.getInitializer()))
@@ -171,6 +185,16 @@ static OrderMap orderModule(const Module *M) {
     for (const BasicBlock &BB : F) {
       orderValue(&BB, OM);
       for (const Instruction &I : BB) {
+        // Debug records can contain Value references, that can then contain
+        // Values disconnected from the rest of the Value hierachy, if wrapped
+        // in some kind of constant-expression. Find and order any Values that
+        // are wrapped in debug-info.
+        for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange())) {
+          OrderConstantFromMetadata(DVR.getRawLocation());
+          if (DVR.isDbgAssign())
+            OrderConstantFromMetadata(DVR.getRawAddress());
+        }
+
         for (const Value *Op : I.operands()) {
           Op = skipMetadataWrapper(Op);
           if ((isa<Constant>(*Op) && !isa<GlobalValue>(*Op)) ||
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index d9ff0687480a4..aa3c2cfa056fe 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -36,13 +36,6 @@ cl::opt<bool> UseNewDbgInfoFormat(
              "eliminating intrinsics. Has no effect if "
              "--preserve-input-debuginfo-format=true."),
     cl::init(true));
-cl::opt<cl::boolOrDefault> PreserveInputDbgFormat(
-    "preserve-input-debuginfo-format", cl::Hidden,
-    cl::desc("When set to true, IR files will be processed and printed in "
-             "their current debug info format, regardless of default behaviour "
-             "or other flags passed. Has no effect if input IR does not "
-             "contain debug records or intrinsics. Ignored in llvm-link, "
-             "llvm-lto, and llvm-lto2."));
 
 DbgMarker *BasicBlock::createMarker(Instruction *I) {
   assert(IsNewDbgInfoFormat &&
diff --git a/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll b/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
index 04927974a708b..bb51192c43c2e 100644
--- a/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
+++ b/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
@@ -13,7 +13,7 @@ target triple = "x86_64-apple-darwin10.2"
 define i32 @main() nounwind readonly !dbg !1 {
   %diff1 = alloca i64                             ; <ptr> [#uses=2]
 ; CHECK: #dbg_value(i64 72,
-  call void @llvm.dbg.declare(metadata ptr %diff1, metadata !0, metadata !DIExpression()), !dbg !DILocation(scope: !1)
+    #dbg_declare(ptr %diff1, !0, !DIExpression(), !DILocation(scope: !1))
   store i64 72, ptr %diff1, align 8
   %v1 = load ptr, ptr @TestArrayPtr, align 8 ; <ptr> [#uses=1]
   %v2 = ptrtoint ptr %v1 to i64 ; <i64> [#uses=1]
diff --git a/llvm/test/Assembler/DIDefaultTemplateParam.ll b/llvm/test/Assembler/DIDefaultTemplateParam.ll
index 015da31629ad8..f4e3a924f6575 100644
--- a/llvm/test/Assembler/DIDefaultTemplateParam.ll
+++ b/llvm/test/Assembler/DIDefaultTemplateParam.ll
@@ -15,8 +15,8 @@ entry:
   %f1 = alloca %class.foo, align 1
   %f2 = alloca %class.foo.0, align 1
   store i32 0, ptr %retval, align 4
-  call void @llvm.dbg.declare(metadata ptr %f1, metadata !11, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.declare(metadata ptr %f2, metadata !17, metadata !DIExpression()), !dbg !23
+    #dbg_declare(ptr %f1, !11, !DIExpression(), !16)
+    #dbg_declare(ptr %f2, !17, !DIExpression(), !23)
   ret i32 0, !dbg !24
 }
 ; Function Attrs: nounwind readnone speculatable willreturn
diff --git a/llvm/test/Assembler/metadata-use-uselistorder.ll b/llvm/test/Assembler/metadata-use-uselistorder.ll
index b91eadd5b6f68..e33a92deb2cac 100644
--- a/llvm/test/Assembler/metadata-use-uselistorder.ll
+++ b/llvm/test/Assembler/metadata-use-uselistorder.ll
@@ -7,6 +7,11 @@
 ; correctly preserved due to the uses in the dbg.value contant expressions not
 ; being considered, since they are wrapped in metadata.
 
+; With debug records (i.e., not with intrinsics) there's less chance of
+; debug-info affecting use-list order as it exists outside of the Value
+; hierachy. However, debug records still use ValueAsMetadata nodes, so this
+; test remains worthwhile.
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -16,21 +21,19 @@ target triple = "x86_64-unknown-linux-gnu"
 define void @foo() local_unnamed_addr !dbg !6 {
 entry:
   %0 = load i64, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 4), align 16
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 5), metadata !10, metadata !DIExpression()), !dbg !13
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 5), !10, !DIExpression(), !13)
   %1 = load i64, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), align 16
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), metadata !10, metadata !DIExpression()), !dbg !14
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), !10, !DIExpression(), !14)
+    #dbg_assign(i32 0, !10, !DIExpression(), !19, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), !DIExpression(), !14)
   ret void
 }
 
 define void @bar() local_unnamed_addr !dbg !15 {
 entry:
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 7), metadata !17, metadata !DIExpression()), !dbg !18
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 7), !17, !DIExpression(), !18)
   ret void
 }
 
-; Function Attrs: nounwind readnone speculatable
-declare void @llvm.dbg.value(metadata, metadata, metadata) #0
-
 attributes #0 = { nounwind readnone speculatable }
 
 !llvm.dbg.cu = !{!0}
@@ -56,3 +59,4 @@ attributes #0 = { nounwind readnone speculatable }
 !16 = !{!17}
 !17 = !DILocalVariable(name: "local2", scope: !15, file: !1, line: 13, type: !11)
 !18 = !DILocation(line: 14, column: 1, scope: !15)
+!19 = distinct !DIAssignID()
diff --git a/llvm/test/Bitcode/dbg-record-roundtrip.ll b/llvm/test/Bitcode/dbg-record-roundtrip.ll
index 95b5b913af001..a75c42b2459e5 100644
--- a/llvm/test/Bitcode/dbg-record-roundtrip.ll
+++ b/llvm/test/Bitcode/dbg-record-roundtrip.ll
@@ -1,12 +1,10 @@
 ;; Roundtrip tests.
 
-;; Tests that bitcode can be printed and interpreted by llvm-dis with non-intrinsic
-;; debug records -- llvm-as will autoupgrade.
 ; RUN: llvm-as %s -o - \
 ; RUN: | llvm-dis \
 ; RUN: | FileCheck %s --check-prefixes=RECORDS
 
-;; Check that verify-uselistorder passes regardless of input format.
+;; Check that verify-uselistorder passes with bitcode and textual IR.
 ; RUN: llvm-as %s -o - | verify-uselistorder
 ; RUN: verify-uselistorder %s
 
@@ -31,17 +29,18 @@ entry:
 ; RECORDS-NEXT: dbg_value(![[empty:[0-9]+]], ![[e]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_value(i32 poison, ![[e]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_value(i32 1, ![[f:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %p, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata !29, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata i32 poison, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata i32 1, metadata !33, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %p, !32, !DIExpression(), !19)
+    #dbg_value(!29, !32, !DIExpression(), !19)
+    #dbg_value(i32 poison, !32, !DIExpression(), !19)
+    #dbg_value(i32 1, !33, !DIExpression(), !19)
 ;; Arglist with an argument, constant, local use before def, poison.
 ; RECORDS-NEXT: dbg_value(!DIArgList(i32 %p, i32 0, i32 %0, i32 poison), ![[f]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata !DIArgList(i32 %p, i32 0, i32 %0, i32 poison), metadata !33, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus)), !dbg !19
+    #dbg_value(!DIArgList(i32 %p, i32 0, i32 %0, i32 poison), !33, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus), !19)
+
 ;; Check dbg.assign use before def (value, addr and ID). Check expression order too.
 ; RECORDS: dbg_assign(i32 %0, ![[i:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 0),
 ; RECORDS-SAME:       ![[ID:[0-9]+]], ptr %a, !DIExpression(DW_OP_plus_uconst, 1), ![[dbg]])
-  tail call void @llvm.dbg.assign(metadata i32 %0, metadata !36, metadata !DIExpression(DW_OP_plus_uconst, 0), metadata !37, metadata ptr %a, metadata !DIExpression(DW_OP_plus_uconst, 1)), !dbg !19
+    #dbg_assign(i32 %0, !36, !DIExpression(DW_OP_plus_uconst, 0), !37, ptr %a, !DIExpression(DW_OP_plus_uconst, 1), !19)
   %a = alloca i32, align 4, !DIAssignID !37
 ; CHECK: %a = alloca i32, align 4, !DIAssignID ![[ID]]
 ;; Check dbg.declare configurations.
@@ -51,25 +50,25 @@ entry:
 ; RECORDS-NEXT: dbg_declare(ptr poison, ![[c:[0-9]+]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_declare(ptr null, ![[d:[0-9]+]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_declare(ptr @g, ![[h:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.declare(metadata ptr %a, metadata !17, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata !29, metadata !28, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr poison, metadata !30, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr null, metadata !31, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr @g, metadata !35, metadata !DIExpression()), !dbg !19
+    #dbg_declare(ptr %a, !17, !DIExpression(), !19)
+    #dbg_declare(!29, !28, !DIExpression(), !19)
+    #dbg_declare(ptr poison, !30, !DIExpression(), !19)
+    #dbg_declare(ptr null, !31, !DIExpression(), !19)
+    #dbg_declare(ptr @g, !35, !DIExpression(), !19)
 ;; Argument value dbg.declare.
 ; RECORDS: dbg_declare(ptr %storage, ![[g:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.declare(metadata ptr %storage, metadata !34, metadata !DIExpression()), !dbg !19
+    #dbg_declare(ptr %storage, !34, !DIExpression(), !19)
 ;; Use before def dbg.value.
 ; RECORDS: dbg_value(i32 %0, ![[e]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %0, metadata !32, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %0, !32, !DIExpression(), !19)
   %0 = load i32, ptr @g, align 4, !dbg !20
 ;; Non-argument local value dbg.value.
 ; RECORDS: dbg_value(i32 %0, ![[e]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %0, metadata !32, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %0, !32, !DIExpression(), !19)
   store i32 %0, ptr %a, align 4, !dbg !19
   %1 = load i32, ptr %a, align 4, !dbg !25
 ; RECORDS: dbg_label(![[label:[0-9]+]], ![[dbg]])
-  tail call void @llvm.dbg.label(metadata !38), !dbg !19
+    #dbg_label(!38, !19)
   ret i32 %1, !dbg !27
 }
 
diff --git a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll b/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
deleted file mode 100644
index 5ce7bb04c5518..0000000000000
--- a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
+++ /dev/null
@@ -1,45 +0,0 @@
-; RUN: llc -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s  2> /dev/null | FileCheck %s
-; RUN: llc --preserve-input-debuginfo-format=true -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s  2> /dev/null | FileCheck %s
-
-define dso_local i32 @a() #0 !dbg !7 {
-entry:
-; CHECK:    b       .LBB0_1
-; CHECK:  .LBB0_1:
-  call void @llvm.dbg.value(metadata i32 0, metadata !12, metadata !DIExpression()), !dbg !13
-  br label %for.cond, !dbg !14
-
-; CHECK:    b       .LBB0_1
-; CHECK:  .Lfunc_end0:
-for.cond:
-  br label %for.cond, !dbg !15, !llvm.loop !18
-}
-declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #1
-
-declare void @llvm.dbg.value(metadata, metadata, metadata) #2
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!3, !4, !5}
-!llvm.ident = !{!6}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
-!1 = !DIFile(filename: "fast-isel-branch-uncond-debug.ll", directory: "/tmp")
-!2 = !{}
-!3 = !{i32 2, !"Dwarf Version", i32 4}
-!4 = !{i32 2, !"Debug Info Version", i32 3}
-!5 = !{i32 1, !"wchar_size", i32 4}
-!6 = !{!""}
-!7 = distinct !DISubprogram(name: "a", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
-!8 = !DISubroutineType(types: !9)
-!9 = !{!10}
-!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!11 = !{!12}
-!12 = !DILocalVariable(name: "b", scope: !7, file: !1, line: 2, type: !10)
-!13 = !DILocation(line: 0, scope: !7)
-!14 = !DILocation(line: 3, column: 3, scope: !7)
-!15 = !DILocation(line: 3, column: 3, scope: !16)
-!16 = distinct !DILexicalBlock(scope: !17, file: !1, line: 3, column: 3)
-!17 = distinct !DILexicalBlock(scope: !7, file: !1, line: 3, column: 3)
-!18 = distinct !{!18, !19, !20}
-!19 = !DILocation(line: 3, column: 3, scope: !17)
-!20 = !DILocation(line: 4, column: 5, scope: !17)
-
diff --git a/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll b/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
index 82d31872ffbc2..ac6bd2f934efc 100644
--- a/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
+++ b/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
@@ -1,7 +1,7 @@
 ; RUN: llvm-reduce %s -o %t --delta-passes=metadata --test %python --test-arg %p/Inputs/remove-metadata.py --abort-on-invalid-reduction
 ; RUN: FileCheck %s --input-file %t
 
-; CHECK: call void @llvm.dbg.declare{{.*}}, !dbg
+; CHECK: #dbg_declare
 ; CHECK: !llvm.dbg.cu = !{!0}
 ; CHECK-NOT: uninteresting
 
diff --git a/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll b/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll
deleted file mode 100644
index 2b75cd5d6c3da..0000000000000
--- a/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll
+++ /dev/null
@@ -1,11 +0,0 @@
-; llvm-reduce shouldn't remove arguments of debug intrinsics, because the resulting module will be ill-formed.
-;
-; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=arguments --test FileCheck --test-arg --check-prefixes=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s --output %t
-; RUN: FileCheck --check-prefix=CHECK-FINAL %s < %t
-
-; CHECK-INTERESTINGNESS: declare void @llvm.dbg.declare
-; CHECK-FINAL: declare void @llvm.dbg.declare(metadata, metadata, metadata)
-declare void @llvm.dbg.declare(metadata, metadata, metadata)
-; CHECK-INTERESTINGNESS: declare void @llvm.dbg.value
-; CHECK-FINAL: declare ...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2025

@llvm/pr-subscribers-llvm-ir

Author: Jeremy Morse (jmorse)

Changes

The "preserve input debug-info format" flag allowed some tooling to opt into not seeing the new debug records yet, and to not autoupgrade. This was good at the time, but un-necessary now that we'll be ditching intrinsics shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag to on, and as a result it hasn't seen debug records before. Thus, we missed a uselistorder variation: constant-expressions such as GEPs can be contained within debug records and completely isolated from the value hierachy, see the metadata-use-uselistorder.ll test. These Values didn't get ordered, but were legitimate uses of constants like "i64 0", and we now run into difficulty handling that. The patch to AsmWriter seeks Values to order even through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we can just delete, such as one in llvm-reduce and another few in the LocalTest unit tests. For the fast-isel test, it was added in https://reviews.llvm.org/D67703 explicitly for checking the size of blocks without debug-info and in 1525abb the codepath it tests moved towards being sunsetted. It'll be totally redundant once RemoveDIs is on permanently.

Note that there's now no explicit test for the textual-IR autoupgrade path. I submit that we can rely on the thousands of .ll files where we've only been bothered to update the outputs, not the inputs, to debug records.


Patch is 30.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133917.diff

18 Files Affected:

  • (modified) llvm/lib/AsmParser/LLParser.cpp (+1-7)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2-35)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+24)
  • (modified) llvm/lib/IR/BasicBlock.cpp (-7)
  • (modified) llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll (+1-1)
  • (modified) llvm/test/Assembler/DIDefaultTemplateParam.ll (+2-2)
  • (modified) llvm/test/Assembler/metadata-use-uselistorder.ll (+10-6)
  • (modified) llvm/test/Bitcode/dbg-record-roundtrip.ll (+17-18)
  • (removed) llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll (-45)
  • (modified) llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll (+1-1)
  • (removed) llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll (-11)
  • (modified) llvm/tools/llvm-link/llvm-link.cpp (-6)
  • (modified) llvm/tools/llvm-lto/llvm-lto.cpp (-7)
  • (modified) llvm/tools/llvm-lto2/llvm-lto2.cpp (-7)
  • (modified) llvm/tools/llvm-reduce/llvm-reduce.cpp (-3)
  • (modified) llvm/tools/verify-uselistorder/verify-uselistorder.cpp (-3)
  • (modified) llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp (-1)
  • (modified) llvm/unittests/Transforms/Utils/LocalTest.cpp (+7-78)
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index ff2cdea6e8ee6..af0422f09bc4f 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -64,7 +64,6 @@ static cl::opt<bool> AllowIncompleteIR(
         "metadata will be dropped)"));
 
 extern llvm::cl::opt<bool> UseNewDbgInfoFormat;
-extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
 static std::string getTypeString(Type *T) {
   std::string Result;
@@ -209,10 +208,6 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
   // records in this IR.
   assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) &&
          "Mixed debug intrinsics/records seen without a parsing error?");
-  if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
-    UseNewDbgInfoFormat = SeenNewDbgInfoFormat;
-    M->setNewDbgInfoFormatFlag(SeenNewDbgInfoFormat);
-  }
 
   // Handle any function attribute group forward references.
   for (const auto &RAG : ForwardRefAttrGroups) {
@@ -447,8 +442,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
   UpgradeNVVMAnnotations(*M);
   UpgradeSectionAttributes(*M);
 
-  if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE)
-    M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);
+  M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);
 
   if (!Slots)
     return false;
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 4de3c4f085ca7..5c62ef4ad8e4e 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -103,7 +103,6 @@ static cl::opt<bool> ExpandConstantExprs(
         "Expand constant expressions to instructions for testing purposes"));
 
 extern cl::opt<bool> UseNewDbgInfoFormat;
-extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
 namespace {
 
@@ -3953,11 +3952,7 @@ Error BitcodeReader::globalCleanup() {
   for (Function &F : *TheModule) {
     MDLoader->upgradeDebugIntrinsics(F);
     Function *NewFn;
-    // If PreserveInputDbgFormat=true, then we don't know whether we want
-    // intrinsics or records, and we won't perform any conversions in either
-    // case, so don't upgrade intrinsics to records.
-    if (UpgradeIntrinsicFunction(
-            &F, NewFn, PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE))
+    if (UpgradeIntrinsicFunction(&F, NewFn))
       UpgradedIntrinsics[&F] = NewFn;
     // Look for functions that rely on old function attribute behavior.
     UpgradeFunctionAttributes(F);
@@ -7002,37 +6997,9 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
   F->setIsMaterializable(false);
 
   // All parsed Functions should load into the debug info format dictated by the
-  // Module, unless we're attempting to preserve the input debug info format.
+  // Module.
   if (SeenDebugIntrinsic && SeenDebugRecord)
     return error("Mixed debug intrinsics and debug records in bitcode module!");
-  if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) {
-    bool SeenAnyDebugInfo = SeenDebugIntrinsic || SeenDebugRecord;
-    bool NewDbgInfoFormatDesired =
-        SeenAnyDebugInfo ? SeenDebugRecord : F->getParent()->IsNewDbgInfoFormat;
-    if (SeenAnyDebugInfo) {
-      UseNewDbgInfoFormat = SeenDebugRecord;
-    }
-    // If the module's debug info format doesn't match the observed input
-    // format, then set its format now; we don't need to call the conversion
-    // function because there must be no existing intrinsics to convert.
-    // Otherwise, just set the format on this function now.
-    if (NewDbgInfoFormatDesired != F->getParent()->IsNewDbgInfoFormat)
-      F->getParent()->setNewDbgInfoFormatFlag(NewDbgInfoFormatDesired);
-    else
-      F->setNewDbgInfoFormatFlag(NewDbgInfoFormatDesired);
-  } else {
-    // If we aren't preserving formats, we use the Module flag to get our
-    // desired format instead of reading flags, in case we are lazy-loading and
-    // the format of the module has been changed since it was set by the flags.
-    // We only need to convert debug info here if we have debug records but
-    // desire the intrinsic format; everything else is a no-op or handled by the
-    // autoupgrader.
-    bool ModuleIsNewDbgInfoFormat = F->getParent()->IsNewDbgInfoFormat;
-    if (ModuleIsNewDbgInfoFormat || !SeenDebugRecord)
-      F->setNewDbgInfoFormatFlag(ModuleIsNewDbgInfoFormat);
-    else
-      F->setIsNewDbgInfoFormat(ModuleIsNewDbgInfoFormat);
-  }
 
   if (StripDebugInfo)
     stripDebugInfo(*F);
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 5f0a9cdfb941a..ab7f32a70c517 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -140,6 +140,20 @@ static void orderValue(const Value *V, OrderMap &OM) {
 static OrderMap orderModule(const Module *M) {
   OrderMap OM;
 
+  auto orderConstantValue = [&OM](const Value *V) {
+    if (isa<Constant>(V) || isa<InlineAsm>(V))
+      orderValue(V, OM);
+  };
+
+  auto OrderConstantFromMetadata = [&](Metadata *MD) {
+      if (const auto *VAM = dyn_cast<ValueAsMetadata>(MD)) {
+        orderConstantValue(VAM->getValue());
+      } else if (const auto *AL = dyn_cast<DIArgList>(MD)) {
+        for (const auto *VAM : AL->getArgs())
+          orderConstantValue(VAM->getValue());
+      }
+    };
+
   for (const GlobalVariable &G : M->globals()) {
     if (G.hasInitializer())
       if (!isa<GlobalValue>(G.getInitializer()))
@@ -171,6 +185,16 @@ static OrderMap orderModule(const Module *M) {
     for (const BasicBlock &BB : F) {
       orderValue(&BB, OM);
       for (const Instruction &I : BB) {
+        // Debug records can contain Value references, that can then contain
+        // Values disconnected from the rest of the Value hierachy, if wrapped
+        // in some kind of constant-expression. Find and order any Values that
+        // are wrapped in debug-info.
+        for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange())) {
+          OrderConstantFromMetadata(DVR.getRawLocation());
+          if (DVR.isDbgAssign())
+            OrderConstantFromMetadata(DVR.getRawAddress());
+        }
+
         for (const Value *Op : I.operands()) {
           Op = skipMetadataWrapper(Op);
           if ((isa<Constant>(*Op) && !isa<GlobalValue>(*Op)) ||
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index d9ff0687480a4..aa3c2cfa056fe 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -36,13 +36,6 @@ cl::opt<bool> UseNewDbgInfoFormat(
              "eliminating intrinsics. Has no effect if "
              "--preserve-input-debuginfo-format=true."),
     cl::init(true));
-cl::opt<cl::boolOrDefault> PreserveInputDbgFormat(
-    "preserve-input-debuginfo-format", cl::Hidden,
-    cl::desc("When set to true, IR files will be processed and printed in "
-             "their current debug info format, regardless of default behaviour "
-             "or other flags passed. Has no effect if input IR does not "
-             "contain debug records or intrinsics. Ignored in llvm-link, "
-             "llvm-lto, and llvm-lto2."));
 
 DbgMarker *BasicBlock::createMarker(Instruction *I) {
   assert(IsNewDbgInfoFormat &&
diff --git a/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll b/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
index 04927974a708b..bb51192c43c2e 100644
--- a/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
+++ b/llvm/test/Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll
@@ -13,7 +13,7 @@ target triple = "x86_64-apple-darwin10.2"
 define i32 @main() nounwind readonly !dbg !1 {
   %diff1 = alloca i64                             ; <ptr> [#uses=2]
 ; CHECK: #dbg_value(i64 72,
-  call void @llvm.dbg.declare(metadata ptr %diff1, metadata !0, metadata !DIExpression()), !dbg !DILocation(scope: !1)
+    #dbg_declare(ptr %diff1, !0, !DIExpression(), !DILocation(scope: !1))
   store i64 72, ptr %diff1, align 8
   %v1 = load ptr, ptr @TestArrayPtr, align 8 ; <ptr> [#uses=1]
   %v2 = ptrtoint ptr %v1 to i64 ; <i64> [#uses=1]
diff --git a/llvm/test/Assembler/DIDefaultTemplateParam.ll b/llvm/test/Assembler/DIDefaultTemplateParam.ll
index 015da31629ad8..f4e3a924f6575 100644
--- a/llvm/test/Assembler/DIDefaultTemplateParam.ll
+++ b/llvm/test/Assembler/DIDefaultTemplateParam.ll
@@ -15,8 +15,8 @@ entry:
   %f1 = alloca %class.foo, align 1
   %f2 = alloca %class.foo.0, align 1
   store i32 0, ptr %retval, align 4
-  call void @llvm.dbg.declare(metadata ptr %f1, metadata !11, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.declare(metadata ptr %f2, metadata !17, metadata !DIExpression()), !dbg !23
+    #dbg_declare(ptr %f1, !11, !DIExpression(), !16)
+    #dbg_declare(ptr %f2, !17, !DIExpression(), !23)
   ret i32 0, !dbg !24
 }
 ; Function Attrs: nounwind readnone speculatable willreturn
diff --git a/llvm/test/Assembler/metadata-use-uselistorder.ll b/llvm/test/Assembler/metadata-use-uselistorder.ll
index b91eadd5b6f68..e33a92deb2cac 100644
--- a/llvm/test/Assembler/metadata-use-uselistorder.ll
+++ b/llvm/test/Assembler/metadata-use-uselistorder.ll
@@ -7,6 +7,11 @@
 ; correctly preserved due to the uses in the dbg.value contant expressions not
 ; being considered, since they are wrapped in metadata.
 
+; With debug records (i.e., not with intrinsics) there's less chance of
+; debug-info affecting use-list order as it exists outside of the Value
+; hierachy. However, debug records still use ValueAsMetadata nodes, so this
+; test remains worthwhile.
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -16,21 +21,19 @@ target triple = "x86_64-unknown-linux-gnu"
 define void @foo() local_unnamed_addr !dbg !6 {
 entry:
   %0 = load i64, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 4), align 16
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 5), metadata !10, metadata !DIExpression()), !dbg !13
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 5), !10, !DIExpression(), !13)
   %1 = load i64, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), align 16
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), metadata !10, metadata !DIExpression()), !dbg !14
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), !10, !DIExpression(), !14)
+    #dbg_assign(i32 0, !10, !DIExpression(), !19, ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 6), !DIExpression(), !14)
   ret void
 }
 
 define void @bar() local_unnamed_addr !dbg !15 {
 entry:
-  call void @llvm.dbg.value(metadata ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 7), metadata !17, metadata !DIExpression()), !dbg !18
+    #dbg_value(ptr getelementptr inbounds ([10 x i64], ptr @global_arr, i64 0, i64 7), !17, !DIExpression(), !18)
   ret void
 }
 
-; Function Attrs: nounwind readnone speculatable
-declare void @llvm.dbg.value(metadata, metadata, metadata) #0
-
 attributes #0 = { nounwind readnone speculatable }
 
 !llvm.dbg.cu = !{!0}
@@ -56,3 +59,4 @@ attributes #0 = { nounwind readnone speculatable }
 !16 = !{!17}
 !17 = !DILocalVariable(name: "local2", scope: !15, file: !1, line: 13, type: !11)
 !18 = !DILocation(line: 14, column: 1, scope: !15)
+!19 = distinct !DIAssignID()
diff --git a/llvm/test/Bitcode/dbg-record-roundtrip.ll b/llvm/test/Bitcode/dbg-record-roundtrip.ll
index 95b5b913af001..a75c42b2459e5 100644
--- a/llvm/test/Bitcode/dbg-record-roundtrip.ll
+++ b/llvm/test/Bitcode/dbg-record-roundtrip.ll
@@ -1,12 +1,10 @@
 ;; Roundtrip tests.
 
-;; Tests that bitcode can be printed and interpreted by llvm-dis with non-intrinsic
-;; debug records -- llvm-as will autoupgrade.
 ; RUN: llvm-as %s -o - \
 ; RUN: | llvm-dis \
 ; RUN: | FileCheck %s --check-prefixes=RECORDS
 
-;; Check that verify-uselistorder passes regardless of input format.
+;; Check that verify-uselistorder passes with bitcode and textual IR.
 ; RUN: llvm-as %s -o - | verify-uselistorder
 ; RUN: verify-uselistorder %s
 
@@ -31,17 +29,18 @@ entry:
 ; RECORDS-NEXT: dbg_value(![[empty:[0-9]+]], ![[e]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_value(i32 poison, ![[e]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_value(i32 1, ![[f:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %p, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata !29, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata i32 poison, metadata !32, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.value(metadata i32 1, metadata !33, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %p, !32, !DIExpression(), !19)
+    #dbg_value(!29, !32, !DIExpression(), !19)
+    #dbg_value(i32 poison, !32, !DIExpression(), !19)
+    #dbg_value(i32 1, !33, !DIExpression(), !19)
 ;; Arglist with an argument, constant, local use before def, poison.
 ; RECORDS-NEXT: dbg_value(!DIArgList(i32 %p, i32 0, i32 %0, i32 poison), ![[f]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata !DIArgList(i32 %p, i32 0, i32 %0, i32 poison), metadata !33, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus)), !dbg !19
+    #dbg_value(!DIArgList(i32 %p, i32 0, i32 %0, i32 poison), !33, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_minus), !19)
+
 ;; Check dbg.assign use before def (value, addr and ID). Check expression order too.
 ; RECORDS: dbg_assign(i32 %0, ![[i:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 0),
 ; RECORDS-SAME:       ![[ID:[0-9]+]], ptr %a, !DIExpression(DW_OP_plus_uconst, 1), ![[dbg]])
-  tail call void @llvm.dbg.assign(metadata i32 %0, metadata !36, metadata !DIExpression(DW_OP_plus_uconst, 0), metadata !37, metadata ptr %a, metadata !DIExpression(DW_OP_plus_uconst, 1)), !dbg !19
+    #dbg_assign(i32 %0, !36, !DIExpression(DW_OP_plus_uconst, 0), !37, ptr %a, !DIExpression(DW_OP_plus_uconst, 1), !19)
   %a = alloca i32, align 4, !DIAssignID !37
 ; CHECK: %a = alloca i32, align 4, !DIAssignID ![[ID]]
 ;; Check dbg.declare configurations.
@@ -51,25 +50,25 @@ entry:
 ; RECORDS-NEXT: dbg_declare(ptr poison, ![[c:[0-9]+]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_declare(ptr null, ![[d:[0-9]+]], !DIExpression(), ![[dbg]])
 ; RECORDS-NEXT: dbg_declare(ptr @g, ![[h:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.declare(metadata ptr %a, metadata !17, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata !29, metadata !28, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr poison, metadata !30, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr null, metadata !31, metadata !DIExpression()), !dbg !19
-  tail call void @llvm.dbg.declare(metadata ptr @g, metadata !35, metadata !DIExpression()), !dbg !19
+    #dbg_declare(ptr %a, !17, !DIExpression(), !19)
+    #dbg_declare(!29, !28, !DIExpression(), !19)
+    #dbg_declare(ptr poison, !30, !DIExpression(), !19)
+    #dbg_declare(ptr null, !31, !DIExpression(), !19)
+    #dbg_declare(ptr @g, !35, !DIExpression(), !19)
 ;; Argument value dbg.declare.
 ; RECORDS: dbg_declare(ptr %storage, ![[g:[0-9]+]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.declare(metadata ptr %storage, metadata !34, metadata !DIExpression()), !dbg !19
+    #dbg_declare(ptr %storage, !34, !DIExpression(), !19)
 ;; Use before def dbg.value.
 ; RECORDS: dbg_value(i32 %0, ![[e]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %0, metadata !32, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %0, !32, !DIExpression(), !19)
   %0 = load i32, ptr @g, align 4, !dbg !20
 ;; Non-argument local value dbg.value.
 ; RECORDS: dbg_value(i32 %0, ![[e]], !DIExpression(), ![[dbg]])
-  tail call void @llvm.dbg.value(metadata i32 %0, metadata !32, metadata !DIExpression()), !dbg !19
+    #dbg_value(i32 %0, !32, !DIExpression(), !19)
   store i32 %0, ptr %a, align 4, !dbg !19
   %1 = load i32, ptr %a, align 4, !dbg !25
 ; RECORDS: dbg_label(![[label:[0-9]+]], ![[dbg]])
-  tail call void @llvm.dbg.label(metadata !38), !dbg !19
+    #dbg_label(!38, !19)
   ret i32 %1, !dbg !27
 }
 
diff --git a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll b/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
deleted file mode 100644
index 5ce7bb04c5518..0000000000000
--- a/llvm/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll
+++ /dev/null
@@ -1,45 +0,0 @@
-; RUN: llc -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s  2> /dev/null | FileCheck %s
-; RUN: llc --preserve-input-debuginfo-format=true -mtriple=aarch64 -O1 -opt-bisect-limit=2 -o - %s  2> /dev/null | FileCheck %s
-
-define dso_local i32 @a() #0 !dbg !7 {
-entry:
-; CHECK:    b       .LBB0_1
-; CHECK:  .LBB0_1:
-  call void @llvm.dbg.value(metadata i32 0, metadata !12, metadata !DIExpression()), !dbg !13
-  br label %for.cond, !dbg !14
-
-; CHECK:    b       .LBB0_1
-; CHECK:  .Lfunc_end0:
-for.cond:
-  br label %for.cond, !dbg !15, !llvm.loop !18
-}
-declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #1
-
-declare void @llvm.dbg.value(metadata, metadata, metadata) #2
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!3, !4, !5}
-!llvm.ident = !{!6}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
-!1 = !DIFile(filename: "fast-isel-branch-uncond-debug.ll", directory: "/tmp")
-!2 = !{}
-!3 = !{i32 2, !"Dwarf Version", i32 4}
-!4 = !{i32 2, !"Debug Info Version", i32 3}
-!5 = !{i32 1, !"wchar_size", i32 4}
-!6 = !{!""}
-!7 = distinct !DISubprogram(name: "a", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
-!8 = !DISubroutineType(types: !9)
-!9 = !{!10}
-!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!11 = !{!12}
-!12 = !DILocalVariable(name: "b", scope: !7, file: !1, line: 2, type: !10)
-!13 = !DILocation(line: 0, scope: !7)
-!14 = !DILocation(line: 3, column: 3, scope: !7)
-!15 = !DILocation(line: 3, column: 3, scope: !16)
-!16 = distinct !DILexicalBlock(scope: !17, file: !1, line: 3, column: 3)
-!17 = distinct !DILexicalBlock(scope: !7, file: !1, line: 3, column: 3)
-!18 = distinct !{!18, !19, !20}
-!19 = !DILocation(line: 3, column: 3, scope: !17)
-!20 = !DILocation(line: 4, column: 5, scope: !17)
-
diff --git a/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll b/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
index 82d31872ffbc2..ac6bd2f934efc 100644
--- a/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
+++ b/llvm/test/tools/llvm-reduce/debug-metadata-verifier.ll
@@ -1,7 +1,7 @@
 ; RUN: llvm-reduce %s -o %t --delta-passes=metadata --test %python --test-arg %p/Inputs/remove-metadata.py --abort-on-invalid-reduction
 ; RUN: FileCheck %s --input-file %t
 
-; CHECK: call void @llvm.dbg.declare{{.*}}, !dbg
+; CHECK: #dbg_declare
 ; CHECK: !llvm.dbg.cu = !{!0}
 ; CHECK-NOT: uninteresting
 
diff --git a/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll b/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll
deleted file mode 100644
index 2b75cd5d6c3da..0000000000000
--- a/llvm/test/tools/llvm-reduce/remove-args-dbg-intrinsics.ll
+++ /dev/null
@@ -1,11 +0,0 @@
-; llvm-reduce shouldn't remove arguments of debug intrinsics, because the resulting module will be ill-formed.
-;
-; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=arguments --test FileCheck --test-arg --check-prefixes=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s --output %t
-; RUN: FileCheck --check-prefix=CHECK-FINAL %s < %t
-
-; CHECK-INTERESTINGNESS: declare void @llvm.dbg.declare
-; CHECK-FINAL: declare void @llvm.dbg.declare(metadata, metadata, metadata)
-declare void @llvm.dbg.declare(metadata, metadata, metadata)
-; CHECK-INTERESTINGNESS: declare void @llvm.dbg.value
-; CHECK-FINAL: declare ...
[truncated]

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jmorse jmorse merged commit 40f9bb9 into llvm:main Apr 9, 2025
6 of 9 checks passed
AllinLeeYL pushed a commit to AllinLeeYL/llvm-project that referenced this pull request Apr 10, 2025
…lvm#133917)

The "preserve input debug-info format" flag allowed some tooling to opt
into not seeing the new debug records yet, and to not autoupgrade. This
was good at the time, but un-necessary now that we'll be ditching
intrinsics shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag
to on, and as a result it hasn't seen debug records before. Thus, we
missed a uselistorder variation: constant-expressions such as GEPs can
be contained within debug records and completely isolated from the value
hierachy, see the metadata-use-uselistorder.ll test. These Values didn't
get ordered, but were legitimate uses of constants like "i64 0", and we
now run into difficulty handling that. The patch to AsmWriter seeks
Values to order even through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we
can just delete, such as one in llvm-reduce and another few in the
LocalTest unit tests. For the fast-isel test, it was added in
https://reviews.llvm.org/D67703 explicitly for checking the size of
blocks without debug-info and in 1525abb the codepath it tests moved
towards being sunsetted. It'll be totally redundant once RemoveDIs is on
permanently.

Note that there's now no explicit test for the textual-IR autoupgrade
path. I submit that we can rely on the thousands of .ll files where
we've only been bothered to update the outputs, not the inputs, to debug
records.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…lvm#133917)

The "preserve input debug-info format" flag allowed some tooling to opt
into not seeing the new debug records yet, and to not autoupgrade. This
was good at the time, but un-necessary now that we'll be ditching
intrinsics shortly.

It also hides errors now: verify-uselistorder was hardcoding this flag
to on, and as a result it hasn't seen debug records before. Thus, we
missed a uselistorder variation: constant-expressions such as GEPs can
be contained within debug records and completely isolated from the value
hierachy, see the metadata-use-uselistorder.ll test. These Values didn't
get ordered, but were legitimate uses of constants like "i64 0", and we
now run into difficulty handling that. The patch to AsmWriter seeks
Values to order even through debug-info now.

Finally there are a few intrinsics-tests relying on this flag that we
can just delete, such as one in llvm-reduce and another few in the
LocalTest unit tests. For the fast-isel test, it was added in
https://reviews.llvm.org/D67703 explicitly for checking the size of
blocks without debug-info and in 1525abb the codepath it tests moved
towards being sunsetted. It'll be totally redundant once RemoveDIs is on
permanently.

Note that there's now no explicit test for the textual-IR autoupgrade
path. I submit that we can rely on the thousands of .ll files where
we've only been bothered to update the outputs, not the inputs, to debug
records.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 llvm:analysis Includes value tracking, cost tables and constant folding llvm:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants