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

[RemoveDIs][DebugInfo] Add interface changes for AT analysis #78460

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

SLTozer
Copy link
Contributor

@SLTozer SLTozer commented Jan 17, 2024

This patch adds the preliminary changes for handling DPValues in AssignmentTrackingAnalysis - very few functional changes are included, but internal data structures have been changed to operate with DPValues as well as Instructions, allowing future patches to process DPValues correctly.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 17, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

This patch adds the preliminary changes for handling DPValues in AssignmentTrackingAnalysis - very few functional changes are included, but internal data structures have been changed to operate with DPValues as well as Instructions, allowing future patches to process DPValues correctly.


Patch is 23.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78460.diff

4 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfo.h (+13-1)
  • (modified) llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp (+70-21)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+89-21)
  • (modified) llvm/lib/Transforms/Utils/Local.cpp (-14)
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index e634f4bd2f5aae..55b8ce0998413b 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -58,6 +58,7 @@ DISubprogram *getDISubprogram(const MDNode *Scope);
 /// Produce a DebugLoc to use for each dbg.declare that is promoted to a
 /// dbg.value.
 DebugLoc getDebugValueLoc(DbgVariableIntrinsic *DII);
+DebugLoc getDebugValueLoc(DPValue *DPV);
 
 /// Strip debug info in the module if it exists.
 ///
@@ -223,6 +224,11 @@ inline AssignmentMarkerRange getAssignmentMarkers(const Instruction *Inst) {
   else
     return make_range(Value::user_iterator(), Value::user_iterator());
 }
+inline SmallVector<DPValue *> getDPVAssignmentMarkers(const Instruction *Inst) {
+  if (auto *ID = Inst->getMetadata(LLVMContext::MD_DIAssignID))
+    return cast<DIAssignID>(ID)->getAllDPValueUsers();
+  return {};
+}
 
 /// Delete the llvm.dbg.assign intrinsics linked to \p Inst.
 void deleteAssignmentMarkers(const Instruction *Inst);
@@ -244,7 +250,11 @@ void deleteAll(Function *F);
 /// Result contains a zero-sized fragment if there's no intersect.
 bool calculateFragmentIntersect(
     const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
-    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DAI,
+    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DbgAssign,
+    std::optional<DIExpression::FragmentInfo> &Result);
+bool calculateFragmentIntersect(
+    const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
+    uint64_t SliceSizeInBits, const DPValue *DPVAssign,
     std::optional<DIExpression::FragmentInfo> &Result);
 
 /// Helper struct for trackAssignments, below. We don't use the similar
