Skip to content
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

[DebugInfo][RemoveDIs] Instrument MergeFunctions for DPValues #80974

Merged
merged 3 commits into from
Feb 7, 2024

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Feb 7, 2024

The MergeFunctions pass has a "preserve some debug-info" mode that tries to preserve parameter values. This patch generalises its decision-making so that it applies to both debug-info stored in intrinsics, and debug-info stored in DPValue objects. For the most part this involves using a generic lambda and applying it to each type of object.

(Normally we avoid debug-info affecting the code generated, but this is hidden behind a command line switch, so won't usually be encountered by users).

Note that this diff is messy, but that's because I'm hoisting some code into lambdas. The actual decision making processes here are identical.

The MergeFunctions pass has a "preserve some debug-info" mode that tries to
preserve parameter values. This patch generalises its decision-making so
that it applies to both debug-info stored in intrinsics, and debug-info
stored in DPValue objects. For the most part this involves using a generic
lambda and applying it to each type of object.

(Normally we avoid debug-info affecting the code generated, but this is
hidden behind a command line switch, so won't usually be encountered by
users).
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

The MergeFunctions pass has a "preserve some debug-info" mode that tries to preserve parameter values. This patch generalises its decision-making so that it applies to both debug-info stored in intrinsics, and debug-info stored in DPValue objects. For the most part this involves using a generic lambda and applying it to each type of object.

(Normally we avoid debug-info affecting the code generated, but this is hidden behind a command line switch, so won't usually be encountered by users).

Note that this diff is messy, but that's because I'm hoisting some code into lambdas. The actual decision making processes here are identical.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/MergeFunctions.cpp (+119-69)
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index c8c011d94e4a3b..e5138ccd3d3595 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -256,15 +256,20 @@ class MergeFunctions {
 
   /// Fill PDIUnrelatedWL with instructions from the entry block that are
   /// unrelated to parameter related debug info.
+  /// \param PDPVUnrelatedWL The equivalent non-intrinsic debug records.
   void filterInstsUnrelatedToPDI(BasicBlock *GEntryBlock,
-                                 std::vector<Instruction *> &PDIUnrelatedWL);
+                                 std::vector<Instruction *> &PDIUnrelatedWL,
+                                 std::vector<DPValue *> &PDPVUnrelatedWL);
 
   /// Erase the rest of the CFG (i.e. barring the entry block).
   void eraseTail(Function *G);
 
   /// Erase the instructions in PDIUnrelatedWL as they are unrelated to the
   /// parameter debug info, from the entry block.
-  void eraseInstsUnrelatedToPDI(std::vector<Instruction *> &PDIUnrelatedWL);
+  /// \param PDPVUnrelatedWL contains the equivalent set of non-instruction
+  /// debug-info records.
+  void eraseInstsUnrelatedToPDI(std::vector<Instruction *> &PDIUnrelatedWL,
+                                std::vector<DPValue *> &PDPVUnrelatedWL);
 
   /// Replace G with a simple tail call to bitcast(F). Also (unless
   /// MergeFunctionsPDI holds) replace direct uses of G with bitcast(F),
@@ -506,7 +511,8 @@ static Value *createCast(IRBuilder<> &Builder, Value *V, Type *DestTy) {
 // Erase the instructions in PDIUnrelatedWL as they are unrelated to the
 // parameter debug info, from the entry block.
 void MergeFunctions::eraseInstsUnrelatedToPDI(
-    std::vector<Instruction *> &PDIUnrelatedWL) {
+    std::vector<Instruction *> &PDIUnrelatedWL,
+    std::vector<DPValue *> &PDPVUnrelatedWL) {
   LLVM_DEBUG(
       dbgs() << " Erasing instructions (in reverse order of appearance in "
                 "entry block) unrelated to parameter debug info from entry "
@@ -519,6 +525,16 @@ void MergeFunctions::eraseInstsUnrelatedToPDI(
     I->eraseFromParent();
     PDIUnrelatedWL.pop_back();
   }
+
+  while (!PDPVUnrelatedWL.empty()) {
+    DPValue *DPV = PDPVUnrelatedWL.back();
+    LLVM_DEBUG(dbgs() << "  Deleting DPValue ");
+    LLVM_DEBUG(DPV->print(dbgs()));
+    LLVM_DEBUG(dbgs() << "\n");
+    DPV->eraseFromParent();
+    PDPVUnrelatedWL.pop_back();
+  }
+
   LLVM_DEBUG(dbgs() << " } // Done erasing instructions unrelated to parameter "
                        "debug info from entry block. \n");
 }
@@ -547,75 +563,99 @@ void MergeFunctions::eraseTail(Function *G) {
 // The rest are unrelated to debug info for the parameters; fill up
 // PDIUnrelatedWL with such instructions.
 void MergeFunctions::filterInstsUnrelatedToPDI(
-    BasicBlock *GEntryBlock, std::vector<Instruction *> &PDIUnrelatedWL) {
+    BasicBlock *GEntryBlock, std::vector<Instruction *> &PDIUnrelatedWL,
+    std::vector<DPValue *> &PDPVUnrelatedWL) {
   std::set<Instruction *> PDIRelated;
-  for (BasicBlock::iterator BI = GEntryBlock->begin(), BIE = GEntryBlock->end();
-       BI != BIE; ++BI) {
-    if (auto *DVI = dyn_cast<DbgValueInst>(&*BI)) {
-      LLVM_DEBUG(dbgs() << " Deciding: ");
-      LLVM_DEBUG(BI->print(dbgs()));
+  std::set<DPValue *> PDPVRelated;
+
+  // Work out whether a dbg.value intrinsic or an equivalent DPValue is a
+  // parameter to be preserved.
+  auto ExamineDbgValue = [](auto *DbgVal, auto &Container) {
+    LLVM_DEBUG(dbgs() << " Deciding: ");
+    LLVM_DEBUG(DbgVal->print(dbgs()));
+    LLVM_DEBUG(dbgs() << "\n");
+    DILocalVariable *DILocVar = DbgVal->getVariable();
+    if (DILocVar->isParameter()) {
+      LLVM_DEBUG(dbgs() << "  Include (parameter): ");
+      LLVM_DEBUG(DbgVal->print(dbgs()));
       LLVM_DEBUG(dbgs() << "\n");
-      DILocalVariable *DILocVar = DVI->getVariable();
-      if (DILocVar->isParameter()) {
-        LLVM_DEBUG(dbgs() << "  Include (parameter): ");
-        LLVM_DEBUG(BI->print(dbgs()));
-        LLVM_DEBUG(dbgs() << "\n");
-        PDIRelated.insert(&*BI);
-      } else {
-        LLVM_DEBUG(dbgs() << "  Delete (!parameter): ");
-        LLVM_DEBUG(BI->print(dbgs()));
-        LLVM_DEBUG(dbgs() << "\n");
-      }
-    } else if (auto *DDI = dyn_cast<DbgDeclareInst>(&*BI)) {
-      LLVM_DEBUG(dbgs() << " Deciding: ");
-      LLVM_DEBUG(BI->print(dbgs()));
+      Container.insert(DbgVal);
+    } else {
+      LLVM_DEBUG(dbgs() << "  Delete (!parameter): ");
+      LLVM_DEBUG(DbgVal->print(dbgs()));
       LLVM_DEBUG(dbgs() << "\n");
-      DILocalVariable *DILocVar = DDI->getVariable();
-      if (DILocVar->isParameter()) {
-        LLVM_DEBUG(dbgs() << "  Parameter: ");
-        LLVM_DEBUG(DILocVar->print(dbgs()));
-        AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DDI->getAddress());
-        if (AI) {
-          LLVM_DEBUG(dbgs() << "  Processing alloca users: ");
-          LLVM_DEBUG(dbgs() << "\n");
-          for (User *U : AI->users()) {
-            if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
-              if (Value *Arg = SI->getValueOperand()) {
-                if (isa<Argument>(Arg)) {
-                  LLVM_DEBUG(dbgs() << "  Include: ");
-                  LLVM_DEBUG(AI->print(dbgs()));
-                  LLVM_DEBUG(dbgs() << "\n");
-                  PDIRelated.insert(AI);
-                  LLVM_DEBUG(dbgs() << "   Include (parameter): ");
-                  LLVM_DEBUG(SI->print(dbgs()));
-                  LLVM_DEBUG(dbgs() << "\n");
-                  PDIRelated.insert(SI);
-                  LLVM_DEBUG(dbgs() << "  Include: ");
-                  LLVM_DEBUG(BI->print(dbgs()));
-                  LLVM_DEBUG(dbgs() << "\n");
-                  PDIRelated.insert(&*BI);
-                } else {
-                  LLVM_DEBUG(dbgs() << "   Delete (!parameter): ");
-                  LLVM_DEBUG(SI->print(dbgs()));
-                  LLVM_DEBUG(dbgs() << "\n");
-                }
+    }
+  };
+
+  auto ExamineDbgDeclare = [&PDIRelated](auto *DbgDecl, auto &Container) {
+    LLVM_DEBUG(dbgs() << " Deciding: ");
+    LLVM_DEBUG(DbgDecl->print(dbgs()));
+    LLVM_DEBUG(dbgs() << "\n");
+    DILocalVariable *DILocVar = DbgDecl->getVariable();
+    if (DILocVar->isParameter()) {
+      LLVM_DEBUG(dbgs() << "  Parameter: ");
+      LLVM_DEBUG(DILocVar->print(dbgs()));
+      AllocaInst *AI = dyn_cast_or_null<AllocaInst>(DbgDecl->getAddress());
+      if (AI) {
+        LLVM_DEBUG(dbgs() << "  Processing alloca users: ");
+        LLVM_DEBUG(dbgs() << "\n");
+        for (User *U : AI->users()) {
+          if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+            if (Value *Arg = SI->getValueOperand()) {
+              if (isa<Argument>(Arg)) {
+                LLVM_DEBUG(dbgs() << "  Include: ");
+                LLVM_DEBUG(AI->print(dbgs()));
+                LLVM_DEBUG(dbgs() << "\n");
+                PDIRelated.insert(AI);
+                LLVM_DEBUG(dbgs() << "   Include (parameter): ");
+                LLVM_DEBUG(SI->print(dbgs()));
+                LLVM_DEBUG(dbgs() << "\n");
+                PDIRelated.insert(SI);
+                LLVM_DEBUG(dbgs() << "  Include: ");
+                LLVM_DEBUG(DbgDecl->print(dbgs()));
+                LLVM_DEBUG(dbgs() << "\n");
+                Container.insert(DbgDecl);
+              } else {
+                LLVM_DEBUG(dbgs() << "   Delete (!parameter): ");
+                LLVM_DEBUG(SI->print(dbgs()));
+                LLVM_DEBUG(dbgs() << "\n");
               }
-            } else {
-              LLVM_DEBUG(dbgs() << "   Defer: ");
-              LLVM_DEBUG(U->print(dbgs()));
-              LLVM_DEBUG(dbgs() << "\n");
             }
+          } else {
+            LLVM_DEBUG(dbgs() << "   Defer: ");
+            LLVM_DEBUG(U->print(dbgs()));
+            LLVM_DEBUG(dbgs() << "\n");
           }
-        } else {
-          LLVM_DEBUG(dbgs() << "  Delete (alloca NULL): ");
-          LLVM_DEBUG(BI->print(dbgs()));
-          LLVM_DEBUG(dbgs() << "\n");
         }
       } else {
-        LLVM_DEBUG(dbgs() << "  Delete (!parameter): ");
-        LLVM_DEBUG(BI->print(dbgs()));
+        LLVM_DEBUG(dbgs() << "  Delete (alloca NULL): ");
+        LLVM_DEBUG(DbgDecl->print(dbgs()));
         LLVM_DEBUG(dbgs() << "\n");
       }
+    } else {
+      LLVM_DEBUG(dbgs() << "  Delete (!parameter): ");
+      LLVM_DEBUG(DbgDecl->print(dbgs()));
+      LLVM_DEBUG(dbgs() << "\n");
+    }
+  };
+
+  for (BasicBlock::iterator BI = GEntryBlock->begin(), BIE = GEntryBlock->end();
+       BI != BIE; ++BI) {
+    // Examine DPValues as they happen "before" the instruction. Are they
+    // connected to parameters?
+    for (DPValue &DPV : BI->getDbgValueRange()) {
+      if (DPV.isDbgValue() || DPV.isDbgAssign()) {
+        ExamineDbgValue(&DPV, PDPVRelated);
+      } else {
+        assert(DPV.isDbgDeclare());
+        ExamineDbgDeclare(&DPV, PDPVRelated);
+      }
+    }
+
+    if (auto *DVI = dyn_cast<DbgValueInst>(&*BI)) {
+      ExamineDbgValue(DVI, PDIRelated);
+    } else if (auto *DDI = dyn_cast<DbgDeclareInst>(&*BI)) {
+      ExamineDbgDeclare(DDI, PDIRelated);
     } else if (BI->isTerminator() && &*BI == GEntryBlock->getTerminator()) {
       LLVM_DEBUG(dbgs() << " Will Include Terminator: ");
       LLVM_DEBUG(BI->print(dbgs()));
@@ -630,17 +670,26 @@ void MergeFunctions::filterInstsUnrelatedToPDI(
   LLVM_DEBUG(
       dbgs()
       << " Report parameter debug info related/related instructions: {\n");
-  for (Instruction &I : *GEntryBlock) {
-    if (PDIRelated.find(&I) == PDIRelated.end()) {
+
+  auto IsPDIRelated = [](auto *Rec, auto &Container, auto &UnrelatedCont) {
+    if (Container.find(Rec) == Container.end()) {
       LLVM_DEBUG(dbgs() << "  !PDIRelated: ");
-      LLVM_DEBUG(I.print(dbgs()));
+      LLVM_DEBUG(Rec->print(dbgs()));
       LLVM_DEBUG(dbgs() << "\n");
-      PDIUnrelatedWL.push_back(&I);
+      UnrelatedCont.push_back(Rec);
     } else {
       LLVM_DEBUG(dbgs() << "   PDIRelated: ");
-      LLVM_DEBUG(I.print(dbgs()));
+      LLVM_DEBUG(Rec->print(dbgs()));
       LLVM_DEBUG(dbgs() << "\n");
     }
+  };
+
+  // Collect the set of unrelated instructions and debug records.
+  for (Instruction &I : *GEntryBlock) {
+    for (auto &DPV : I.getDbgValueRange()) {
+      IsPDIRelated(&DPV, PDPVRelated, PDPVUnrelatedWL);
+    }
+    IsPDIRelated(&I, PDIRelated, PDIUnrelatedWL);
   }
   LLVM_DEBUG(dbgs() << " }\n");
 }
@@ -680,6 +729,7 @@ static void copyMetadataIfPresent(Function *From, Function *To, StringRef Key) {
 void MergeFunctions::writeThunk(Function *F, Function *G) {
   BasicBlock *GEntryBlock = nullptr;
   std::vector<Instruction *> PDIUnrelatedWL;
+  std::vector<DPValue *> PDPVUnrelatedWL;
   BasicBlock *BB = nullptr;
   Function *NewG = nullptr;
   if (MergeFunctionsPDI) {
@@ -691,7 +741,7 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
         dbgs() << "writeThunk: (MergeFunctionsPDI) filter parameter related "
                   "debug info for "
                << G->getName() << "() {\n");
-    filterInstsUnrelatedToPDI(GEntryBlock, PDIUnrelatedWL);
+    filterInstsUnrelatedToPDI(GEntryBlock, PDIUnrelatedWL, PDPVUnrelatedWL);
     GEntryBlock->getTerminator()->eraseFromParent();
     BB = GEntryBlock;
   } else {
@@ -740,7 +790,7 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
                  << G->getName() << "()\n");
     }
     eraseTail(G);
-    eraseInstsUnrelatedToPDI(PDIUnrelatedWL);
+    eraseInstsUnrelatedToPDI(PDIUnrelatedWL, PDPVUnrelatedWL);
     LLVM_DEBUG(
         dbgs() << "} // End of parameter related debug info filtering for: "
                << G->getName() << "()\n");

Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

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

Besides a single style nit, LGTM!

Comment on lines 689 to 691
for (auto &DPV : I.getDbgValueRange()) {
IsPDIRelated(&DPV, PDPVRelated, PDPVUnrelatedWL);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Unnecessary braces.

@jmorse jmorse merged commit 34f61cf into llvm:main Feb 7, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants