[llvm] [SROA] Fix debug locations for variables with non-zero offsets (PR #97750)
Orlando Cazalet-Hyams via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 16 03:05:28 PDT 2024
https://github.com/OCHyams updated https://github.com/llvm/llvm-project/pull/97750
>From da36d796e6395b16f988184df15609e53ed9992a Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 4 Jul 2024 05:48:37 +0100
Subject: [PATCH 1/2] [SROA] Fix debug locations for variables with non-zero
offsets
Fixes #61981 by adjusting variable location offsets (in the DIExpression) when
splittign allocas.
AKA Fix debug locations for C++ structured bindings in SROA.
NOTE: There's still a bug in mem2reg which generates incorrect locations in some
situations: if the variable fragment has an offset into the new (split) alloca,
mem2reg will fail to convert that into a bit shift (the location contains a
garbage offset). That's not addressed here.
insertNewDbgInst - Now takes the address-expression and FragmentInfo as
serperate parameters because unlike dbg_declares dbg_assigns want those to go
to different places. dbg_assign records put the variable fragment info in the
value expression only (whereas dbg_declare has only one expression so puts it
there - ideally this information wouldn't live in DIExpression, but that's
another issue).
MigrateOne - Modified to correctly compute the necessary offsets and fragment
adjustments. The previous implementation produced bogus locations for variables
with non-zero offsets. The changes replace most of the body of this lambda, so
it might be easier to review in a split-diff view and focus on the change as a
whole than to compare it to the old implementation.
This uses calculateFragmentIntersect and extractLeadingOffset added in previous
patches in this series, and createOrReplaceFragment described below.
createOrReplaceFragment - Similar to DIExpression::createFragmentExpression
except for 3 important distinctions:
1. The new fragment isn't relative to an existing fragment.
2. There are no checks on the the operation types because it is assumed
the location this expression is computing is not implicit (i.e., it's
always safe to create a fragment because arithmetic operations apply
to the address computation, not to an implicit value computation).
3. Existing extract_bits are modified independetly of fragment changes
using \p BitExtractOffset. A change to the fragment offset or size
may affect a bit extract. But a bit extract offset can change
independently of the fragment dimensions.
Returns the new expression, or nullptr if one couldn't be created. Ideally
this is only used to signal that a bit-extract has become zero-sized (and thus
the new debug record has no size and can be dropped), however, it fails for
other reasons too - see the FIXME below.
FIXME: To keep this patch NFC the function bails in situations that
DIExpression::createFragmentExpression fails in.E.g. when fragment and bit
extract sizes differ. These limitations can be removed in the future.
---
llvm/lib/Transforms/Scalar/SROA.cpp | 327 ++++++++++++++----
.../sroa/var-sized-fragment.ll | 5 +-
.../DebugInfo/Generic/sroa-alloca-offset.ll | 256 ++++++++++++++
3 files changed, 521 insertions(+), 67 deletions(-)
create mode 100644 llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 4d8fd5d3b9f5c..7d9bab78c7e15 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4967,32 +4967,207 @@ 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,
+// There isn't a shared interface to get the "address" parts out of a
+// dbg.declare and dbg.assign, so provide some wrappers now for
+// both debug intrinsics and records.
+const Value *getAddress(const DbgVariableIntrinsic *DVI) {
+ if (const auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI))
+ return DAI->getAddress();
+ return cast<DbgDeclareInst>(DVI)->getAddress();
+}
+const Value *getAddress(const DbgVariableRecord *DVR) {
+ assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
+ DVR->getType() == DbgVariableRecord::LocationType::Assign);
+ return DVR->getAddress();
+}
+bool isKillAddress(const DbgVariableIntrinsic *DVI) {
+ if (const auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI))
+ return DAI->isKillAddress();
+ return cast<DbgDeclareInst>(DVI)->isKillLocation();
+}
+bool isKillAddress(const DbgVariableRecord *DVR) {
+ assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
+ DVR->getType() == DbgVariableRecord::LocationType::Assign);
+ if (DVR->getType() == DbgVariableRecord::LocationType::Assign)
+ return DVR->isKillAddress();
+ return DVR->isKillLocation();
+}
+const DIExpression *getAddressExpression(const DbgVariableIntrinsic *DVI) {
+ if (const auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI))
+ return DAI->getAddressExpression();
+ return cast<DbgDeclareInst>(DVI)->getExpression();
+}
+const DIExpression *getAddressExpression(const DbgVariableRecord *DVR) {
+ assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
+ DVR->getType() == DbgVariableRecord::LocationType::Assign);
+ if (DVR->getType() == DbgVariableRecord::LocationType::Assign)
+ return DVR->getAddressExpression();
+ return DVR->getExpression();
+}
+
+/// Similar to DIExpression::createFragmentExpression except for 3 important
+/// distinctions:
+/// 1. The new fragment isn't relative to an existing fragment.
+/// 2. There are no checks on the the operation types because it is assumed
+/// the location this expression is computing is not implicit (i.e., it's
+/// always safe to create a fragment because arithmetic operations apply
+/// to the address computation, not to an implicit value computation).
+/// 3. Existing extract_bits are modified independetly of fragment changes
+/// using \p BitExtractOffset. A change to the fragment offset or size
+/// may affect a bit extract. But a bit extract offset can change
+/// independently of the fragment dimensions.
+///
+/// Returns the new expression, or nullptr if one couldn't be created.
+/// Ideally this is only used to signal that a bit-extract has become
+/// zero-sized (and thus the new debug record has no size and can be
+/// dropped), however, it fails for other reasons too - see the FIXME below.
+///
+/// FIXME: To keep the change that introduces this function NFC it bails
+/// in some situations unecessarily, e.g. when fragment and bit extract
+/// sizes differ.
+static DIExpression *createOrReplaceFragment(const DIExpression *Expr,
+ DIExpression::FragmentInfo Frag,
+ int64_t BitExtractOffset) {
+ SmallVector<uint64_t, 8> Ops;
+ bool HasFragment = false;
+ bool HasBitExtract = false;
+
+ for (auto &Op : Expr->expr_ops()) {
+ if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
+ HasFragment = true;
+ continue;
+ }
+ if (Op.getOp() == dwarf::DW_OP_LLVM_extract_bits_zext ||
+ Op.getOp() == dwarf::DW_OP_LLVM_extract_bits_sext) {
+ HasBitExtract = true;
+ int64_t ExtractOffsetInBits = Op.getArg(0);
+ int64_t ExtractSizeInBits = Op.getArg(1);
+
+ // DIExpression::createFragmentExpression doesn't know how to handle
+ // a fragment that is smaller than the extract. Copy the behaviour
+ // (bail) to avoid non-NFC changes.
+ // FIXME: Don't do this.
+ if (Frag.SizeInBits < uint64_t(ExtractSizeInBits))
+ return nullptr;
+
+ assert(BitExtractOffset <= 0);
+ int64_t AdjustedOffset = ExtractOffsetInBits + BitExtractOffset;
+
+ // DIExpression::createFragmentExpression doesn't know what to do
+ // if the new extract starts "outside" the existing one. Copy the
+ // behaviour (bail) to avoid non-NFC changes.
+ // FIXME: Don't do this.
+ if (AdjustedOffset < 0)
+ return nullptr;
+
+ Ops.push_back(Op.getOp());
+ Ops.push_back(std::max<int64_t>(0, AdjustedOffset));
+ Ops.push_back(ExtractSizeInBits);
+ continue;
+ }
+ Op.appendToVector(Ops);
+ }
+
+ // Unsupported by createFragmentExpression, so don't support it here yet to
+ // preserve NFC-ness.
+ if (HasFragment && HasBitExtract)
+ return nullptr;
+
+ if (!HasBitExtract) {
+ Ops.push_back(dwarf::DW_OP_LLVM_fragment);
+ Ops.push_back(Frag.OffsetInBits);
+ Ops.push_back(Frag.SizeInBits);
+ }
+ return DIExpression::get(Expr->getContext(), Ops);
+}
+
+/// Insert a new dbg.declare.
+/// \p Orig Original to copy debug loc and variable from.
+/// \p NewAddr Location's new base address.
+/// \p NewAddrExpr New expression to apply to address.
+/// \p BeforeInst Insert position.
+/// \p NewFragment New fragment (absolute, non-relative).
+/// \p BitExtractAdjustment Offset to apply to any extract_bits op.
+static void
+insertNewDbgInst(DIBuilder &DIB, DbgDeclareInst *Orig, AllocaInst *NewAddr,
+ DIExpression *NewAddrExpr, Instruction *BeforeInst,
+ std::optional<DIExpression::FragmentInfo> NewFragment,
+ int64_t BitExtractAdjustment) {
+ if (NewFragment)
+ NewAddrExpr = createOrReplaceFragment(NewAddrExpr, *NewFragment,
+ BitExtractAdjustment);
+ if (!NewAddrExpr)
+ return;
+
+ DIB.insertDeclare(NewAddr, Orig->getVariable(), NewAddrExpr,
Orig->getDebugLoc(), BeforeInst);
}
-static void insertNewDbgInst(DIBuilder &DIB, DbgAssignIntrinsic *Orig,
- AllocaInst *NewAddr, DIExpression *NewFragmentExpr,
- Instruction *BeforeInst) {
+
+/// Insert a new dbg.assign.
+/// \p Orig Original to copy debug loc, variable, value and value expression
+/// from.
+/// \p NewAddr Location's new base address.
+/// \p NewAddrExpr New expression to apply to address.
+/// \p BeforeInst Insert position.
+/// \p NewFragment New fragment (absolute, non-relative).
+/// \p BitExtractAdjustment Offset to apply to any extract_bits op.
+static void
+insertNewDbgInst(DIBuilder &DIB, DbgAssignIntrinsic *Orig, AllocaInst *NewAddr,
+ DIExpression *NewAddrExpr, Instruction *BeforeInst,
+ std::optional<DIExpression::FragmentInfo> NewFragment,
+ int64_t BitExtractAdjustment) {
(void)BeforeInst;
+
+ // A dbg.assign puts fragment info in the value expression only. The address
+ // expression has already been built: NewAddrExpr.
+ DIExpression *NewFragmentExpr = Orig->getExpression();
+ if (NewFragment)
+ NewFragmentExpr = createOrReplaceFragment(NewFragmentExpr, *NewFragment,
+ BitExtractAdjustment);
+ if (!NewFragmentExpr)
+ return;
+
+ // Apply a DIAssignID to the store if it doesn't already have it.
if (!NewAddr->hasMetadata(LLVMContext::MD_DIAssignID)) {
NewAddr->setMetadata(LLVMContext::MD_DIAssignID,
DIAssignID::getDistinct(NewAddr->getContext()));
}
+
Instruction *NewAssign =
DIB.insertDbgAssign(NewAddr, Orig->getValue(), Orig->getVariable(),
- NewFragmentExpr, NewAddr,
- Orig->getAddressExpression(), Orig->getDebugLoc())
+ NewFragmentExpr, NewAddr, NewAddrExpr,
+ Orig->getDebugLoc())
.get<Instruction *>();
LLVM_DEBUG(dbgs() << "Created new assign intrinsic: " << *NewAssign << "\n");
(void)NewAssign;
}
-static void insertNewDbgInst(DIBuilder &DIB, DbgVariableRecord *Orig,
- AllocaInst *NewAddr, DIExpression *NewFragmentExpr,
- Instruction *BeforeInst) {
+
+/// Insert a new DbgRecord.
+/// \p Orig Original to copy record type, debug loc and variable from, and
+/// additionally value and value expression for dbg_assign records.
+/// \p NewAddr Location's new base address.
+/// \p NewAddrExpr New expression to apply to address.
+/// \p BeforeInst Insert position.
+/// \p NewFragment New fragment (absolute, non-relative).
+/// \p BitExtractAdjustment Offset to apply to any extract_bits op.
+static void
+insertNewDbgInst(DIBuilder &DIB, DbgVariableRecord *Orig, AllocaInst *NewAddr,
+ DIExpression *NewAddrExpr, Instruction *BeforeInst,
+ std::optional<DIExpression::FragmentInfo> NewFragment,
+ int64_t BitExtractAdjustment) {
(void)DIB;
+
+ // A dbg_assign puts fragment info in the value expression only. The address
+ // expression has already been built: NewAddrExpr. A dbg_declare puts the
+ // new fragment info into NewAddrExpr (as it only has one expression).
+ DIExpression *NewFragmentExpr =
+ Orig->isDbgAssign() ? Orig->getExpression() : NewAddrExpr;
+ if (NewFragment)
+ NewFragmentExpr = createOrReplaceFragment(NewFragmentExpr, *NewFragment,
+ BitExtractAdjustment);
+ if (!NewFragmentExpr)
+ return;
+
if (Orig->isDbgDeclare()) {
DbgVariableRecord *DVR = DbgVariableRecord::createDVRDeclare(
NewAddr, Orig->getVariable(), NewFragmentExpr, Orig->getDebugLoc());
@@ -5000,13 +5175,16 @@ static void insertNewDbgInst(DIBuilder &DIB, DbgVariableRecord *Orig,
BeforeInst->getIterator());
return;
}
+
+ // Apply a DIAssignID to the store if it doesn't already have it.
if (!NewAddr->hasMetadata(LLVMContext::MD_DIAssignID)) {
NewAddr->setMetadata(LLVMContext::MD_DIAssignID,
DIAssignID::getDistinct(NewAddr->getContext()));
}
+
DbgVariableRecord *NewAssign = DbgVariableRecord::createLinkedDVRAssign(
NewAddr, Orig->getValue(), Orig->getVariable(), NewFragmentExpr, NewAddr,
- Orig->getAddressExpression(), Orig->getDebugLoc());
+ NewAddrExpr, Orig->getDebugLoc());
LLVM_DEBUG(dbgs() << "Created new DVRAssign: " << *NewAssign << "\n");
(void)NewAssign;
}
@@ -5019,7 +5197,7 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
unsigned NumPartitions = 0;
bool Changed = false;
- const DataLayout &DL = AI.getDataLayout();
+ const DataLayout &DL = AI.getModule()->getDataLayout();
// First try to pre-split loads and stores.
Changed |= presplitLoadsAndStores(AI, AS);
@@ -5110,57 +5288,79 @@ 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.
auto MigrateOne = [&](auto *DbgVariable) {
- auto *Expr = DbgVariable->getExpression();
- DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);
- uint64_t AllocaSize =
- DL.getTypeSizeInBits(AI.getAllocatedType()).getFixedValue();
- for (auto Fragment : Fragments) {
- // Create a fragment expression describing the new partition or reuse AI's
- // expression if there is only one partition.
- auto *FragmentExpr = Expr;
- if (Fragment.Size < AllocaSize || Expr->isFragment()) {
- // If this alloca is already a scalar replacement of a larger aggregate,
- // Fragment.Offset describes the offset inside the scalar.
- auto ExprFragment = Expr->getFragmentInfo();
- uint64_t Offset = ExprFragment ? ExprFragment->OffsetInBits : 0;
- uint64_t Start = Offset + Fragment.Offset;
- uint64_t Size = Fragment.Size;
- if (ExprFragment) {
- uint64_t AbsEnd =
- ExprFragment->OffsetInBits + ExprFragment->SizeInBits;
- if (Start >= AbsEnd) {
- // No need to describe a SROAed padding.
- continue;
- }
- Size = std::min(Size, AbsEnd - Start);
- }
- // The new, smaller fragment is stenciled out from the old fragment.
- if (auto OrigFragment = FragmentExpr->getFragmentInfo()) {
- assert(Start >= OrigFragment->OffsetInBits &&
- "new fragment is outside of original fragment");
- Start -= OrigFragment->OffsetInBits;
- }
+ // Can't overlap with undef memory.
+ if (isKillAddress(DbgVariable))
+ return;
- // The alloca may be larger than the variable.
- auto VarSize = DbgVariable->getVariable()->getSizeInBits();
- if (VarSize) {
- if (Size > *VarSize)
- Size = *VarSize;
- if (Size == 0 || Start + Size > *VarSize)
- continue;
- }
+ const Value *DbgPtr = getAddress(DbgVariable);
+ DIExpression::FragmentInfo VarFrag =
+ DbgVariable->getFragmentOrEntireVariable();
+ // Get the address expression constant offset if one exists and the ops
+ // that come after it.
+ int64_t CurrentExprOffsetInBytes = 0;
+ SmallVector<uint64_t> PostOffsetOps;
+ if (!getAddressExpression(DbgVariable)
+ ->extractLeadingOffset(CurrentExprOffsetInBytes, PostOffsetOps))
+ return; // Couldn't interpret this DIExpression - drop the var.
+
+ // Offset defined by a DW_OP_LLVM_extract_bits_[sz]ext.
+ int64_t ExtractOffsetInBits = 0;
+ for (auto Op : getAddressExpression(DbgVariable)->expr_ops()) {
+ if (Op.getOp() == dwarf::DW_OP_LLVM_extract_bits_zext ||
+ Op.getOp() == dwarf::DW_OP_LLVM_extract_bits_sext) {
+ ExtractOffsetInBits = Op.getArg(0);
+ break;
+ }
+ }
- // Avoid creating a fragment expression that covers the entire variable.
- if (!VarSize || *VarSize != Size) {
- if (auto E =
- DIExpression::createFragmentExpression(Expr, Start, Size))
- FragmentExpr = *E;
- else
- continue;
- }
+ DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);
+ for (auto Fragment : Fragments) {
+ int64_t OffsetFromLocationInBits;
+ std::optional<DIExpression::FragmentInfo> NewDbgFragment;
+ // Find the variable fragment that the new alloca slice covers.
+ // Drop debug info for this variable fragment if we can't compute an
+ // intersect between it and the alloca slice.
+ if (!DIExpression::calculateFragmentIntersect(
+ DL, &AI, Fragment.Offset, Fragment.Size, DbgPtr,
+ CurrentExprOffsetInBytes * 8, ExtractOffsetInBits, VarFrag,
+ NewDbgFragment, OffsetFromLocationInBits))
+ continue; // Do not migrate this fragment to this slice.
+
+ // Zero sized fragment indicates there's no intersect between the variable
+ // fragment and the alloca slice. Skip this slice for this variable
+ // fragment.
+ if (NewDbgFragment && !NewDbgFragment->SizeInBits)
+ continue; // Do not migrate this fragment to this slice.
+
+ // No fragment indicates DbgVariable's variable or fragment exactly
+ // overlaps the slice; copy its fragment (or nullopt if there isn't one).
+ if (!NewDbgFragment)
+ NewDbgFragment = DbgVariable->getFragment();
+
+ // Reduce the new expression offset by the bit-extract offset since
+ // we'll be keeping that.
+ int64_t OffestFromNewAllocaInBits =
+ OffsetFromLocationInBits - ExtractOffsetInBits;
+ // We need to adjust an existing bit extract if the offset expression
+ // can't eat the slack (i.e., if the new offset would be negative).
+ int64_t BitExtractOffset =
+ std::min<int64_t>(0, OffestFromNewAllocaInBits);
+ // The magnitude of a negative value indicates the number of bits into
+ // the existing variable fragment that the memory region begins. The new
+ // variable fragment already excludes those bits - the new DbgPtr offset
+ // only needs to be applied if it's positive.
+ OffestFromNewAllocaInBits =
+ std::max(int64_t(0), OffestFromNewAllocaInBits);
+
+ // Rebuild the expression:
+ // {Offset(OffestFromNewAllocaInBits), PostOffsetOps, NewDbgFragment}
+ // Add NewDbgFragment later, because dbg.assigns don't want it in the
+ // address expression but the value expression instead.
+ DIExpression *NewExpr = DIExpression::get(AI.getContext(), PostOffsetOps);
+ if (OffestFromNewAllocaInBits > 0) {
+ int64_t OffsetInBytes = (OffestFromNewAllocaInBits + 7) / 8;
+ NewExpr = DIExpression::prepend(NewExpr, /*flags=*/0, OffsetInBytes);
}
// Remove any existing intrinsics on the new alloca describing
@@ -5177,7 +5377,8 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
for_each(findDbgDeclares(Fragment.Alloca), RemoveOne);
for_each(findDVRDeclares(Fragment.Alloca), RemoveOne);
- insertNewDbgInst(DIB, DbgVariable, Fragment.Alloca, FragmentExpr, &AI);
+ insertNewDbgInst(DIB, DbgVariable, Fragment.Alloca, NewExpr, &AI,
+ NewDbgFragment, BitExtractOffset);
}
};
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/var-sized-fragment.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/var-sized-fragment.ll
index 55119114bd602..c2bcd4dcfeeee 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/var-sized-fragment.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/var-sized-fragment.ll
@@ -19,10 +19,7 @@
;; return a;
;; }
-;; FIXME: Variable 'b' gets an incorrect location (value and expression) - see
-;; llvm.org/PR61981. This check just ensures that no fragment info is added to
-;; the dbg.value.
-; CHECK: #dbg_value(i32 %.sroa.0.0.extract.trunc, ![[B:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4),
+; CHECK: #dbg_value(i32 %.sroa.2.0.extract.trunc, ![[B:[0-9]+]], !DIExpression(),
; CHECK: #dbg_value(i32 %.sroa.0.0.extract.trunc, ![[A:[0-9]+]], !DIExpression(),
; CHECK: ![[A]] = !DILocalVariable(name: "a",
; CHECK: ![[B]] = !DILocalVariable(name: "b",
diff --git a/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll b/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
new file mode 100644
index 0000000000000..2d3bbc236b54f
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
@@ -0,0 +1,256 @@
+; RUN: opt %s -passes=sroa -S | FileCheck %s --check-prefixes=COMMON,OLD
+; RUN: opt %s -passes=declare-to-assign,sroa -S | FileCheck %s --check-prefixes=COMMON,NEW
+
+;; C++17 source:
+;; struct two { int a, b; } gt;
+;; int fun1() {
+;; auto [x, y] = gt;
+;; return x + y;
+;; }
+;;
+;; struct four { two a, b; } gf;
+;; int fun2() {
+;; auto [x, y] = gf;
+;; return x.a + y.b;
+;; }
+;; Plus some hand-written IR.
+;;
+;; Check that SROA understands how to split dbg.declares and dbg.assigns with
+;; offsets into their storge (e.g., the second variable in a structured binding
+;; is stored at an offset into the shared alloca).
+;;
+;; Additional notes:
+;; We expect the same dbg.value intrinsics to come out of SROA whether assignment
+;; tracking is enabled or not. However, the order of the debug intrinsics may
+;; differ, and assignment tracking replaces some dbg.declares with dbg.assigns.
+;;
+;; Structured bindings produce an artificial variable that covers the entire
+;; alloca. Although they add clutter to the test, they've been preserved in
+;; order to increase coverage. These use the placehold name 'A' in comments and
+;; checks.
+
+%struct.two = type { i32, i32 }
+%struct.four = type { %struct.two, %struct.two }
+
+ at gt = dso_local global %struct.two zeroinitializer, align 4, !dbg !0
+ at gf = dso_local global %struct.four zeroinitializer, align 4, !dbg !5
+
+
+; COMMON-LABEL: @_Z4fun1v
+; COMMON-NEXT: entry
+;; 32 bit variable x (!27): value a_reg.
+;;
+;; 32 bit variable y (!28): value b_reg.
+;;
+;; 64 bit variable A (!29) bits [0, 32): value a_reg.
+;; 64 bit variable A (!29) bits [32, 64): value b_reg.
+
+; OLD-NEXT: %[[a_reg:.*]] = load i32, ptr @gt
+; OLD-NEXT: #dbg_value(i32 %[[a_reg]], ![[x0:[0-9]+]], !DIExpression(),
+; OLD-NEXT: #dbg_value(i32 %[[a_reg]], ![[A0:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+; OLD-NEXT: %[[b_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gt, i64 4)
+; OLD-NEXT: #dbg_value(i32 %[[b_reg]], ![[y0:[0-9]+]], !DIExpression(),
+; OLD-NEXT: #dbg_value(i32 %[[b_reg]], ![[A0]], !DIExpression(DW_OP_LLVM_fragment, 32, 32),
+
+; NEW-NEXT: %[[a_reg:.*]] = load i32, ptr @gt
+; NEW-NEXT: %[[b_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gt, i64 4)
+; NEW-NEXT: #dbg_value(i32 %[[b_reg]], ![[y0:[0-9]+]], !DIExpression(),
+; NEW-NEXT: #dbg_value(i32 %[[a_reg]], ![[A0:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+; NEW-NEXT: #dbg_value(i32 %[[b_reg]], ![[A0]], !DIExpression(DW_OP_LLVM_fragment, 32, 32),
+; NEW-NEXT: #dbg_value(i32 %[[a_reg]], ![[x0:[0-9]+]], !DIExpression(),
+define dso_local noundef i32 @_Z4fun1v() #0 !dbg !23 {
+entry:
+ %0 = alloca %struct.two, align 4
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !27, metadata !DIExpression()), !dbg !31
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !28, metadata !DIExpression(DW_OP_plus_uconst, 4)), !dbg !31
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !29, metadata !DIExpression()), !dbg !31
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gt, i64 8, i1 false), !dbg !31
+ %a = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 0, !dbg !31
+ %1 = load i32, ptr %a, align 4, !dbg !31
+ %b = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 1, !dbg !31
+ %2 = load i32, ptr %b, align 4, !dbg !31
+ %add = add nsw i32 %1, %2, !dbg !31
+ ret i32 %add, !dbg !31
+}
+
+; COMMON-LABEL: _Z4fun2v()
+; COMMON-NEXT: entry:
+;; 64 bit variable x (!50) bits [0, 32): value aa_reg.
+;; 64 bit variable x (!50) bits [32, 64): deref ab_ba_addr
+;;
+;; 64 bit variable y (!51) bits [0, 32): deref ab_ba_addr + 32
+;; 64 bit variable y (!51) bits [32, 64): value bb_reg.
+;;
+;; 128 bit variable A (!52) bits [0, 32): value aa_reg
+;; 128 bit variable A (!52) bits [32, 64): deref ab_ba_addr
+;; 128 bit variable A (!52) bits [96, 128): value bb_reg
+;;
+;; NOTE: This 8 byte alloca contains x.b (4 bytes) and y.a (4 bytes).
+; COMMON-NEXT: %[[ab_ba_addr:.*]] = alloca [8 x i8], align 4
+; OLD-NEXT: #dbg_declare(ptr %[[ab_ba_addr]], ![[A1:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 32, 64),
+; OLD-NEXT: #dbg_declare(ptr %[[ab_ba_addr]], ![[y1:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_LLVM_fragment, 0, 32),
+; OLD-NEXT: #dbg_declare(ptr %[[ab_ba_addr]], ![[x1:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 32, 32),
+; OLD-NEXT: %[[aa_reg:.*]] = load i32, ptr @gf, align 4
+; OLD-NEXT: #dbg_value(i32 %[[aa_reg]], ![[x1]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+; OLD-NEXT: #dbg_value(i32 %[[aa_reg]], ![[A1]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+; OLD-NEXT: call void @llvm.memcpy{{.*}}(ptr align 4 %[[ab_ba_addr]], ptr align 4 getelementptr inbounds (i8, ptr @gf, i64 4), i64 8, i1 false)
+; OLD-NEXT: %[[bb_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gf, i64 12), align 4
+; OLD-NEXT: #dbg_value(i32 %[[bb_reg]], ![[y1]], !DIExpression(DW_OP_LLVM_fragment, 32, 32),
+; OLD-NEXT: #dbg_value(i32 %[[bb_reg]], ![[A1]], !DIExpression(DW_OP_LLVM_fragment, 96, 32),
+
+; NEW-NEXT: #dbg_assign(i1 undef, ![[x1:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 32, 32), ![[#]], ptr %[[ab_ba_addr]], !DIExpression(),
+; NEW-NEXT: #dbg_assign(i1 undef, ![[A1:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 32, 64), ![[#]], ptr %[[ab_ba_addr]], !DIExpression(),
+; NEW-NEXT: #dbg_declare(ptr %[[ab_ba_addr]], ![[y1:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_LLVM_fragment, 0, 32),
+; NEW-NEXT: %[[aa_reg:.*]] = load i32, ptr @gf, align 4
+; NEW-NEXT: llvm.memcpy{{.*}}(ptr align 4 %[[ab_ba_addr]], ptr align 4 getelementptr inbounds (i8, ptr @gf, i64 4), i64 8, i1 false){{.*}}, !DIAssignID ![[ID:[0-9]+]]
+; NEW-NEXT: %[[bb_reg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gf, i64 12), align 4
+; NEW-NEXT: #dbg_value(i32 %[[bb_reg]], ![[y1:[0-9]+]], !DIExpression(DW_OP_LLVM_fragment, 32, 32),
+; NEW-NEXT: #dbg_value(i32 %[[aa_reg]], ![[A1]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+; NEW-NEXT: #dbg_assign(i1 undef, ![[A1]], !DIExpression(DW_OP_LLVM_fragment, 32, 64), ![[ID]], ptr %[[ab_ba_addr]], !DIExpression(),
+; NEW-NEXT: #dbg_value(i32 %[[bb_reg]], ![[A1]], !DIExpression(DW_OP_LLVM_fragment, 96, 32),
+; NEW-NEXT: #dbg_value(i32 %[[aa_reg]], ![[x1]], !DIExpression(DW_OP_LLVM_fragment, 0, 32),
+define dso_local noundef i32 @_Z4fun2v() #0 !dbg !48 {
+entry:
+ %0 = alloca %struct.four, align 4
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !50, metadata !DIExpression()), !dbg !54
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !51, metadata !DIExpression(DW_OP_plus_uconst, 8)), !dbg !54
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !52, metadata !DIExpression()), !dbg !54
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gf, i64 16, i1 false), !dbg !54
+ %a = getelementptr inbounds %struct.four, ptr %0, i32 0, i32 0, !dbg !54
+ %a1 = getelementptr inbounds %struct.two, ptr %a, i32 0, i32 0, !dbg !54
+ %1 = load i32, ptr %a1, align 4, !dbg !54
+ %b = getelementptr inbounds %struct.four, ptr %0, i32 0, i32 1, !dbg !54
+ %b2 = getelementptr inbounds %struct.two, ptr %b, i32 0, i32 1, !dbg !54
+ %2 = load i32, ptr %b2, align 4, !dbg !54
+ %add = add nsw i32 %1, %2, !dbg !54
+ ret i32 %add, !dbg !54
+}
+
+;; Hand-written part to test what happens when variables are smaller than the
+;; new alloca slices (i.e., check offset rewriting works correctly). Note that
+;; mem2reg incorrectly preserves the offest in the DIExpression a variable
+;; stuffed into the upper bits of a value (that is a bug), e.g. alloca+offset
+;; becomes vreg+offest. It should either convert the offest to a shift, or
+;; encode the register-bit offest using DW_OP_bit_piece.
+; COMMON-LABEL: _Z4fun3v()
+; COMMON-NEXT: entry:
+;; 16 bit variable e (!61): value ve (upper bits)
+;;
+;; 16 bit variable f (!62): value vgf (lower bits)
+;; 16 bit variable g (!63): value vgf (upper bits)
+;;
+;; 16 bit variable h (!64): deref dead_64_128
+; COMMON-NEXT: %[[dead_64_128:.*]] = alloca %struct.two
+; COMMON-NEXT: #dbg_declare(ptr %[[dead_64_128]], ![[h:[0-9]+]], !DIExpression(),
+; COMMON-NEXT: %[[ve:.*]] = load i32, ptr @gf
+;; FIXME: mem2reg bug - offset is incorrect - see comment above.
+; COMMON-NEXT: #dbg_value(i32 %[[ve]], ![[e:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 2),
+; COMMON-NEXT: %[[vfg:.*]] = load i32, ptr getelementptr inbounds (i8, ptr @gf, i64 4)
+; COMMON-NEXT: #dbg_value(i32 %[[vfg]], ![[f:[0-9]+]], !DIExpression(),
+;; FIXME: mem2reg bug - offset is incorrect - see comment above.
+; COMMON-NEXT: #dbg_value(i32 %[[vfg]], ![[g:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 2),
+define dso_local noundef i32 @_Z4fun3v() #0 !dbg !55 {
+entry:
+ %0 = alloca %struct.four, align 4
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !61, metadata !DIExpression(DW_OP_plus_uconst, 2)), !dbg !58
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !62, metadata !DIExpression(DW_OP_plus_uconst, 4)), !dbg !58
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !63, metadata !DIExpression(DW_OP_plus_uconst, 6)), !dbg !58
+ call void @llvm.dbg.declare(metadata ptr %0, metadata !64, metadata !DIExpression(DW_OP_plus_uconst, 8)), !dbg !58
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gf, i64 16, i1 false), !dbg !58
+ %1 = getelementptr inbounds %struct.four, ptr %0, i32 0, i32 0, !dbg !58
+ %2 = getelementptr inbounds %struct.two, ptr %1, i32 0, i32 1, !dbg !58
+ %3 = load i32, ptr %2, align 4, !dbg !58
+ ret i32 %3, !dbg !58
+}
+
+;; splitAlloca in SROA.cpp only understands expressions with an optional offset
+;; and optional fragment. Check a dbg.declare with an extra op is dropped. If
+;; this test fails it indicates the debug info updating code in SROA.cpp
+;; splitAlloca may need to be update to account for more complex input
+;; expressions. Search for this file name in SROA.cpp.
+; COMMON-LABEL: fun4
+; COMMON-NOT: llvm.dbg
+; COMMON: #dbg_value(ptr %0, ![[p:[0-9]+]], !DIExpression(DW_OP_deref),
+; COMMON: #dbg_value(ptr %0, ![[q:[0-9]+]], !DIExpression(),
+; COMMON-NOT: llvm.dbg
+; COMMON: ret
+define dso_local noundef i32 @fun4(ptr %0) #0 !dbg !65 {
+entry:
+ %p = alloca [2 x ptr]
+ store ptr %0, ptr %p
+ call void @llvm.dbg.declare(metadata ptr %p, metadata !67, metadata !DIExpression(DW_OP_deref)), !dbg !66
+ call void @llvm.dbg.declare(metadata ptr %p, metadata !68, metadata !DIExpression()), !dbg !66
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gf, i64 16, i1 false), !dbg !66
+ %deref = load ptr, ptr %p
+ %1 = getelementptr inbounds %struct.four, ptr %deref, i32 0, i32 0, !dbg !66
+ %2 = load i32, ptr %1, align 4, !dbg !66
+ ret i32 %2, !dbg !66
+}
+
+; COMMON-DAG: ![[x0]] = !DILocalVariable(name: "x",
+; COMMON-DAG: ![[y0]] = !DILocalVariable(name: "y",
+; COMMON-DAG: ![[A0]] = !DILocalVariable(scope:
+
+; COMMON-DAG: ![[x1]] = !DILocalVariable(name: "x",
+; COMMON-DAG: ![[y1]] = !DILocalVariable(name: "y",
+; COMMON-DAG: ![[A1]] = !DILocalVariable(scope:
+
+; COMMON-DAG: ![[e]] = !DILocalVariable(name: "e",
+; COMMON-DAG: ![[f]] = !DILocalVariable(name: "f",
+; COMMON-DAG: ![[g]] = !DILocalVariable(name: "g",
+; COMMON-DAG: ![[h]] = !DILocalVariable(name: "h",
+
+; COMMON-DAG: ![[q]] = !DILocalVariable(name: "q"
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!16, !17}
+!llvm.ident = !{!22}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "gt", scope: !2, file: !3, line: 1, type: !10, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang version 17.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "test.cpp", directory: "/")
+!4 = !{!0, !5}
+!5 = !DIGlobalVariableExpression(var: !6, expr: !DIExpression())
+!6 = distinct !DIGlobalVariable(name: "gf", scope: !2, file: !3, line: 7, type: !7, isLocal: false, isDefinition: true)
+!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "four", file: !3, line: 7, size: 128, flags: DIFlagTypePassByValue, elements: !8, identifier: "_ZTS4four")
+!8 = !{!9, !15}
+!9 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !7, file: !3, line: 7, baseType: !10, size: 64)
+!10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "two", file: !3, line: 1, size: 64, flags: DIFlagTypePassByValue, elements: !11, identifier: "_ZTS3two")
+!11 = !{!12, !14}
+!12 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !10, file: !3, line: 1, baseType: !13, size: 32)
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !10, file: !3, line: 1, baseType: !13, size: 32, offset: 32)
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !7, file: !3, line: 7, baseType: !10, size: 64, offset: 64)
+!16 = !{i32 7, !"Dwarf Version", i32 5}
+!17 = !{i32 2, !"Debug Info Version", i32 3}
+!22 = !{!"clang version 17.0.0"}
+!23 = distinct !DISubprogram(name: "fun1", linkageName: "_Z4fun1v", scope: !3, file: !3, line: 2, type: !24, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !26)
+!24 = !DISubroutineType(types: !25)
+!25 = !{!13}
+!26 = !{!27, !28, !29}
+!27 = !DILocalVariable(name: "x", scope: !23, file: !3, line: 3, type: !13)
+!28 = !DILocalVariable(name: "y", scope: !23, file: !3, line: 3, type: !13)
+!29 = !DILocalVariable(scope: !23, file: !3, line: 3, type: !10)
+!31 = !DILocation(line: 3, column: 9, scope: !23)
+!48 = distinct !DISubprogram(name: "fun2", linkageName: "_Z4fun2v", scope: !3, file: !3, line: 8, type: !24, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !49)
+!49 = !{!50, !51, !52}
+!50 = !DILocalVariable(name: "x", scope: !48, file: !3, line: 9, type: !10)
+!51 = !DILocalVariable(name: "y", scope: !48, file: !3, line: 9, type: !10)
+!52 = !DILocalVariable(scope: !48, file: !3, line: 9, type: !7)
+!54 = !DILocation(line: 9, column: 9, scope: !48)
+!55 = distinct !DISubprogram(name: "fun3", linkageName: "_Z4fun3v", scope: !3, file: !3, line: 8, type: !24, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !56)
+!56 = !{}
+!58 = !DILocation(line: 9, column: 9, scope: !55)
+!60 = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+!61 = !DILocalVariable(name: "e", scope: !55, file: !3, line: 9, type: !60)
+!62 = !DILocalVariable(name: "f", scope: !55, file: !3, line: 9, type: !60)
+!63 = !DILocalVariable(name: "g", scope: !55, file: !3, line: 9, type: !60)
+!64 = !DILocalVariable(name: "h", scope: !55, file: !3, line: 9, type: !60)
+!65 = distinct !DISubprogram(name: "fun4", linkageName: "_Z4fun4v", scope: !3, file: !3, line: 8, type: !24, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !56)
+!66 = !DILocation(line: 9, column: 9, scope: !65)
+!67 = !DILocalVariable(name: "p", scope: !65, file: !3, line: 9, type: !13)
+!68 = !DILocalVariable(name: "q", scope: !65, file: !3, line: 9, type: !13)
>From 493ce091cf4e2adae98222ddc3e0e9ef6199d7ef Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Tue, 16 Jul 2024 11:05:03 +0100
Subject: [PATCH 2/2] address comments
---
llvm/lib/Transforms/Scalar/SROA.cpp | 25 +++++--
.../DebugInfo/Generic/sroa-alloca-offset.ll | 68 ++++++++++++-------
2 files changed, 62 insertions(+), 31 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 7d9bab78c7e15..32aadc936827b 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4975,16 +4975,19 @@ const Value *getAddress(const DbgVariableIntrinsic *DVI) {
return DAI->getAddress();
return cast<DbgDeclareInst>(DVI)->getAddress();
}
+
const Value *getAddress(const DbgVariableRecord *DVR) {
assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
DVR->getType() == DbgVariableRecord::LocationType::Assign);
return DVR->getAddress();
}
+
bool isKillAddress(const DbgVariableIntrinsic *DVI) {
if (const auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI))
return DAI->isKillAddress();
return cast<DbgDeclareInst>(DVI)->isKillLocation();
}
+
bool isKillAddress(const DbgVariableRecord *DVR) {
assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
DVR->getType() == DbgVariableRecord::LocationType::Assign);
@@ -4992,11 +4995,13 @@ bool isKillAddress(const DbgVariableRecord *DVR) {
return DVR->isKillAddress();
return DVR->isKillLocation();
}
+
const DIExpression *getAddressExpression(const DbgVariableIntrinsic *DVI) {
if (const auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI))
return DAI->getAddressExpression();
return cast<DbgDeclareInst>(DVI)->getExpression();
}
+
const DIExpression *getAddressExpression(const DbgVariableRecord *DVR) {
assert(DVR->getType() == DbgVariableRecord::LocationType::Declare ||
DVR->getType() == DbgVariableRecord::LocationType::Assign);
@@ -5005,14 +5010,19 @@ const DIExpression *getAddressExpression(const DbgVariableRecord *DVR) {
return DVR->getExpression();
}
-/// Similar to DIExpression::createFragmentExpression except for 3 important
-/// distinctions:
+/// Create or replace an existing fragment in a DIExpression with \p Frag.
+/// If the expression already contains a DW_OP_LLVM_extract_bits_[sz]ext
+/// operation, add \p BitExtractOffset to the offset part.
+///
+/// Returns the new expression, or nullptr if this fails (see details below).
+///
+/// This function is similar to DIExpression::createFragmentExpression except
+/// for 3 important distinctions:
/// 1. The new fragment isn't relative to an existing fragment.
-/// 2. There are no checks on the the operation types because it is assumed
-/// the location this expression is computing is not implicit (i.e., it's
-/// always safe to create a fragment because arithmetic operations apply
-/// to the address computation, not to an implicit value computation).
-/// 3. Existing extract_bits are modified independetly of fragment changes
+/// 2. It assumes the computed location is a memory location. This means we
+/// don't need to perform checks that creating the fragment preserves the
+/// expression semantics.
+/// 3. Existing extract_bits are modified independently of fragment changes
/// using \p BitExtractOffset. A change to the fragment offset or size
/// may affect a bit extract. But a bit extract offset can change
/// independently of the fragment dimensions.
@@ -5116,6 +5126,7 @@ insertNewDbgInst(DIBuilder &DIB, DbgAssignIntrinsic *Orig, AllocaInst *NewAddr,
DIExpression *NewAddrExpr, Instruction *BeforeInst,
std::optional<DIExpression::FragmentInfo> NewFragment,
int64_t BitExtractAdjustment) {
+ // DIBuilder::insertDbgAssign will insert the #dbg_assign after NewAddr.
(void)BeforeInst;
// A dbg.assign puts fragment info in the value expression only. The address
diff --git a/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll b/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
index 2d3bbc236b54f..b09b18af67167 100644
--- a/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
+++ b/llvm/test/DebugInfo/Generic/sroa-alloca-offset.ll
@@ -127,11 +127,12 @@ entry:
}
;; Hand-written part to test what happens when variables are smaller than the
-;; new alloca slices (i.e., check offset rewriting works correctly). Note that
-;; mem2reg incorrectly preserves the offest in the DIExpression a variable
+;; new alloca slices (i.e., check offset rewriting works correctly). Note that
+;; mem2reg incorrectly preserves the offest in the DIExpression of a variable
;; stuffed into the upper bits of a value (that is a bug), e.g. alloca+offset
-;; becomes vreg+offest. It should either convert the offest to a shift, or
-;; encode the register-bit offest using DW_OP_bit_piece.
+;; becomes vreg+offest. It should either convert the offest to a shift, encode
+;; the register-bit offest using DW_OP_bit_piece, or use the new
+;; DW_OP_LLVM_extract_bits_[sz]ext operation.
; COMMON-LABEL: _Z4fun3v()
; COMMON-NEXT: entry:
;; 16 bit variable e (!61): value ve (upper bits)
@@ -163,28 +164,44 @@ entry:
ret i32 %3, !dbg !58
}
-;; splitAlloca in SROA.cpp only understands expressions with an optional offset
-;; and optional fragment. Check a dbg.declare with an extra op is dropped. If
-;; this test fails it indicates the debug info updating code in SROA.cpp
-;; splitAlloca may need to be update to account for more complex input
-;; expressions. Search for this file name in SROA.cpp.
+;; Check that DW_OP_extract_bits_[sz]ext compose with expression offsets and
+;; that new fragments are not created. DW_OP_extract_bits_[sz]ext and fragments
+;; don't compose currently (but could). There are checks that expressions with
+;; bit extracts and fragments are dropped in SROA the test
+;; in llvm/test/DebugInfo/Generic/sroa-extract-bits.ll. FIXME: Don't do that.
+;;
+;; Checks are inline for this one.
+;;
+;; %p alloca is 128 bits
+;; SROA is going to split it in half, discard the lower bits, then split
+;; the upper bits in half and discard the upper bits leaving us with
+;; bits [64, 96) of the original alloca.
+;;
; COMMON-LABEL: fun4
-; COMMON-NOT: llvm.dbg
-; COMMON: #dbg_value(ptr %0, ![[p:[0-9]+]], !DIExpression(DW_OP_deref),
-; COMMON: #dbg_value(ptr %0, ![[q:[0-9]+]], !DIExpression(),
-; COMMON-NOT: llvm.dbg
-; COMMON: ret
-define dso_local noundef i32 @fun4(ptr %0) #0 !dbg !65 {
+define dso_local noundef i32 @fun4(i64 %0) !dbg !65 {
entry:
- %p = alloca [2 x ptr]
- store ptr %0, ptr %p
- call void @llvm.dbg.declare(metadata ptr %p, metadata !67, metadata !DIExpression(DW_OP_deref)), !dbg !66
- call void @llvm.dbg.declare(metadata ptr %p, metadata !68, metadata !DIExpression()), !dbg !66
- call void @llvm.memcpy.p0.p0.i64(ptr align 4 %0, ptr align 4 @gf, i64 16, i1 false), !dbg !66
- %deref = load ptr, ptr %p
- %1 = getelementptr inbounds %struct.four, ptr %deref, i32 0, i32 0, !dbg !66
- %2 = load i32, ptr %1, align 4, !dbg !66
- ret i32 %2, !dbg !66
+ %p = alloca [2 x i64]
+ %1 = getelementptr inbounds [2 x i64], ptr %p, i32 0, i32 1
+ store i64 %0, ptr %1
+ ; COMMON: %p.sroa.0.8.extract.trunc = trunc i64 %0 to i32
+ ;; Simple case - the expression offset (8 bytes) matches the offset of the
+ ;; slice into the alloca, so can be discarded away entirely.
+ ; COMMON-NEXT: #dbg_value(i32 %p.sroa.0.8.extract.trunc, ![[p:[0-9]+]], !DIExpression(DW_OP_LLVM_extract_bits_zext, 0, 32)
+ call void @llvm.dbg.declare(metadata ptr %p, metadata !67, metadata !DIExpression(DW_OP_plus_uconst, 8, DW_OP_LLVM_extract_bits_zext, 0, 32)), !dbg !66
+ ;; The expression offset is 6 bytes, with a bit-extract offset of 32 bits from
+ ;; there for a total offset of 80 bits. SROA is going to split the alloca in
+ ;; half (at bit 64). The new expression needs a final bit extract offset of
+ ;; 80-64=16 bits applied to the mem2reg'd value.
+ ; COMMON-NEXT: #dbg_value(i32 %p.sroa.0.8.extract.trunc, ![[q:[0-9]+]], !DIExpression(DW_OP_LLVM_extract_bits_zext, 16, 8)
+ call void @llvm.dbg.declare(metadata ptr %p, metadata !68, metadata !DIExpression(DW_OP_plus_uconst, 6, DW_OP_LLVM_extract_bits_zext, 32, 8)), !dbg !66
+ ;; FIXME: Just as in _Z4fun3v, the offset from the new alloca (2 bytes) is
+ ;; correct but mem2reg needs to change it from an offset to a shift or
+ ;; adjust the bit-extract (e.g., add the 2 byte offset to the existing 8 bit
+ ;; offset for a 24 bit total bit-extract offset).
+ ; COMMON-NEXT: #dbg_value(i32 %p.sroa.0.8.extract.trunc, ![[r:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 2, DW_OP_LLVM_extract_bits_zext, 8, 8)
+ call void @llvm.dbg.declare(metadata ptr %p, metadata !69, metadata !DIExpression(DW_OP_plus_uconst, 10, DW_OP_LLVM_extract_bits_zext, 8, 8)), !dbg !66
+ %2 = load i32, ptr %1, align 4
+ ret i32 %2
}
; COMMON-DAG: ![[x0]] = !DILocalVariable(name: "x",
@@ -200,7 +217,9 @@ entry:
; COMMON-DAG: ![[g]] = !DILocalVariable(name: "g",
; COMMON-DAG: ![[h]] = !DILocalVariable(name: "h",
+; COMMON-DAG: ![[p]] = !DILocalVariable(name: "p"
; COMMON-DAG: ![[q]] = !DILocalVariable(name: "q"
+; COMMON-DAG: ![[r]] = !DILocalVariable(name: "r"
declare void @llvm.dbg.declare(metadata, metadata, metadata)
declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)
@@ -254,3 +273,4 @@ declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias
!66 = !DILocation(line: 9, column: 9, scope: !65)
!67 = !DILocalVariable(name: "p", scope: !65, file: !3, line: 9, type: !13)
!68 = !DILocalVariable(name: "q", scope: !65, file: !3, line: 9, type: !13)
+!69 = !DILocalVariable(name: "r", scope: !65, file: !3, line: 9, type: !13)
More information about the llvm-commits
mailing list