@@ -259,6 +269,8 @@ struct VarRecord {
 
   VarRecord(DbgVariableIntrinsic *DVI)
       : Var(DVI->getVariable()), DL(getDebugValueLoc(DVI)) {}
+  VarRecord(DPValue *DPV)
+      : Var(DPV->getVariable()), DL(getDebugValueLoc(DPV)) {}
   VarRecord(DILocalVariable *Var, DILocation *DL) : Var(Var), DL(DL) {}
   friend bool operator<(const VarRecord &LHS, const VarRecord &RHS) {
     return std::tie(LHS.Var, LHS.DL) < std::tie(RHS.Var, RHS.DL);
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index eb372655e5f165..497d9391a3b954 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -20,6 +20,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DebugProgramInstruction.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
@@ -81,6 +82,19 @@ template <> struct llvm::DenseMapInfo<VariableID> {
   }
 };
 
+using VarLocInsertPt = PointerUnion<const Instruction *, const DPValue *>;
+
+namespace std {
+template <> struct hash<VarLocInsertPt> {
+  using argument_type = VarLocInsertPt;
+  using result_type = std::size_t;
+
+  result_type operator()(const argument_type &Arg) const {
+    return std::hash<void *>()(Arg.getOpaqueValue());
+  }
+};
+} // namespace std
+
 /// Helper class to build FunctionVarLocs, since that class isn't easy to
 /// modify. TODO: There's not a great deal of value in the split, it could be
 /// worth merging the two classes.
@@ -89,8 +103,7 @@ class FunctionVarLocsBuilder {
   UniqueVector<DebugVariable> Variables;
   // Use an unordered_map so we don't invalidate iterators after
   // insert/modifications.
-  std::unordered_map<const Instruction *, SmallVector<VarLocInfo>>
-      VarLocsBeforeInst;
+  std::unordered_map<VarLocInsertPt, SmallVector<VarLocInfo>> VarLocsBeforeInst;
 
   SmallVector<VarLocInfo> SingleLocVars;
 
@@ -109,7 +122,7 @@ class FunctionVarLocsBuilder {
 
   /// Return ptr to wedge of defs or nullptr if no defs come just before /p
   /// Before.
-  const SmallVectorImpl<VarLocInfo> *getWedge(const Instruction *Before) const {
+  const SmallVectorImpl<VarLocInfo> *getWedge(VarLocInsertPt Before) const {
     auto R = VarLocsBeforeInst.find(Before);
     if (R == VarLocsBeforeInst.end())
       return nullptr;
@@ -117,7 +130,7 @@ class FunctionVarLocsBuilder {
   }
 
   /// Replace the defs that come just before /p Before with /p Wedge.
-  void setWedge(const Instruction *Before, SmallVector<VarLocInfo> &&Wedge) {
+  void setWedge(VarLocInsertPt Before, SmallVector<VarLocInfo> &&Wedge) {
     VarLocsBeforeInst[Before] = std::move(Wedge);
   }
 
@@ -133,7 +146,7 @@ class FunctionVarLocsBuilder {
   }
 
   /// Add a def to the wedge of defs just before /p Before.
-  void addVarLoc(Instruction *Before, DebugVariable Var, DIExpression *Expr,
+  void addVarLoc(VarLocInsertPt Before, DebugVariable Var, DIExpression *Expr,
                  DebugLoc DL, RawLocationWrapper R) {
     VarLocInfo VarLoc;
     VarLoc.VariableID = insertVariable(Var);
@@ -201,15 +214,31 @@ void FunctionVarLocs::init(FunctionVarLocsBuilder &Builder) {
   SingleVarLocEnd = VarLocRecords.size();
 
   // Insert a contiguous block of VarLocInfos for each instruction, mapping it
-  // to the start and end position in the vector with VarLocsBeforeInst.
+  // to the start and end position in the vector with VarLocsBeforeInst. This
+  // block includes VarLocs for any DPValues attached to that instruction.
   for (auto &P : Builder.VarLocsBeforeInst) {
+    // Process VarLocs attached to a DPValue alongside their marker Instruction.
+    if (isa<const DPValue *>(P.first))
+      continue;
+    const Instruction *I = cast<const Instruction *>(P.first);
     unsigned BlockStart = VarLocRecords.size();
+    // Any VarLocInfos attached to a DPValue should now be remapped to their
+    // marker Instruction, in order of DPValue appearance and prior to any
+    // VarLocInfos attached directly to that instruction.
+    for (const DPValue &DPV : I->getDbgValueRange()) {
+      // Even though DPV defines a variable location, VarLocsBeforeInst can
+      // still be empty if that VarLoc was redundant.
+      if (!Builder.VarLocsBeforeInst.count(&DPV))
+        continue;
+      for (const VarLocInfo &VarLoc : Builder.VarLocsBeforeInst[&DPV])
+        VarLocRecords.emplace_back(VarLoc);
+    }
     for (const VarLocInfo &VarLoc : P.second)
       VarLocRecords.emplace_back(VarLoc);
     unsigned BlockEnd = VarLocRecords.size();
     // Record the start and end indices.
     if (BlockEnd != BlockStart)
-      VarLocsBeforeInst[P.first] = {BlockStart, BlockEnd};
+      VarLocsBeforeInst[I] = {BlockStart, BlockEnd};
   }
 
   // Copy the Variables vector from the builder's UniqueVector.
@@ -370,7 +399,7 @@ class MemLocFragmentFill {
     unsigned SizeInBits;
     DebugLoc DL;
   };
-  using InsertMap = MapVector<Instruction *, SmallVector<FragMemLoc>>;
+  using InsertMap = MapVector<VarLocInsertPt, SmallVector<FragMemLoc>>;
 
   /// BBInsertBeforeMap holds a description for the set of location defs to be
   /// inserted after the analysis is complete. It is updated during the dataflow
@@ -590,7 +619,7 @@ class MemLocFragmentFill {
     return /*Changed=*/false;
   }
 
-  void insertMemLoc(BasicBlock &BB, Instruction &Before, unsigned Var,
+  void insertMemLoc(BasicBlock &BB, VarLocInsertPt Before, unsigned Var,
                     unsigned StartBit, unsigned EndBit, unsigned Base,
                     DebugLoc DL) {
     assert(StartBit < EndBit && "Cannot create fragment of size <= 0");
@@ -603,7 +632,7 @@ class MemLocFragmentFill {
     assert(Base && "Expected a non-zero ID for Base address");
     Loc.Base = Base;
     Loc.DL = DL;
-    BBInsertBeforeMap[&BB][&Before].push_back(Loc);
+    BBInsertBeforeMap[&BB][Before].push_back(Loc);
     LLVM_DEBUG(dbgs() << "Add mem def for " << Aggregates[Var].first->getName()
                       << " bits [" << StartBit << ", " << EndBit << ")\n");
   }
@@ -612,7 +641,7 @@ class MemLocFragmentFill {
   /// in \p FragMap starts before \p StartBit or ends after \p EndBit (which
   /// indicates - assuming StartBit->EndBit has just been inserted - that the
   /// slice has been coalesced in the map).
-  void coalesceFragments(BasicBlock &BB, Instruction &Before, unsigned Var,
+  void coalesceFragments(BasicBlock &BB, VarLocInsertPt Before, unsigned Var,
                          unsigned StartBit, unsigned EndBit, unsigned Base,
                          DebugLoc DL, const FragsInMemMap &FragMap) {
     if (!CoalesceAdjacentFragments)
@@ -633,7 +662,7 @@ class MemLocFragmentFill {
                  Base, DL);
   }
 
-  void addDef(const VarLocInfo &VarLoc, Instruction &Before, BasicBlock &BB,
+  void addDef(const VarLocInfo &VarLoc, VarLocInsertPt Before, BasicBlock &BB,
               VarFragMap &LiveSet) {
     DebugVariable DbgVar = FnVarLocs->getVariable(VarLoc.VariableID);
     if (skipVariable(DbgVar.getVariable()))
@@ -802,7 +831,7 @@ class MemLocFragmentFill {
     for (auto &I : BB) {
       if (const auto *Locs = FnVarLocs->getWedge(&I)) {
         for (const VarLocInfo &Loc : *Locs) {
-          addDef(Loc, I, *I.getParent(), LiveSet);
+          addDef(Loc, &I, *I.getParent(), LiveSet);
         }
       }
     }
@@ -923,7 +952,7 @@ class MemLocFragmentFill {
     for (auto &Pair : BBInsertBeforeMap) {
       InsertMap &Map = Pair.second;
       for (auto &Pair : Map) {
-        Instruction *InsertBefore = Pair.first;
+        auto InsertBefore = Pair.first;
         assert(InsertBefore && "should never be null");
         auto FragMemLocs = Pair.second;
         auto &Ctx = Fn.getContext();
@@ -1056,11 +1085,12 @@ class AssignmentTrackingLowering {
   UntaggedStoreAssignmentMap UntaggedStoreVars;
 
   // Machinery to defer inserting dbg.values.
-  using InsertMap = MapVector<Instruction *, SmallVector<VarLocInfo>>;
-  InsertMap InsertBeforeMap;
+  using InstInsertMap = MapVector<VarLocInsertPt, SmallVector<VarLocInfo>>;
+  InstInsertMap InsertBeforeMap;
   /// Clear the location definitions currently cached for insertion after /p
   /// After.
   void resetInsertionPoint(Instruction &After);
+  void resetInsertionPoint(DPValue &After);
   void emitDbgValue(LocKind Kind, const DbgVariableIntrinsic *Source,
                     Instruction *After);
 
@@ -1418,6 +1448,24 @@ const char *locStr(AssignmentTrackingLowering::LocKind Loc) {
 }
 #endif
 
+VarLocInsertPt getNextNode(const DPValue *DPV) {
+  auto NextIt = ++(DPV->getIterator());
+  if (NextIt == DPV->getMarker()->getDbgValueRange().end())
+    return DPV->getMarker()->MarkedInstr;
+  return &*NextIt;
+}
+VarLocInsertPt getNextNode(const Instruction *Inst) {
+  const Instruction *Next = Inst->getNextNode();
+  if (!Next->hasDbgValues())
+    return Next;
+  return &*Next->getDbgValueRange().begin();
+}
+VarLocInsertPt getNextNode(VarLocInsertPt InsertPt) {
+  if (isa<const Instruction *>(InsertPt))
+    return getNextNode(cast<const Instruction *>(InsertPt));
+  return getNextNode(cast<const DPValue *>(InsertPt));
+}
+
 void AssignmentTrackingLowering::emitDbgValue(
     AssignmentTrackingLowering::LocKind Kind,
     const DbgVariableIntrinsic *Source, Instruction *After) {
@@ -1430,7 +1478,7 @@ void AssignmentTrackingLowering::emitDbgValue(
           PoisonValue::get(Type::getInt1Ty(Source->getContext())));
 
     // Find a suitable insert point.
-    Instruction *InsertBefore = After->getNextNode();
+    auto InsertBefore = getNextNode(After);
     assert(InsertBefore && "Shouldn't be inserting after a terminator");
 
     VariableID Var = getVariableID(DebugVariable(Source));
@@ -1538,8 +1586,9 @@ void AssignmentTrackingLowering::processUntaggedInstruction(
     Ops.push_back(dwarf::DW_OP_deref);
     DIE = DIExpression::prependOpcodes(DIE, Ops, /*StackValue=*/false,
                                        /*EntryValue=*/false);
-    // Find a suitable insert point.
-    Instruction *InsertBefore = I.getNextNode();
+    // Find a suitable insert point, before the next instruction or DPValue
+    // after I.
+    auto InsertBefore = getNextNode(&I);
     assert(InsertBefore && "Shouldn't be inserting after a terminator");
 
     // Get DILocation for this unrecorded assignment.
@@ -1710,8 +1759,8 @@ void AssignmentTrackingLowering::processDbgValue(DbgValueInst &DVI,
   emitDbgValue(LocKind::Val, &DVI, &DVI);
 }
 
-static bool hasZeroSizedFragment(DbgVariableIntrinsic &DVI) {
-  if (auto F = DVI.getExpression()->getFragmentInfo())
+template <typename T> static bool hasZeroSizedFragment(T &DbgValue) {
+  if (auto F = DbgValue.getExpression()->getFragmentInfo())
     return F->SizeInBits == 0;
   return false;
 }
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 312d670d5b30e7..fcd3f77f8f6e2b 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -163,6 +163,18 @@ DebugLoc llvm::getDebugValueLoc(DbgVariableIntrinsic *DII) {
   return DILocation::get(DII->getContext(), 0, 0, Scope, InlinedAt);
 }
 
+DebugLoc llvm::getDebugValueLoc(DPValue *DPV) {
+  // Original dbg.declare must have a location.
+  const DebugLoc &DeclareLoc = DPV->getDebugLoc();
+  MDNode *Scope = DeclareLoc.getScope();
+  DILocation *InlinedAt = DeclareLoc.getInlinedAt();
+  // Because no machine insts can come from debug intrinsics, only the scope
+  // and inlinedAt is significant. Zero line numbers are used in case this
+  // DebugLoc leaks into any adjacent instructions. Produce an unknown location
+  // with the correct scope / inlinedAt fields.
+  return DILocation::get(DPV->getContext(), 0, 0, Scope, InlinedAt);
+}
+
 //===----------------------------------------------------------------------===//
 // DebugInfoFinder implementations.
 //===----------------------------------------------------------------------===//
@@ -1779,11 +1791,14 @@ AssignmentMarkerRange at::getAssignmentMarkers(DIAssignID *ID) {
 
 void at::deleteAssignmentMarkers(const Instruction *Inst) {
   auto Range = getAssignmentMarkers(Inst);
-  if (Range.empty())
+  SmallVector<DPValue *> DPVAssigns = getDPVAssignmentMarkers(Inst);
+  if (Range.empty() && DPVAssigns.empty())
     return;
   SmallVector<DbgAssignIntrinsic *> ToDelete(Range.begin(), Range.end());
   for (auto *DAI : ToDelete)
     DAI->eraseFromParent();
+  for (auto *DPV : DPVAssigns)
+    DPV->eraseFromParent();
 }
 
 void at::RAUW(DIAssignID *Old, DIAssignID *New) {
@@ -1813,9 +1828,34 @@ void at::deleteAll(Function *F) {
     DAI->eraseFromParent();
 }
 
-bool at::calculateFragmentIntersect(
+/// Get the FragmentInfo for the variable if it exists, otherwise return a
+/// FragmentInfo that covers the entire variable if the variable size is
+/// known, otherwise return a zero-sized fragment.
+static DIExpression::FragmentInfo
+getFragmentOrEntireVariable(const DPValue *DPV) {
+  DIExpression::FragmentInfo VariableSlice(0, 0);
+  // Get the fragment or variable size, or zero.
+  if (auto Sz = DPV->getFragmentSizeInBits())
+    VariableSlice.SizeInBits = *Sz;
+  if (auto Frag = DPV->getExpression()->getFragmentInfo())
+    VariableSlice.OffsetInBits = Frag->OffsetInBits;
+  return VariableSlice;
+}
+
+static DIExpression::FragmentInfo
+getFragmentOrEntireVariable(const DbgVariableIntrinsic *DVI) {
+  DIExpression::FragmentInfo VariableSlice(0, 0);
+  // Get the fragment or variable size, or zero.
+  if (auto Sz = DVI->getFragmentSizeInBits())
+    VariableSlice.SizeInBits = *Sz;
+  if (auto Frag = DVI->getExpression()->getFragmentInfo())
+    VariableSlice.OffsetInBits = Frag->OffsetInBits;
+  return VariableSlice;
+}
+template <typename T>
+bool calculateFragmentIntersectImpl(
     const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
-    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DAI,
+    uint64_t SliceSizeInBits, const T *AssignRecord,
     std::optional<DIExpression::FragmentInfo> &Result) {
   // There are multiple offsets at play in this function, so let's break it
   // down. Starting with how variables may be stored in allocas:
@@ -1844,7 +1884,7 @@ bool at::calculateFragmentIntersect(
   // dbg.assign, that has been killed, if any.
   //
   //     calculateFragmentIntersect(..., SliceOffsetInBits=0,
-  //                 SliceSizeInBits=32, Dest=%dest, DAI=dbg.assign)
+  //                 SliceSizeInBits=32, Dest=%dest, Assign=dbg.assign)
   //
   // Drawing the store (s) in memory followed by the shortened version ($),
   // then the dbg.assign (d), with the fragment information on a seperate scale
@@ -1857,7 +1897,8 @@ bool at::calculateFragmentIntersect(
   //        |      |
   //       s[######] - Original stores 64 bits to Dest.
   //       $----[##] - DSE says the lower 32 bits are dead, to be removed.
-  //       d    [##] - DAI's address-modifying expression adds 4 bytes to dest.
+  //       d    [##] - Assign's address-modifying expression adds 4 bytes to
+  //       dest.
   // Variable   |  |
   // Fragment   128|
   //  Offsets      159
@@ -1872,10 +1913,10 @@ bool at::calculateFragmentIntersect(
   //
   // 3. That offset along with the store size (32) represents the bits of the
   //    variable that'd be affected by the store. Call it SliceOfVariable.
-  //    Intersect that with DAI's fragment info:
-  //      SliceOfVariable ∩ DAI_fragment = none
+  //    Intersect that with Assign's fragment info:
+  //      SliceOfVariable ∩ Assign_fragment = none
   //
-  // In this case: none of the dead bits of the store affect DAI.
+  // In this case: none of the dead bits of the store affect Assign.
   //
   // # Example 2
   // Similar example with the same goal. This time the upper 16 bits
@@ -1886,7 +1927,7 @@ bool at::calculateFragmentIntersect(
   //               !DIExpression(DW_OP_plus_uconst, 4))
   //
   //     calculateFragmentIntersect(..., SliceOffsetInBits=48,
-  //                 SliceSizeInBits=16, Dest=%dest, DAI=dbg.assign)
+  //                 SliceSizeInBits=16, Dest=%dest, Assign=dbg.assign)
   //
   // Memory
   // offset
@@ -1895,7 +1936,8 @@ bool at::calculateFragmentIntersect(
   //        |      |
   //       s[######] - Original stores 64 bits to Dest.
   //       $[####]-- - DSE says the upper 16 bits are dead, to be removed.
-  //       d    [##] - DAI's address-modifying expression adds 4 bytes to dest.
+  //       d    [##] - Assign's address-modifying expression adds 4 bytes to
+  //       dest.
   // Variable   |  |
   // Fragment   128|
   //  Offsets      159
@@ -1904,13 +1946,14 @@ bool at::calculateFragmentIntersect(
   // 1. SliceOffsetInBits:48 + VarFrag.OffsetInBits:128 = 176
   // 2. 176 - (expression_offset:32 + (d.address - dest):0) = 144
   // 3. SliceOfVariable offset = 144, size = 16:
-  //      SliceOfVariable ∩ DAI_fragment = (offset: 144, size: 16)
-  // SliceOfVariable tells us the bits of the variable described by DAI that are
-  // affected by the DSE.
-  if (DAI->isKillAddress())
+  //      SliceOfVariable ∩ Assign_fragment = (offset: 144, size: 16)
+  // SliceOfVariable tells us the bits of the variable described by Assign that
+  // are affected by the DSE.
+  if (AssignRecord->isKillAddress())
     return false;
 
-  DIExpression::FragmentInfo VarFrag = DAI->getFragmentOrEntireVariable();
+  DIExpression::FragmentInfo VarFrag =
+      getFragmentOrEntireVariable(AssignRecord);
   if (VarFrag.SizeInBits == 0)
     return false; // Variable size is unknown.
 
@@ -1918,12 +1961,14 @@ bool at::calculateFragmentIntersect(
   // address-modifying expression.
   int64_t PointerOffsetInBits;
   {
-    auto DestOffsetInBytes = DAI->getAddress()->getPointerOffsetFrom(Dest, DL);
+    auto DestOffsetInBytes =
+        AssignRecord->getAddress()->getPointerOffsetFrom(Dest, DL);
     if (!DestOffsetInBytes)
       return false; // Can't calculate difference in addresses.
 
     int64_t ExprOffsetInBytes;
-    if (!DAI->getAddressExpression()->extractIfOffset(ExprOffsetInBytes))
+    if (!AssignRecord->getAddressExpression()->extractIfOffset(
+            ExprOffsetInBytes))
       return false;
 
     int64_t PointerOffsetInBytes = *DestOffsetInBytes + ExprOffsetInBytes;
@@ -1937,7 +1982,8 @@ bool at::calculateFragmentIntersect(
   if (NewOffsetInBits < 0)
     return false; // Fragment offsets can only be positive.
   DIExpression::FragmentInfo SliceOfVariable(SliceSizeInBits, NewOffsetInBits);
-  // Intersect the variable slice with DAI's fragment to trim it down to size.
+  // Intersect the variable slice with AssignRecord's fragment to trim it down
+  // to size.
   DIExpression::FragmentInfo TrimmedSlic...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 17, 2024

@llvm/pr-subscribers-llvm-ir

Author: Stephen Tozer (SLTozer)

Changes

This patch adds the preliminary changes for handling DPValues in AssignmentTrackingAnalysis - very few functional changes are included, but internal data structures have been changed to operate with DPValues as well as Instructions, allowing future patches to process DPValues correctly.


Patch is 23.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78460.diff

4 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfo.h (+13-1)
  • (modified) llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp (+70-21)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+89-21)
  • (modified) llvm/lib/Transforms/Utils/Local.cpp (-14)
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index e634f4bd2f5aae..55b8ce0998413b 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -58,6 +58,7 @@ DISubprogram *getDISubprogram(const MDNode *Scope);
 /// Produce a DebugLoc to use for each dbg.declare that is promoted to a
 /// dbg.value.
 DebugLoc getDebugValueLoc(DbgVariableIntrinsic *DII);
+DebugLoc getDebugValueLoc(DPValue *DPV);
 
 /// Strip debug info in the module if it exists.
 ///
@@ -223,6 +224,11 @@ inline AssignmentMarkerRange getAssignmentMarkers(const Instruction *Inst) {
   else
     return make_range(Value::user_iterator(), Value::user_iterator());
 }
+inline SmallVector<DPValue *> getDPVAssignmentMarkers(const Instruction *Inst) {
+  if (auto *ID = Inst->getMetadata(LLVMContext::MD_DIAssignID))
+    return cast<DIAssignID>(ID)->getAllDPValueUsers();
+  return {};
+}
 
 /// Delete the llvm.dbg.assign intrinsics linked to \p Inst.
 void deleteAssignmentMarkers(const Instruction *Inst);
@@ -244,7 +250,11 @@ void deleteAll(Function *F);
 /// Result contains a zero-sized fragment if there's no intersect.
 bool calculateFragmentIntersect(
     const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
-    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DAI,
+    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DbgAssign,
+    std::optional<DIExpression::FragmentInfo> &Result);
+bool calculateFragmentIntersect(
+    const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
+    uint64_t SliceSizeInBits, const DPValue *DPVAssign,
     std::optional<DIExpression::FragmentInfo> &Result);
 
 /// Helper struct for trackAssignments, below. We don't use the similar
@@ -259,6 +269,8 @@ struct VarRecord {
 
   VarRecord(DbgVariableIntrinsic *DVI)
       : Var(DVI->getVariable()), DL(getDebugValueLoc(DVI)) {}
+  VarRecord(DPValue *DPV)
+      : Var(DPV->getVariable()), DL(getDebugValueLoc(DPV)) {}
   VarRecord(DILocalVariable *Var, DILocation *DL) : Var(Var), DL(DL) {}
   friend bool operator<(const VarRecord &LHS, const VarRecord &RHS) {
     return std::tie(LHS.Var, LHS.DL) < std::tie(RHS.Var, RHS.DL);
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index eb372655e5f165..497d9391a3b954 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -20,6 +20,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DebugProgramInstruction.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
@@ -81,6 +82,19 @@ template <> struct llvm::DenseMapInfo<VariableID> {
   }
 };
 
+using VarLocInsertPt = PointerUnion<const Instruction *, const DPValue *>;
+
+namespace std {
+template <> struct hash<VarLocInsertPt> {
+  using argument_type = VarLocInsertPt;
+  using result_type = std::size_t;
+
+  result_type operator()(const argument_type &Arg) const {
+    return std::hash<void *>()(Arg.getOpaqueValue());
+  }
+};
+} // namespace std
+
 /// Helper class to build FunctionVarLocs, since that class isn't easy to
 /// modify. TODO: There's not a great deal of value in the split, it could be
 /// worth merging the two classes.
@@ -89,8 +103,7 @@ class FunctionVarLocsBuilder {
   UniqueVector<DebugVariable> Variables;
   // Use an unordered_map so we don't invalidate iterators after
   // insert/modifications.
-  std::unordered_map<const Instruction *, SmallVector<VarLocInfo>>
-      VarLocsBeforeInst;
+  std::unordered_map<VarLocInsertPt, SmallVector<VarLocInfo>> VarLocsBeforeInst;
 
   SmallVector<VarLocInfo> SingleLocVars;
 
@@ -109,7 +122,7 @@ class FunctionVarLocsBuilder {
 
   /// Return ptr to wedge of defs or nullptr if no defs come just before /p
   /// Before.
-  const SmallVectorImpl<VarLocInfo> *getWedge(const Instruction *Before) const {
+  const SmallVectorImpl<VarLocInfo> *getWedge(VarLocInsertPt Before) const {
     auto R = VarLocsBeforeInst.find(Before);
     if (R == VarLocsBeforeInst.end())
       return nullptr;
@@ -117,7 +130,7 @@ class FunctionVarLocsBuilder {
   }
 
   /// Replace the defs that come just before /p Before with /p Wedge.
-  void setWedge(const Instruction *Before, SmallVector<VarLocInfo> &&Wedge) {
+  void setWedge(VarLocInsertPt Before, SmallVector<VarLocInfo> &&Wedge) {
     VarLocsBeforeInst[Before] = std::move(Wedge);
   }
 
@@ -133,7 +146,7 @@ class FunctionVarLocsBuilder {
   }
 
   /// Add a def to the wedge of defs just before /p Before.
-  void addVarLoc(Instruction *Before, DebugVariable Var, DIExpression *Expr,
+  void addVarLoc(VarLocInsertPt Before, DebugVariable Var, DIExpression *Expr,
                  DebugLoc DL, RawLocationWrapper R) {
     VarLocInfo VarLoc;
     VarLoc.VariableID = insertVariable(Var);
@@ -201,15 +214,31 @@ void FunctionVarLocs::init(FunctionVarLocsBuilder &Builder) {
   SingleVarLocEnd = VarLocRecords.size();
 
   // Insert a contiguous block of VarLocInfos for each instruction, mapping it
-  // to the start and end position in the vector with VarLocsBeforeInst.
+  // to the start and end position in the vector with VarLocsBeforeInst. This
+  // block includes VarLocs for any DPValues attached to that instruction.
   for (auto &P : Builder.VarLocsBeforeInst) {
+    // Process VarLocs attached to a DPValue alongside their marker Instruction.
+    if (isa<const DPValue *>(P.first))
+      continue;
+    const Instruction *I = cast<const Instruction *>(P.first);
     unsigned BlockStart = VarLocRecords.size();
+    // Any VarLocInfos attached to a DPValue should now be remapped to their
+    // marker Instruction, in order of DPValue appearance and prior to any
+    // VarLocInfos attached directly to that instruction.
+    for (const DPValue &DPV : I->getDbgValueRange()) {
+      // Even though DPV defines a variable location, VarLocsBeforeInst can
+      // still be empty if that VarLoc was redundant.
+      if (!Builder.VarLocsBeforeInst.count(&DPV))
+        continue;
+      for (const VarLocInfo &VarLoc : Builder.VarLocsBeforeInst[&DPV])
+        VarLocRecords.emplace_back(VarLoc);
+    }
     for (const VarLocInfo &VarLoc : P.second)
       VarLocRecords.emplace_back(VarLoc);
     unsigned BlockEnd = VarLocRecords.size();
     // Record the start and end indices.
     if (BlockEnd != BlockStart)
-      VarLocsBeforeInst[P.first] = {BlockStart, BlockEnd};
+      VarLocsBeforeInst[I] = {BlockStart, BlockEnd};
   }
 
   // Copy the Variables vector from the builder's UniqueVector.
@@ -370,7 +399,7 @@ class MemLocFragmentFill {
     unsigned SizeInBits;
     DebugLoc DL;
   };
-  using InsertMap = MapVector<Instruction *, SmallVector<FragMemLoc>>;
+  using InsertMap = MapVector<VarLocInsertPt, SmallVector<FragMemLoc>>;
 
   /// BBInsertBeforeMap holds a description for the set of location defs to be
   /// inserted after the analysis is complete. It is updated during the dataflow
@@ -590,7 +619,7 @@ class MemLocFragmentFill {
     return /*Changed=*/false;
   }
 
-  void insertMemLoc(BasicBlock &BB, Instruction &Before, unsigned Var,
+  void insertMemLoc(BasicBlock &BB, VarLocInsertPt Before, unsigned Var,
                     unsigned StartBit, unsigned EndBit, unsigned Base,
                     DebugLoc DL) {
     assert(StartBit < EndBit && "Cannot create fragment of size <= 0");
@@ -603,7 +632,7 @@ class MemLocFragmentFill {
     assert(Base && "Expected a non-zero ID for Base address");
     Loc.Base = Base;
     Loc.DL = DL;
-    BBInsertBeforeMap[&BB][&Before].push_back(Loc);
+    BBInsertBeforeMap[&BB][Before].push_back(Loc);
     LLVM_DEBUG(dbgs() << "Add mem def for " << Aggregates[Var].first->getName()
                       << " bits [" << StartBit << ", " << EndBit << ")\n");
   }
@@ -612,7 +641,7 @@ class MemLocFragmentFill {
   /// in \p FragMap starts before \p StartBit or ends after \p EndBit (which
   /// indicates - assuming StartBit->EndBit has just been inserted - that the
   /// slice has been coalesced in the map).
-  void coalesceFragments(BasicBlock &BB, Instruction &Before, unsigned Var,
+  void coalesceFragments(BasicBlock &BB, VarLocInsertPt Before, unsigned Var,
                          unsigned StartBit, unsigned EndBit, unsigned Base,
                          DebugLoc DL, const FragsInMemMap &FragMap) {
     if (!CoalesceAdjacentFragments)
@@ -633,7 +662,7 @@ class MemLocFragmentFill {
                  Base, DL);
   }
 
-  void addDef(const VarLocInfo &VarLoc, Instruction &Before, BasicBlock &BB,
+  void addDef(const VarLocInfo &VarLoc, VarLocInsertPt Before, BasicBlock &BB,
               VarFragMap &LiveSet) {
     DebugVariable DbgVar = FnVarLocs->getVariable(VarLoc.VariableID);
     if (skipVariable(DbgVar.getVariable()))
@@ -802,7 +831,7 @@ class MemLocFragmentFill {
     for (auto &I : BB) {
       if (const auto *Locs = FnVarLocs->getWedge(&I)) {
         for (const VarLocInfo &Loc : *Locs) {
-          addDef(Loc, I, *I.getParent(), LiveSet);
+          addDef(Loc, &I, *I.getParent(), LiveSet);
         }
       }
     }
@@ -923,7 +952,7 @@ class MemLocFragmentFill {
     for (auto &Pair : BBInsertBeforeMap) {
       InsertMap &Map = Pair.second;
       for (auto &Pair : Map) {
-        Instruction *InsertBefore = Pair.first;
+        auto InsertBefore = Pair.first;
         assert(InsertBefore && "should never be null");
         auto FragMemLocs = Pair.second;
         auto &Ctx = Fn.getContext();
@@ -1056,11 +1085,12 @@ class AssignmentTrackingLowering {
   UntaggedStoreAssignmentMap UntaggedStoreVars;
 
   // Machinery to defer inserting dbg.values.
-  using InsertMap = MapVector<Instruction *, SmallVector<VarLocInfo>>;
-  InsertMap InsertBeforeMap;
+  using InstInsertMap = MapVector<VarLocInsertPt, SmallVector<VarLocInfo>>;
+  InstInsertMap InsertBeforeMap;
   /// Clear the location definitions currently cached for insertion after /p
   /// After.
   void resetInsertionPoint(Instruction &After);
+  void resetInsertionPoint(DPValue &After);
   void emitDbgValue(LocKind Kind, const DbgVariableIntrinsic *Source,
                     Instruction *After);
 
@@ -1418,6 +1448,24 @@ const char *locStr(AssignmentTrackingLowering::LocKind Loc) {
 }
 #endif
 
+VarLocInsertPt getNextNode(const DPValue *DPV) {
+  auto NextIt = ++(DPV->getIterator());
+  if (NextIt == DPV->getMarker()->getDbgValueRange().end())
+    return DPV->getMarker()->MarkedInstr;
+  return &*NextIt;
+}
+VarLocInsertPt getNextNode(const Instruction *Inst) {
+  const Instruction *Next = Inst->getNextNode();
+  if (!Next->hasDbgValues())
+    return Next;
+  return &*Next->getDbgValueRange().begin();
+}
+VarLocInsertPt getNextNode(VarLocInsertPt InsertPt) {
+  if (isa<const Instruction *>(InsertPt))
+    return getNextNode(cast<const Instruction *>(InsertPt));
+  return getNextNode(cast<const DPValue *>(InsertPt));
+}
+
 void AssignmentTrackingLowering::emitDbgValue(
     AssignmentTrackingLowering::LocKind Kind,
     const DbgVariableIntrinsic *Source, Instruction *After) {
@@ -1430,7 +1478,7 @@ void AssignmentTrackingLowering::emitDbgValue(
           PoisonValue::get(Type::getInt1Ty(Source->getContext())));
 
     // Find a suitable insert point.
-    Instruction *InsertBefore = After->getNextNode();
+    auto InsertBefore = getNextNode(After);
     assert(InsertBefore && "Shouldn't be inserting after a terminator");
 
     VariableID Var = getVariableID(DebugVariable(Source));
@@ -1538,8 +1586,9 @@ void AssignmentTrackingLowering::processUntaggedInstruction(
     Ops.push_back(dwarf::DW_OP_deref);
     DIE = DIExpression::prependOpcodes(DIE, Ops, /*StackValue=*/false,
                                        /*EntryValue=*/false);
-    // Find a suitable insert point.
-    Instruction *InsertBefore = I.getNextNode();
+    // Find a suitable insert point, before the next instruction or DPValue
+    // after I.
+    auto InsertBefore = getNextNode(&I);
     assert(InsertBefore && "Shouldn't be inserting after a terminator");
 
     // Get DILocation for this unrecorded assignment.
@@ -1710,8 +1759,8 @@ void AssignmentTrackingLowering::processDbgValue(DbgValueInst &DVI,
   emitDbgValue(LocKind::Val, &DVI, &DVI);
 }
 
-static bool hasZeroSizedFragment(DbgVariableIntrinsic &DVI) {
-  if (auto F = DVI.getExpression()->getFragmentInfo())
+template <typename T> static bool hasZeroSizedFragment(T &DbgValue) {
+  if (auto F = DbgValue.getExpression()->getFragmentInfo())
     return F->SizeInBits == 0;
   return false;
 }
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 312d670d5b30e7..fcd3f77f8f6e2b 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -163,6 +163,18 @@ DebugLoc llvm::getDebugValueLoc(DbgVariableIntrinsic *DII) {
   return DILocation::get(DII->getContext(), 0, 0, Scope, InlinedAt);
 }
 
+DebugLoc llvm::getDebugValueLoc(DPValue *DPV) {
+  // Original dbg.declare must have a location.
+  const DebugLoc &DeclareLoc = DPV->getDebugLoc();
+  MDNode *Scope = DeclareLoc.getScope();
+  DILocation *InlinedAt = DeclareLoc.getInlinedAt();
+  // Because no machine insts can come from debug intrinsics, only the scope
+  // and inlinedAt is significant. Zero line numbers are used in case this
+  // DebugLoc leaks into any adjacent instructions. Produce an unknown location
+  // with the correct scope / inlinedAt fields.
+  return DILocation::get(DPV->getContext(), 0, 0, Scope, InlinedAt);
+}
+
 //===----------------------------------------------------------------------===//
 // DebugInfoFinder implementations.
 //===----------------------------------------------------------------------===//
@@ -1779,11 +1791,14 @@ AssignmentMarkerRange at::getAssignmentMarkers(DIAssignID *ID) {
 
 void at::deleteAssignmentMarkers(const Instruction *Inst) {
   auto Range = getAssignmentMarkers(Inst);
-  if (Range.empty())
+  SmallVector<DPValue *> DPVAssigns = getDPVAssignmentMarkers(Inst);
+  if (Range.empty() && DPVAssigns.empty())
     return;
   SmallVector<DbgAssignIntrinsic *> ToDelete(Range.begin(), Range.end());
   for (auto *DAI : ToDelete)
     DAI->eraseFromParent();
+  for (auto *DPV : DPVAssigns)
+    DPV->eraseFromParent();
 }
 
 void at::RAUW(DIAssignID *Old, DIAssignID *New) {
@@ -1813,9 +1828,34 @@ void at::deleteAll(Function *F) {
     DAI->eraseFromParent();
 }
 
-bool at::calculateFragmentIntersect(
+/// Get the FragmentInfo for the variable if it exists, otherwise return a
+/// FragmentInfo that covers the entire variable if the variable size is
+/// known, otherwise return a zero-sized fragment.
+static DIExpression::FragmentInfo
+getFragmentOrEntireVariable(const DPValue *DPV) {
+  DIExpression::FragmentInfo VariableSlice(0, 0);
+  // Get the fragment or variable size, or zero.
+  if (auto Sz = DPV->getFragmentSizeInBits())
+    VariableSlice.SizeInBits = *Sz;
+  if (auto Frag = DPV->getExpression()->getFragmentInfo())
+    VariableSlice.OffsetInBits = Frag->OffsetInBits;
+  return VariableSlice;
+}
+
+static DIExpression::FragmentInfo
+getFragmentOrEntireVariable(const DbgVariableIntrinsic *DVI) {
+  DIExpression::FragmentInfo VariableSlice(0, 0);
+  // Get the fragment or variable size, or zero.
+  if (auto Sz = DVI->getFragmentSizeInBits())
+    VariableSlice.SizeInBits = *Sz;
+  if (auto Frag = DVI->getExpression()->getFragmentInfo())
+    VariableSlice.OffsetInBits = Frag->OffsetInBits;
+  return VariableSlice;
+}
+template <typename T>
+bool calculateFragmentIntersectImpl(
     const DataLayout &DL, const Value *Dest, uint64_t SliceOffsetInBits,
-    uint64_t SliceSizeInBits, const DbgAssignIntrinsic *DAI,
+    uint64_t SliceSizeInBits, const T *AssignRecord,
     std::optional<DIExpression::FragmentInfo> &Result) {
   // There are multiple offsets at play in this function, so let's break it
   // down. Starting with how variables may be stored in allocas:
@@ -1844,7 +1884,7 @@ bool at::calculateFragmentIntersect(
   // dbg.assign, that has been killed, if any.
   //
   //     calculateFragmentIntersect(..., SliceOffsetInBits=0,
-  //                 SliceSizeInBits=32, Dest=%dest, DAI=dbg.assign)
+  //                 SliceSizeInBits=32, Dest=%dest, Assign=dbg.assign)
   //
   // Drawing the store (s) in memory followed by the shortened version ($),
   // then the dbg.assign (d), with the fragment information on a seperate scale
@@ -1857,7 +1897,8 @@ bool at::calculateFragmentIntersect(
   //        |      |
   //       s[######] - Original stores 64 bits to Dest.
   //       $----[##] - DSE says the lower 32 bits are dead, to be removed.
-  //       d    [##] - DAI's address-modifying expression adds 4 bytes to dest.
+  //       d    [##] - Assign's address-modifying expression adds 4 bytes to
+  //       dest.
   // Variable   |  |
   // Fragment   128|
   //  Offsets      159
@@ -1872,10 +1913,10 @@ bool at::calculateFragmentIntersect(
   //
   // 3. That offset along with the store size (32) represents the bits of the
   //    variable that'd be affected by the store. Call it SliceOfVariable.
-  //    Intersect that with DAI's fragment info:
-  //      SliceOfVariable ∩ DAI_fragment = none
+  //    Intersect that with Assign's fragment info:
+  //      SliceOfVariable ∩ Assign_fragment = none
   //
-  // In this case: none of the dead bits of the store affect DAI.
+  // In this case: none of the dead bits of the store affect Assign.
   //
   // # Example 2
   // Similar example with the same goal. This time the upper 16 bits
@@ -1886,7 +1927,7 @@ bool at::calculateFragmentIntersect(
   //               !DIExpression(DW_OP_plus_uconst, 4))
   //
   //     calculateFragmentIntersect(..., SliceOffsetInBits=48,
-  //                 SliceSizeInBits=16, Dest=%dest, DAI=dbg.assign)
+  //                 SliceSizeInBits=16, Dest=%dest, Assign=dbg.assign)
   //
   // Memory
   // offset
@@ -1895,7 +1936,8 @@ bool at::calculateFragmentIntersect(
   //        |      |
   //       s[######] - Original stores 64 bits to Dest.
   //       $[####]-- - DSE says the upper 16 bits are dead, to be removed.
-  //       d    [##] - DAI's address-modifying expression adds 4 bytes to dest.
+  //       d    [##] - Assign's address-modifying expression adds 4 bytes to
+  //       dest.
   // Variable   |  |
   // Fragment   128|
   //  Offsets      159
@@ -1904,13 +1946,14 @@ bool at::calculateFragmentIntersect(
   // 1. SliceOffsetInBits:48 + VarFrag.OffsetInBits:128 = 176
   // 2. 176 - (expression_offset:32 + (d.address - dest):0) = 144
   // 3. SliceOfVariable offset = 144, size = 16:
-  //      SliceOfVariable ∩ DAI_fragment = (offset: 144, size: 16)
-  // SliceOfVariable tells us the bits of the variable described by DAI that are
-  // affected by the DSE.
-  if (DAI->isKillAddress())
+  //      SliceOfVariable ∩ Assign_fragment = (offset: 144, size: 16)
+  // SliceOfVariable tells us the bits of the variable described by Assign that
+  // are affected by the DSE.
+  if (AssignRecord->isKillAddress())
     return false;
 
-  DIExpression::FragmentInfo VarFrag = DAI->getFragmentOrEntireVariable();
+  DIExpression::FragmentInfo VarFrag =
+      getFragmentOrEntireVariable(AssignRecord);
   if (VarFrag.SizeInBits == 0)
     return false; // Variable size is unknown.
 
@@ -1918,12 +1961,14 @@ bool at::calculateFragmentIntersect(
   // address-modifying expression.
   int64_t PointerOffsetInBits;
   {
-    auto DestOffsetInBytes = DAI->getAddress()->getPointerOffsetFrom(Dest, DL);
+    auto DestOffsetInBytes =
+        AssignRecord->getAddress()->getPointerOffsetFrom(Dest, DL);
     if (!DestOffsetInBytes)
       return false; // Can't calculate difference in addresses.
 
     int64_t ExprOffsetInBytes;
-    if (!DAI->getAddressExpression()->extractIfOffset(ExprOffsetInBytes))
+    if (!AssignRecord->getAddressExpression()->extractIfOffset(
+            ExprOffsetInBytes))
       return false;
 
     int64_t PointerOffsetInBytes = *DestOffsetInBytes + ExprOffsetInBytes;
@@ -1937,7 +1982,8 @@ bool at::calculateFragmentIntersect(
   if (NewOffsetInBits < 0)
     return false; // Fragment offsets can only be positive.
   DIExpression::FragmentInfo SliceOfVariable(SliceSizeInBits, NewOffsetInBits);
-  // Intersect the variable slice with DAI's fragment to trim it down to size.
+  // Intersect the variable slice with AssignRecord's fragment to trim it down
+  // to size.
   DIExpression::FragmentInfo TrimmedSlic...
[truncated]

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

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

LGTM - These codepaths will naturally get test coverage in subsequent patches where these functions are used

@SLTozer SLTozer merged commit 6aeb7a7 into llvm:main Jan 22, 2024
6 of 7 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