[NFC][DebugInfo] Maintain RemoveDIs flag when attributor creates functions

[jmorse]

We're using this flag (IsNewDbgInfoFormat) to detect the boundaries in LLVM of what's treating debug-info as intrinsics (i.e. dbg.value), and what's using DPValue objects (the non-intrinsic replacement). The attributor tends to create new wrapper functions and doesn't insert them into Modules in the usual way, thus we have to manually update that flag to signal what debug-info mode it's using.

I've added some --try-experimental-debuginfo-iterators RUN lines to tests that would otherwise crash because of this, so that they're exercised by our new-debuginfo-iterators buildbot.

NB: there's an attributor test with a dbg.value in it, however attributes re-order themselves in RemoveDIs mode for various reasons, so we're going to address that in a different patch.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Jeremy Morse (jmorse)

Changes

We're using this flag (IsNewDbgInfoFormat) to detect the boundaries in LLVM of what's treating debug-info as intrinsics (i.e. dbg.value), and what's using DPValue objects (the non-intrinsic replacement). The attributor tends to create new wrapper functions and doesn't insert them into Modules in the usual way, thus we have to manually update that flag to signal what debug-info mode it's using.

I've added some --try-experimental-debuginfo-iterators RUN lines to tests that would otherwise crash because of this, so that they're exercised by our new-debuginfo-iterators buildbot.

NB: there's an attributor test with a dbg.value in it, however attributes re-order themselves in RemoveDIs mode for various reasons, so we're going to address that in a different patch.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/Attributor.cpp (+6)
• (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll (+5)

diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index d8e290cbc8a4d04..5d1a783b2996d79 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -2738,6 +2738,8 @@ void Attributor::createShallowWrapper(Function &F) {
       Function::Create(FnTy, F.getLinkage(), F.getAddressSpace(), F.getName());
   F.setName(""); // set the inside function anonymous
   M.getFunctionList().insert(F.getIterator(), Wrapper);
+  // Flag whether the function is using new-debug-info or not.
+  Wrapper->IsNewDbgInfoFormat = M.IsNewDbgInfoFormat;
 
   F.setLinkage(GlobalValue::InternalLinkage);
 
@@ -2818,6 +2820,8 @@ bool Attributor::internalizeFunctions(SmallPtrSetImpl<Function *> &FnSet,
       VMap[&Arg] = &(*NewFArgIt++);
     }
     SmallVector<ReturnInst *, 8> Returns;
+    // Flag whether the function is using new-debug-info or not.
+    Copied->IsNewDbgInfoFormat = F->IsNewDbgInfoFormat;
 
     // Copy the body of the original function to the new one
     CloneFunctionInto(Copied, F, VMap,
@@ -3035,6 +3039,8 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
     OldFn->getParent()->getFunctionList().insert(OldFn->getIterator(), NewFn);
     NewFn->takeName(OldFn);
     NewFn->copyAttributesFrom(OldFn);
+    // Flag whether the function is using new-debug-info or not.
+    NewFn->IsNewDbgInfoFormat = OldFn->IsNewDbgInfoFormat;
 
     // Patch the pointer to LLVM function in debug info descriptor.
     NewFn->setSubprogram(OldFn->getSubprogram());
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll
index 9ae21ca44ee8c83..b3c3573ae275c7b 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll
@@ -2,6 +2,11 @@
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-annotate-decl-cs  -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
 
+;; Temporarily re-run with RemoveDIs debug-info mode to exercise a
+;; potential crash path.
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-annotate-decl-cs  -S < %s --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=CHECK,TUNIT
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=CHECK,CGSCC
+
 define void @f() {
 ; TUNIT-LABEL: define {{[^@]+}}@f() {
 ; TUNIT-NEXT:  entry:

[SLTozer]

LGTM, nit inline.

[SLTozer]

I'd argue the "temporarily" isn't necessary - this change is "temporary" in the same way all RemoveDIs check lines are, in that we can get rid of it when RemoveDIs is the default, so I'd just leave it as "Test with RemoveDIs debug-info..."