[RemoveDIs] Preserve debug info format in llvm-reduce

[SLTozer]

As the goal of LLVM reduce is to simplify the input file, it should not modify the debug info format - doing so by default would make it impossible to reduce an error that only occurs in the old format, for example (as briefly discussed at #86275). This patch uses the new "preserve debug info format" flag in llvm-reduce to prevent the input from being subtly transformed by llvm-reduce itself; this has no effect on any tools used for the interestingness check (i.e. if opt is invoked, it will still convert the reduced input to the new format by default), but simply ensures that the reduced file is strictly reduced rather than modified.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

As the goal of LLVM reduce is to simplify the input file, it should not modify the debug info format - doing so by default would make it impossible to reduce an error that only occurs in the old format, for example (as briefly discussed at #86275). This patch uses the new "preserve debug info format" flag in llvm-reduce to prevent the input from being subtly transformed by llvm-reduce itself; this has no effect on any tools used for the interestingness check (i.e. if opt is invoked, it will still convert the reduced input to the new format by default), but simply ensures that the reduced file is strictly reduced rather than modified.

---

Full diff: https://github.com/llvm/llvm-project/pull/89220.diff

2 Files Affected:

• (modified) llvm/test/tools/llvm-reduce/remove-dp-values.ll (+6-9)
• (modified) llvm/tools/llvm-reduce/llvm-reduce.cpp (+2-13)

diff --git a/llvm/test/tools/llvm-reduce/remove-dp-values.ll b/llvm/test/tools/llvm-reduce/remove-dp-values.ll
index d137b279f4ea01..4a0dfeef204ee0 100644
--- a/llvm/test/tools/llvm-reduce/remove-dp-values.ll
+++ b/llvm/test/tools/llvm-reduce/remove-dp-values.ll
@@ -1,17 +1,14 @@
-; RUN: llvm-reduce --abort-on-invalid-reduction --test FileCheck --test-arg --check-prefixes=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t --try-experimental-debuginfo-iterators
-; RUN: FileCheck --check-prefixes=CHECK-FINAL --input-file=%t %s --implicit-check-not=dbg.value
+; RUN: llvm-reduce --abort-on-invalid-reduction --test FileCheck --test-arg --check-prefixes=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefixes=CHECK-FINAL --input-file=%t %s --implicit-check-not=#dbg_value
 
 ; Test that we can, in RemoveDIs mode / DbgVariableRecords mode (where variable location
 ; information isn't an instruction), remove one variable location assignment
 ; but not another.
 
-; CHECK-INTERESTINGNESS:     call void @llvm.dbg.value(metadata i32 %added,
+; CHECK-INTERESTINGNESS:     #dbg_value(i32 %added,
 
-; CHECK-FINAL:     declare void @llvm.dbg.value(metadata,
 ; CHECK-FINAL:     %added = add
-; CHECK-FINAL-NEXT: call void @llvm.dbg.value(metadata i32 %added,
-
-declare void @llvm.dbg.value(metadata, metadata, metadata)
+; CHECK-FINAL-NEXT: #dbg_value(i32 %added,
 
 define i32 @main() !dbg !7 {
 entry:
@@ -22,10 +19,10 @@ entry:
   store i32 0, ptr %interesting, align 4
   %0 = load i32, ptr %interesting, align 4
   %added = add nsw i32 %0, 1
-  tail call void @llvm.dbg.value(metadata i32 %added, metadata !13, metadata !DIExpression()), !dbg !14
+    #dbg_value(i32 %added, !13, !DIExpression(), !14)
   store i32 %added, ptr %interesting, align 4
   %alsoloaded = load i32, ptr %interesting, align 4
-  tail call void @llvm.dbg.value(metadata i32 %alsoloaded, metadata !13, metadata !DIExpression()), !dbg !14
+    #dbg_value(i32 %alsoloaded, !13, !DIExpression(), !14)
   store i32 %alsoloaded, ptr %uninteresting2, align 4
   ret i32 0
 }
diff --git a/llvm/tools/llvm-reduce/llvm-reduce.cpp b/llvm/tools/llvm-reduce/llvm-reduce.cpp
index f913771487afe1..288a384c2ed498 100644
--- a/llvm/tools/llvm-reduce/llvm-reduce.cpp
+++ b/llvm/tools/llvm-reduce/llvm-reduce.cpp
@@ -100,12 +100,7 @@ static cl::opt<int>
                                "of delta passes (default=5)"),
                       cl::init(5), cl::cat(LLVMReduceOptions));
 
-static cl::opt<bool> TryUseNewDbgInfoFormat(
-    "try-experimental-debuginfo-iterators",
-    cl::desc("Enable debuginfo iterator positions, if they're built in"),
-    cl::init(false));
-
-extern cl::opt<bool> UseNewDbgInfoFormat;
+extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
 static codegen::RegisterCodeGenFlags CGF;
 
@@ -146,17 +141,11 @@ static std::pair<StringRef, bool> determineOutputType(bool IsMIR,
 int main(int Argc, char **Argv) {
   InitLLVM X(Argc, Argv);
   const StringRef ToolName(Argv[0]);
+  PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE;
 
   cl::HideUnrelatedOptions({&LLVMReduceOptions, &getColorCategory()});
   cl::ParseCommandLineOptions(Argc, Argv, "LLVM automatic testcase reducer.\n");
 
-  // RemoveDIs debug-info transition: tests may request that we /try/ to use the
-  // new debug-info format.
-  if (TryUseNewDbgInfoFormat) {
-    // Turn the new debug-info format on.
-    UseNewDbgInfoFormat = true;
-  }
-
   if (Argc == 1) {
     cl::PrintHelpMessage();
     return 0;

[OCHyams]

LGTM with one question about the test

[OCHyams]

Is it worth adding another set of RUNs and CHECKs that convert the input file to debug intrinsics, and test they stay as intrinsics. E.g., write to temp file, then pass that to --input-file=?

[SLTozer]

I would have said that this should be covered by existing tests, but apparently we don't have any tests that explicitly test removing debug intrinsics (since they're just like any other instruction) - for the sake of ensuring coverage as we transition to using records by default, I've added such a test here!

[OCHyams]

(Still LGTM - thanks!)