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

[RemoveDI] Handle DPValues in SROA #74089

Merged
merged 2 commits into from
Dec 12, 2023
Merged

[RemoveDI] Handle DPValues in SROA #74089

merged 2 commits into from
Dec 12, 2023

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Dec 1, 2023

Handle dbg.declares in SROA using DPValues.

In order to reduce duplication, the migrate-debug-info loop has been changed to a generic lambda with some helper function overloads, which is called for dbg.declares, dbg.assigns, and DPValues alike.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Handle dbg.declares in SROA using DPValues.

In order to reduce duplication, the migrate-debug-info loop has been changed to a generic lambda with some helper function overloads, which is called for dbg.declares, dbg.assigns, and DPValues alike.


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

13 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/SROA.cpp (+56-31)
  • (modified) llvm/test/DebugInfo/ARM/sroa-complex.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/sroa-larger.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/sroa-samesize.ll (+1)
  • (modified) llvm/test/DebugInfo/X86/sroa-after-inlining.ll (+1)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-1.ll (+1)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-2.ll (+1)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-3.ll (+2)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-4.ll (+1)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-5.ll (+2)
  • (modified) llvm/test/DebugInfo/X86/sroasplit-dbg-declare.ll (+2)
  • (modified) llvm/test/DebugInfo/salvage-overflow.ll (+1)
  • (modified) llvm/test/Transforms/Util/dbg-user-of-aext.ll (+1)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index f578762d2b4971c..24934eaeb0331c2 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4838,6 +4838,35 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
   return NewAI;
 }
 
+static void insertNewDbgInst(DIBuilder &DIB, DbgDeclareInst *Orig,
+                             AllocaInst *NewAddr, DIExpression *NewFragmentExpr,
+                             Instruction *BeforeInst) {
+  DIB.insertDeclare(NewAddr, Orig->getVariable(), NewFragmentExpr,
+                    Orig->getDebugLoc(), BeforeInst);
+}
+static void insertNewDbgInst(DIBuilder &DIB, DbgAssignIntrinsic *Orig,
+                             AllocaInst *NewAddr, DIExpression *NewFragmentExpr,
+                             Instruction *BeforeInst) {
+  (void)BeforeInst;
+  if (!NewAddr->hasMetadata(LLVMContext::MD_DIAssignID)) {
+    NewAddr->setMetadata(LLVMContext::MD_DIAssignID,
+                         DIAssignID::getDistinct(NewAddr->getContext()));
+  }
+  auto *NewAssign = DIB.insertDbgAssign(
+      NewAddr, Orig->getValue(), Orig->getVariable(), NewFragmentExpr, NewAddr,
+      Orig->getAddressExpression(), Orig->getDebugLoc());
+  LLVM_DEBUG(dbgs() << "Created new assign intrinsic: " << *NewAssign << "\n");
+}
+static void insertNewDbgInst(DIBuilder &DIB, DPValue *Orig, AllocaInst *NewAddr,
+                             DIExpression *NewFragmentExpr,
+                             Instruction *BeforeInst) {
+  (void)DIB;
+  DPValue *New = new DPValue(ValueAsMetadata::get(NewAddr), Orig->getVariable(),
+                             NewFragmentExpr, Orig->getDebugLoc(),
+                             DPValue::LocationType::Declare);
+  BeforeInst->getParent()->insertDPValueBefore(New, BeforeInst->getIterator());
+}
+
 /// Walks the slices of an alloca and form partitions based on them,
 /// rewriting each of their uses.
 bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
@@ -4937,14 +4966,7 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
   NumAllocaPartitions += NumPartitions;
   MaxPartitionsPerAlloca.updateMax(NumPartitions);
 
-  // Migrate debug information from the old alloca to the new alloca(s)
-  // and the individual partitions.
-  TinyPtrVector<DbgVariableIntrinsic *> DbgVariables;
-  for (auto *DbgDeclare : FindDbgDeclareUses(&AI))
-    DbgVariables.push_back(DbgDeclare);
-  for (auto *DbgAssign : at::getAssignmentMarkers(&AI))
-    DbgVariables.push_back(DbgAssign);
-  for (DbgVariableIntrinsic *DbgVariable : DbgVariables) {
+  auto MigrateOne = [&](auto *DbgVariable) {
     auto *Expr = DbgVariable->getExpression();
     DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);
     uint64_t AllocaSize =
@@ -4997,36 +5019,34 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
 
       // Remove any existing intrinsics on the new alloca describing
       // the variable fragment.
-      for (DbgDeclareInst *OldDII : FindDbgDeclareUses(Fragment.Alloca)) {
-        auto SameVariableFragment = [](const DbgVariableIntrinsic *LHS,
-                                       const DbgVariableIntrinsic *RHS) {
+      SmallVector<DbgDeclareInst *> FragDbgDeclares;
+      SmallVector<DPValue *> FragDPVs;
+      findDbgDeclares(FragDbgDeclares, Fragment.Alloca, &FragDPVs);
+      auto RemoveOne = [DbgVariable](auto *OldDII) {
+        auto SameVariableFragment = [](const auto *LHS, const auto *RHS) {
           return LHS->getVariable() == RHS->getVariable() &&
                  LHS->getDebugLoc()->getInlinedAt() ==
                      RHS->getDebugLoc()->getInlinedAt();
         };
         if (SameVariableFragment(OldDII, DbgVariable))
           OldDII->eraseFromParent();
-      }
+      };
+      for_each(FragDbgDeclares, RemoveOne);
+      for_each(FragDPVs, RemoveOne);
 
-      if (auto *DbgAssign = dyn_cast<DbgAssignIntrinsic>(DbgVariable)) {
-        if (!Fragment.Alloca->hasMetadata(LLVMContext::MD_DIAssignID)) {
-          Fragment.Alloca->setMetadata(
-              LLVMContext::MD_DIAssignID,
-              DIAssignID::getDistinct(AI.getContext()));
-        }
-        auto *NewAssign = DIB.insertDbgAssign(
-            Fragment.Alloca, DbgAssign->getValue(), DbgAssign->getVariable(),
-            FragmentExpr, Fragment.Alloca, DbgAssign->getAddressExpression(),
-            DbgAssign->getDebugLoc());
-        NewAssign->setDebugLoc(DbgAssign->getDebugLoc());
-        LLVM_DEBUG(dbgs() << "Created new assign intrinsic: " << *NewAssign
-                          << "\n");
-      } else {
-        DIB.insertDeclare(Fragment.Alloca, DbgVariable->getVariable(),
-                          FragmentExpr, DbgVariable->getDebugLoc(), &AI);
-      }
+      insertNewDbgInst(DIB, DbgVariable, Fragment.Alloca, FragmentExpr, &AI);
     }
-  }
+  };
+
+  // Migrate debug information from the old alloca to the new alloca(s)
+  // and the individual partitions.
+  SmallVector<DbgDeclareInst *> DbgDeclares;
+  SmallVector<DPValue *> DPValues;
+  findDbgDeclares(DbgDeclares, &AI, &DPValues);
+  for_each(DbgDeclares, MigrateOne);
+  for_each(DPValues, MigrateOne);
+  for_each(at::getAssignmentMarkers(&AI), MigrateOne);
+
   return Changed;
 }
 
@@ -5147,7 +5167,12 @@ bool SROA::deleteDeadInstructions(
     // not be able to find it.
     if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
       DeletedAllocas.insert(AI);
-      for (DbgDeclareInst *OldDII : FindDbgDeclareUses(AI))
+      SmallVector<DbgDeclareInst *> DbgDeclares;
+      SmallVector<DPValue *> DPValues;
+      findDbgDeclares(DbgDeclares, AI, &DPValues);
+      for (DbgDeclareInst *OldDII : DbgDeclares)
+        OldDII->eraseFromParent();
+      for (DPValue *OldDII : DPValues)
         OldDII->eraseFromParent();
     }
 
diff --git a/llvm/test/DebugInfo/ARM/sroa-complex.ll b/llvm/test/DebugInfo/ARM/sroa-complex.ll
index 7e81e488d59eaaf..f85e4cd29ba9e3f 100644
--- a/llvm/test/DebugInfo/ARM/sroa-complex.ll
+++ b/llvm/test/DebugInfo/ARM/sroa-complex.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes='sroa' -S -o - %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes='sroa' -S -o - %s | FileCheck %s
 target datalayout = "e-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
 target triple = "thumbv7-apple-unknown-macho"
 
diff --git a/llvm/test/DebugInfo/Generic/sroa-larger.ll b/llvm/test/DebugInfo/Generic/sroa-larger.ll
index 55e82034bea919e..2121b57c535b535 100644
--- a/llvm/test/DebugInfo/Generic/sroa-larger.ll
+++ b/llvm/test/DebugInfo/Generic/sroa-larger.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes='sroa' -S -o - %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes='sroa' -S -o - %s | FileCheck %s
 ; Generated from clang -c  -O2 -g -target x86_64-pc-windows-msvc
 ; struct A {
 ;   int _Myval2;
diff --git a/llvm/test/DebugInfo/Generic/sroa-samesize.ll b/llvm/test/DebugInfo/Generic/sroa-samesize.ll
index 0527f6fe6c72b38..1e70bad225c7875 100644
--- a/llvm/test/DebugInfo/Generic/sroa-samesize.ll
+++ b/llvm/test/DebugInfo/Generic/sroa-samesize.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes='sroa' -S -o - %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes='sroa' -S -o - %s | FileCheck %s
 ; Generated from clang -c  -O2 -g -target x86_64-pc-windows-msvc
 ; struct A { double x1[]; };
 ; struct x2 {
diff --git a/llvm/test/DebugInfo/X86/sroa-after-inlining.ll b/llvm/test/DebugInfo/X86/sroa-after-inlining.ll
index c648547f4a90fa7..d8f2c9a3ade0dfc 100644
--- a/llvm/test/DebugInfo/X86/sroa-after-inlining.ll
+++ b/llvm/test/DebugInfo/X86/sroa-after-inlining.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -passes='cgscc(function(sroa,instcombine),inline),function(instcombine,sroa),verify' -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='cgscc(function(sroa,instcombine),inline),function(instcombine,sroa),verify' -S -o - | FileCheck %s
 ;
 ; This test checks that SROA pass processes debug info correctly if applied twice.
 ; Specifically, after SROA works first time, instcombine converts dbg.declare
diff --git a/llvm/test/DebugInfo/X86/sroasplit-1.ll b/llvm/test/DebugInfo/X86/sroasplit-1.ll
index 5ac5c828b11ed84..426f522410c7e4f 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-1.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-1.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -passes='sroa,verify' -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='sroa,verify' -S -o - | FileCheck %s
 ;
 ; Test that we can partial emit debug info for aggregates repeatedly
 ; split up by SROA.
diff --git a/llvm/test/DebugInfo/X86/sroasplit-2.ll b/llvm/test/DebugInfo/X86/sroasplit-2.ll
index d002bcf6a1b641b..ddafc6b57f75b5c 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-2.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-2.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -passes='sroa,verify' -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='sroa,verify' -S -o - | FileCheck %s
 ;
 ; Test that we can partial emit debug info for aggregates repeatedly
 ; split up by SROA.
diff --git a/llvm/test/DebugInfo/X86/sroasplit-3.ll b/llvm/test/DebugInfo/X86/sroasplit-3.ll
index 48575ec018d93c4..ea70a776cebe565 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-3.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-3.ll
@@ -1,4 +1,6 @@
 ; RUN: opt %s -passes='sroa,verify' -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='sroa,verify' -S -o - | FileCheck %s
+
 ; ModuleID = 'test.c'
 ; Test that SROA updates the debug info correctly if an alloca was rewritten but
 ; not partitioned into multiple allocas.
diff --git a/llvm/test/DebugInfo/X86/sroasplit-4.ll b/llvm/test/DebugInfo/X86/sroasplit-4.ll
index 7f6cd3dde73a4c0..d3a6d147f2d3b62 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-4.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-4.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes='sroa' < %s -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes='sroa' < %s -S -o - | FileCheck %s
 ;
 ; Test that recursively splitting an alloca updates the debug info correctly.
 ; CHECK: %[[T:.*]] = load i64, ptr @t, align 8
diff --git a/llvm/test/DebugInfo/X86/sroasplit-5.ll b/llvm/test/DebugInfo/X86/sroasplit-5.ll
index d9dab2cad795256..67acb71363e36b1 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-5.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-5.ll
@@ -1,4 +1,6 @@
 ; RUN: opt %s -passes='sroa,verify' -S -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='sroa,verify' -S -o - | FileCheck %s
+
 ; From:
 ; struct prog_src_register {
 ;   unsigned : 4;
diff --git a/llvm/test/DebugInfo/X86/sroasplit-dbg-declare.ll b/llvm/test/DebugInfo/X86/sroasplit-dbg-declare.ll
index 6a9f749b5c68ad0..c591d2eb5a44748 100644
--- a/llvm/test/DebugInfo/X86/sroasplit-dbg-declare.ll
+++ b/llvm/test/DebugInfo/X86/sroasplit-dbg-declare.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -S -passes='sroa' -o - %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -S -passes='sroa' -o - %s | FileCheck %s
 
 ; SROA should split the alloca in two new ones, each with its own dbg.declare.
 ; The original alloca and dbg.declare should be removed.
@@ -55,3 +56,4 @@ attributes #1 = { nounwind readnone speculatable }
 ; CHECK-NOT:  = alloca [9 x i32]
 ; CHECK-NOT:  call void @llvm.dbg.declare(metadata ptr
 
+; CHECK: declare void @llvm.dbg.declare(metadata, metadata, metadata)
diff --git a/llvm/test/DebugInfo/salvage-overflow.ll b/llvm/test/DebugInfo/salvage-overflow.ll
index eff7514f82f6042..1c14d4b1d20591c 100644
--- a/llvm/test/DebugInfo/salvage-overflow.ll
+++ b/llvm/test/DebugInfo/salvage-overflow.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -passes='sroa,early-cse' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes='sroa,early-cse' -S | FileCheck %s
 ; CHECK: DIExpression(DW_OP_constu, 9223372036854775808, DW_OP_minus, DW_OP_stack_value)
 ; Created from the following C input (and then delta-reduced the IR):
 ;
diff --git a/llvm/test/Transforms/Util/dbg-user-of-aext.ll b/llvm/test/Transforms/Util/dbg-user-of-aext.ll
index c91b68a68e899b6..0511c41398981b0 100644
--- a/llvm/test/Transforms/Util/dbg-user-of-aext.ll
+++ b/llvm/test/Transforms/Util/dbg-user-of-aext.ll
@@ -2,6 +2,7 @@
 ; (here exposed through the SROA) pass refers to [s|z]exts of values (as
 ; opposed to the operand of a [s|z]ext).
 ; RUN: opt -S -passes='sroa' %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -S -passes='sroa' %s | FileCheck %s
 
 ; Built from:
 ; struct foo { bool b; long i; };

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.

LGTM, line changes seem graciously minimal.

@@ -55,3 +56,4 @@ attributes #1 = { nounwind readnone speculatable }
; CHECK-NOT: = alloca [9 x i32]
; CHECK-NOT: call void @llvm.dbg.declare(metadata ptr

; CHECK: declare void @llvm.dbg.declare(metadata, metadata, metadata)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this become necessary as part of the changes to this test, or is this purely a drive-by addition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, this was a hang-over from fiddling with the test. I'll undo this change before pushing.

Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

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

LGTM too; feels simpler as well.

In order to reduce duplication, the migrate-debug-info loop has been changed to
a generic lambda with some helper function overloads, which is called for
dbg.declares, dbg.assigns, and DPValues alike.
@OCHyams OCHyams merged commit 3d42557 into llvm:main Dec 12, 2023
3 of 4 checks passed
@OCHyams OCHyams deleted the ddd/SROA branch January 5, 2024 10:25
ichaer added a commit to ichaer/llvm-project-onesided_lower_bound that referenced this pull request Jan 29, 2024
* main: (5908 commits)
  [readtapi] Cleanup printing command line options (llvm#75106)
  [flang] Fix compilation error due to variable no being used (llvm#75210)
  [C API] Add getters and setters for fast-math flags on relevant instructions (llvm#75123)
  [libc++][CI] Tests the no RTTI configuration. (llvm#65518)
  [RemoveDIs] Fold variable into assert, it's only used once. NFC
  [RemoveDI] Handle DPValues in SROA (llvm#74089)
  [AArch64][GlobalISel] Test Pre-Commit for Look into array's element
  [mlir][tensor] Fix bug in `tensor.extract(tensor.from_elements)` folder (llvm#75109)
  [analyzer] Move alpha checker EnumCastOutOfRange to optin (llvm#67157)
  [RemoveDIs] Handle DPValues in replaceDbgDeclare (llvm#73507)
  [SHT_LLVM_BB_ADDR_MAP] Implements PGOAnalysisMap in Object and ObjectYAML with tests.
  [X86][GlobalISel] Add instruction selection for G_SELECT (llvm#70753)
  [AMDGPU] Remove unused function splitScalar64BitAddSub
  [LLVM][DWARF] Add compilation directory and dwo name to TU in dwo section (llvm#74909)
  [clang][Interp] Implement __builtin_ffs (llvm#72988)
  [RemoveDIs] Update ConvertDebugDeclareToDebugValue after llvm#72276 (llvm#73508)
  [libc][NFC] Reuse `FloatProperties` constant instead of creating new ones (llvm#75187)
  [RemoveDIs] Fix removeRedundantDdbgInstrs utils for dbg.declares (llvm#74102)
  Reapply "[RemoveDIs][NFC] Find DPValues using findDbgDeclares  (llvm#73500)"
  [GitHub] Remove author association print from new-prs workflow
  ...
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

4 participants