[llvm] [RemoveDIs][DebugInfo] Handle DPVAssigns in AssignmentTrackingLowering (PR #78980)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 22 06:32:38 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: Stephen Tozer (SLTozer)

<details>
<summary>Changes</summary>

Following on from the previous patch [6aeb7a7](https://github.com/llvm/llvm-project/commit/6aeb7a71d40faed14820523b5be24ff93a4e9bf9), this patch adds the necessary code to process the DPV equivalents of llvm.dbg.assign intrinsics. Most of the content of this patch is simply duplicating existing functionality, using generic code for simple functions and PointerUnions where storage is required. The most complex changes are in the places that iterate over instructions, as iterating over DPValues between instructions is different to iterating over instructions that may or may not be debug intrinsics; this is most complex in `AssignmentTrackingLowering::process`, where I've added some comments to explain the state of the program at each key point depending on whether we are operating on intrinsics or DPValues.

---

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


1 Files Affected:

- (modified) llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp (+266-138) 


``````````diff
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index 52b464cc76af4c..563b9c67f81d39 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -1019,13 +1019,14 @@ class AssignmentTrackingLowering {
   /// i.e. for all values x and y where x != y:
   /// join(x, x) = x
   /// join(x, y) = NoneOrPhi
+  using AssignRecord = PointerUnion<DbgAssignIntrinsic *, DPValue *>;
   struct Assignment {
     enum S { Known, NoneOrPhi } Status;
     /// ID of the assignment. nullptr if Status is not Known.
     DIAssignID *ID;
     /// The dbg.assign that marks this dbg-def. Mem-defs don't use this field.
     /// May be nullptr.
-    DbgAssignIntrinsic *Source;
+    AssignRecord Source;
 
     bool isSameSourceAssignment(const Assignment &Other) const {
       // Don't include Source in the equality check. Assignments are
@@ -1040,31 +1041,51 @@ class AssignmentTrackingLowering {
       else
         OS << "null";
       OS << ", s=";
-      if (Source)
-        OS << *Source;
-      else
+      if (Source.isNull())
         OS << "null";
+      else if (isa<DbgAssignIntrinsic *>(Source))
+        OS << Source.get<DbgAssignIntrinsic *>();
+      else
+        OS << Source.get<DPValue *>();
       OS << ")";
     }
 
     static Assignment make(DIAssignID *ID, DbgAssignIntrinsic *Source) {
       return Assignment(Known, ID, Source);
     }
-    static Assignment makeFromMemDef(DIAssignID *ID) {
-      return Assignment(Known, ID, nullptr);
+    static Assignment make(DIAssignID *ID, DPValue *Source) {
+      assert(Source->isDbgAssign() &&
+             "Cannot make an assignment from a non-assign DPValue");
+      return Assignment(Known, ID, Source);
+    }
+    static Assignment make(DIAssignID *ID, AssignRecord Source) {
+      return Assignment(Known, ID, Source);
     }
-    static Assignment makeNoneOrPhi() {
-      return Assignment(NoneOrPhi, nullptr, nullptr);
+    static Assignment makeFromMemDef(DIAssignID *ID) {
+      return Assignment(Known, ID);
     }
+    static Assignment makeNoneOrPhi() { return Assignment(NoneOrPhi, nullptr); }
     // Again, need a Top value?
-    Assignment()
-        : Status(NoneOrPhi), ID(nullptr), Source(nullptr) {
-    } // Can we delete this?
+    Assignment() : Status(NoneOrPhi), ID(nullptr) {} // Can we delete this?
+    Assignment(S Status, DIAssignID *ID) : Status(Status), ID(ID) {
+      // If the Status is Known then we expect there to be an assignment ID.
+      assert(Status == NoneOrPhi || ID);
+    }
     Assignment(S Status, DIAssignID *ID, DbgAssignIntrinsic *Source)
         : Status(Status), ID(ID), Source(Source) {
       // If the Status is Known then we expect there to be an assignment ID.
       assert(Status == NoneOrPhi || ID);
     }
+    Assignment(S Status, DIAssignID *ID, DPValue *Source)
+        : Status(Status), ID(ID), Source(Source) {
+      // If the Status is Known then we expect there to be an assignment ID.
+      assert(Status == NoneOrPhi || ID);
+    }
+    Assignment(S Status, DIAssignID *ID, AssignRecord Source)
+        : Status(Status), ID(ID), Source(Source) {
+      // If the Status is Known then we expect there to be an assignment ID.
+      assert(Status == NoneOrPhi || ID);
+    }
   };
 
   using AssignmentMap = SmallVector<Assignment>;
@@ -1091,8 +1112,16 @@ class AssignmentTrackingLowering {
   /// After.
   void resetInsertionPoint(Instruction &After);
   void resetInsertionPoint(DPValue &After);
-  void emitDbgValue(LocKind Kind, const DbgVariableIntrinsic *Source,
-                    Instruction *After);
+
+  // emitDbgValue can be called with:
+  //   Source=[AssignRecord|DbgValueInst*|DbgAssignIntrinsic*|DPValue*]
+  // Since AssignRecord can be cast to one of the latter two types, and all
+  // other types have a shared interface, we use a template to handle the latter
+  // three types, and an explicit overload for AssignRecord that forwards to
+  // the template version with the right type.
+  void emitDbgValue(LocKind Kind, AssignRecord Source, VarLocInsertPt After);
+  template <typename T>
+  void emitDbgValue(LocKind Kind, const T Source, VarLocInsertPt After);
 
   static bool mapsAreEqual(const BitVector &Mask, const AssignmentMap &A,
                            const AssignmentMap &B) {
@@ -1317,8 +1346,10 @@ class AssignmentTrackingLowering {
   /// Update \p LiveSet after encountering an instruciton without a DIAssignID
   /// attachment, \p I.
   void processUntaggedInstruction(Instruction &I, BlockInfo *LiveSet);
-  void processDbgAssign(DbgAssignIntrinsic &DAI, BlockInfo *LiveSet);
-  void processDbgValue(DbgValueInst &DVI, BlockInfo *LiveSet);
+  void processDbgAssign(AssignRecord Assign, BlockInfo *LiveSet);
+  void processDPValue(DPValue &DPV, BlockInfo *LiveSet);
+  void processDbgValue(PointerUnion<DbgValueInst *, DPValue *> DbgValueRecord,
+                       BlockInfo *LiveSet);
   /// Add an assignment to memory for the variable /p Var.
   void addMemDef(BlockInfo *LiveSet, VariableID Var, const Assignment &AV);
   /// Add an assignment to the variable /p Var.
@@ -1468,7 +1499,16 @@ VarLocInsertPt getNextNode(VarLocInsertPt InsertPt) {
 
 void AssignmentTrackingLowering::emitDbgValue(
     AssignmentTrackingLowering::LocKind Kind,
-    const DbgVariableIntrinsic *Source, Instruction *After) {
+    AssignmentTrackingLowering::AssignRecord Source, VarLocInsertPt After) {
+  if (isa<DbgAssignIntrinsic *>(Source))
+    emitDbgValue(Kind, cast<DbgAssignIntrinsic *>(Source), After);
+  else
+    emitDbgValue(Kind, cast<DPValue *>(Source), After);
+}
+template <typename T>
+void AssignmentTrackingLowering::emitDbgValue(
+    AssignmentTrackingLowering::LocKind Kind, const T Source,
+    VarLocInsertPt After) {
 
   DILocation *DL = Source->getDebugLoc();
   auto Emit = [this, Source, After, DL](Metadata *Val, DIExpression *Expr) {
@@ -1493,15 +1533,15 @@ void AssignmentTrackingLowering::emitDbgValue(
 
   // NOTE: This block can mutate Kind.
   if (Kind == LocKind::Mem) {
-    const auto *DAI = cast<DbgAssignIntrinsic>(Source);
+    const auto *Assign = CastToDbgAssign(Source);
     // Check the address hasn't been dropped (e.g. the debug uses may not have
     // been replaced before deleting a Value).
-    if (DAI->isKillAddress()) {
+    if (Assign->isKillAddress()) {
       // The address isn't valid so treat this as a non-memory def.
       Kind = LocKind::Val;
     } else {
-      Value *Val = DAI->getAddress();
-      DIExpression *Expr = DAI->getAddressExpression();
+      Value *Val = Assign->getAddress();
+      DIExpression *Expr = Assign->getAddressExpression();
       assert(!Expr->getFragmentInfo() &&
              "fragment info should be stored in value-expression only");
       // Copy the fragment info over from the value-expression to the new
@@ -1610,25 +1650,26 @@ void AssignmentTrackingLowering::processUntaggedInstruction(
 void AssignmentTrackingLowering::processTaggedInstruction(
     Instruction &I, AssignmentTrackingLowering::BlockInfo *LiveSet) {
   auto Linked = at::getAssignmentMarkers(&I);
+  auto LinkedDPAssigns = at::getDPAssignmentMarkers(&I);
   // No dbg.assign intrinsics linked.
   // FIXME: All vars that have a stack slot this store modifies that don't have
   // a dbg.assign linked to it should probably treat this like an untagged
   // store.
-  if (Linked.empty())
+  if (Linked.empty() && LinkedDPAssigns.empty())
     return;
 
   LLVM_DEBUG(dbgs() << "processTaggedInstruction on " << I << "\n");
-  for (DbgAssignIntrinsic *DAI : Linked) {
-    VariableID Var = getVariableID(DebugVariable(DAI));
+  auto ProcessLinkedAssign = [&](auto *Assign) {
+    VariableID Var = getVariableID(DebugVariable(Assign));
     // Something has gone wrong if VarsWithStackSlot doesn't contain a variable
     // that is linked to a store.
-    assert(VarsWithStackSlot->count(getAggregate(DAI)) &&
-           "expected DAI's variable to have stack slot");
+    assert(VarsWithStackSlot->count(getAggregate(Assign)) &&
+           "expected Assign's variable to have stack slot");
 
     Assignment AV = Assignment::makeFromMemDef(getIDFromInst(I));
     addMemDef(LiveSet, Var, AV);
 
-    LLVM_DEBUG(dbgs() << "   linked to " << *DAI << "\n");
+    LLVM_DEBUG(dbgs() << "   linked to " << *Assign << "\n");
     LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
                       << " -> ");
 
@@ -1643,8 +1684,8 @@ void AssignmentTrackingLowering::processTaggedInstruction(
                  LiveSet->DebugValue[static_cast<unsigned>(Var)].dump(dbgs());
                  dbgs() << "\n");
       setLocKind(LiveSet, Var, LocKind::Mem);
-      emitDbgValue(LocKind::Mem, DAI, &I);
-      continue;
+      emitDbgValue(LocKind::Mem, Assign, &I);
+      return;
     }
 
     // The StackHomeValue and DebugValue for this variable do not match. I.e.
@@ -1669,7 +1710,7 @@ void AssignmentTrackingLowering::processTaggedInstruction(
         // We need to terminate any previously open location now.
         LLVM_DEBUG(dbgs() << "None, No Debug value available\n";);
         setLocKind(LiveSet, Var, LocKind::None);
-        emitDbgValue(LocKind::None, DAI, &I);
+        emitDbgValue(LocKind::None, Assign, &I);
       } else {
         // The previous DebugValue Value can be used here.
         LLVM_DEBUG(dbgs() << "Val, Debug value is Known\n";);
@@ -1678,7 +1719,7 @@ void AssignmentTrackingLowering::processTaggedInstruction(
           emitDbgValue(LocKind::Val, DbgAV.Source, &I);
         } else {
           // PrevAV.Source is nullptr so we must emit undef here.
-          emitDbgValue(LocKind::None, DAI, &I);
+          emitDbgValue(LocKind::None, Assign, &I);
         }
       }
     } break;
@@ -1689,74 +1730,89 @@ void AssignmentTrackingLowering::processTaggedInstruction(
       setLocKind(LiveSet, Var, LocKind::None);
     } break;
     }
-  }
+  };
+  for (DbgAssignIntrinsic *DAI : Linked)
+    ProcessLinkedAssign(DAI);
+  for (DPValue *DPV : LinkedDPAssigns)
+    ProcessLinkedAssign(DPV);
 }
 
-void AssignmentTrackingLowering::processDbgAssign(DbgAssignIntrinsic &DAI,
+void AssignmentTrackingLowering::processDbgAssign(AssignRecord Assign,
                                                   BlockInfo *LiveSet) {
-  // Only bother tracking variables that are at some point stack homed. Other
-  // variables can be dealt with trivially later.
-  if (!VarsWithStackSlot->count(getAggregate(&DAI)))
-    return;
+  auto ProcessDbgAssignImpl = [&](auto *DbgAssign) {
+    // Only bother tracking variables that are at some point stack homed. Other
+    // variables can be dealt with trivially later.
+    if (!VarsWithStackSlot->count(getAggregate(DbgAssign)))
+      return;
 
-  VariableID Var = getVariableID(DebugVariable(&DAI));
-  Assignment AV = Assignment::make(getIDFromMarker(DAI), &DAI);
-  addDbgDef(LiveSet, Var, AV);
-
-  LLVM_DEBUG(dbgs() << "processDbgAssign on " << DAI << "\n";);
-  LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
-                    << " -> ");
-
-  // Check if the DebugValue and StackHomeValue both hold the same
-  // Assignment.
-  if (hasVarWithAssignment(LiveSet, BlockInfo::Stack, Var, AV)) {
-    // They match. We can use the stack home because the debug intrinsics state
-    // that an assignment happened here, and we know that specific assignment
-    // was the last one to take place in memory for this variable.
-    LocKind Kind;
-    if (DAI.isKillAddress()) {
-      LLVM_DEBUG(
-          dbgs()
-              << "Val, Stack matches Debug program but address is killed\n";);
-      Kind = LocKind::Val;
+    VariableID Var = getVariableID(DebugVariable(DbgAssign));
+    Assignment AV = Assignment::make(getIDFromMarker(*DbgAssign), DbgAssign);
+    addDbgDef(LiveSet, Var, AV);
+
+    LLVM_DEBUG(dbgs() << "processDbgAssign on " << *DbgAssign << "\n";);
+    LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
+                      << " -> ");
+
+    // Check if the DebugValue and StackHomeValue both hold the same
+    // Assignment.
+    if (hasVarWithAssignment(LiveSet, BlockInfo::Stack, Var, AV)) {
+      // They match. We can use the stack home because the debug intrinsics
+      // state that an assignment happened here, and we know that specific
+      // assignment was the last one to take place in memory for this variable.
+      LocKind Kind;
+      if (DbgAssign->isKillAddress()) {
+        LLVM_DEBUG(
+            dbgs()
+                << "Val, Stack matches Debug program but address is killed\n";);
+        Kind = LocKind::Val;
+      } else {
+        LLVM_DEBUG(dbgs() << "Mem, Stack matches Debug program\n";);
+        Kind = LocKind::Mem;
+      };
+      setLocKind(LiveSet, Var, Kind);
+      emitDbgValue(Kind, DbgAssign, DbgAssign);
     } else {
-      LLVM_DEBUG(dbgs() << "Mem, Stack matches Debug program\n";);
-      Kind = LocKind::Mem;
-    };
-    setLocKind(LiveSet, Var, Kind);
-    emitDbgValue(Kind, &DAI, &DAI);
-  } else {
-    // The last assignment to the memory location isn't the one that we want to
-    // show to the user so emit a dbg.value(Value). Value may be undef.
-    LLVM_DEBUG(dbgs() << "Val, Stack contents is unknown\n";);
-    setLocKind(LiveSet, Var, LocKind::Val);
-    emitDbgValue(LocKind::Val, &DAI, &DAI);
-  }
+      // The last assignment to the memory location isn't the one that we want
+      // to show to the user so emit a dbg.value(Value). Value may be undef.
+      LLVM_DEBUG(dbgs() << "Val, Stack contents is unknown\n";);
+      setLocKind(LiveSet, Var, LocKind::Val);
+      emitDbgValue(LocKind::Val, DbgAssign, DbgAssign);
+    }
+  };
+  if (isa<DPValue *>(Assign))
+    return ProcessDbgAssignImpl(cast<DPValue *>(Assign));
+  return ProcessDbgAssignImpl(cast<DbgAssignIntrinsic *>(Assign));
 }
 
-void AssignmentTrackingLowering::processDbgValue(DbgValueInst &DVI,
-                                                 BlockInfo *LiveSet) {
-  // Only other tracking variables that are at some point stack homed.
-  // Other variables can be dealt with trivally later.
-  if (!VarsWithStackSlot->count(getAggregate(&DVI)))
-    return;
+void AssignmentTrackingLowering::processDbgValue(
+    PointerUnion<DbgValueInst *, DPValue *> DbgValueRecord,
+    BlockInfo *LiveSet) {
+  auto ProcessDbgValueImpl = [&](auto *DbgValue) {
+    // Only other tracking variables that are at some point stack homed.
+    // Other variables can be dealt with trivally later.
+    if (!VarsWithStackSlot->count(getAggregate(DbgValue)))
+      return;
 
-  VariableID Var = getVariableID(DebugVariable(&DVI));
-  // We have no ID to create an Assignment with so we mark this assignment as
-  // NoneOrPhi. Note that the dbg.value still exists, we just cannot determine
-  // the assignment responsible for setting this value.
-  // This is fine; dbg.values are essentially interchangable with unlinked
-  // dbg.assigns, and some passes such as mem2reg and instcombine add them to
-  // PHIs for promoted variables.
-  Assignment AV = Assignment::makeNoneOrPhi();
-  addDbgDef(LiveSet, Var, AV);
-
-  LLVM_DEBUG(dbgs() << "processDbgValue on " << DVI << "\n";);
-  LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
-                    << " -> Val, dbg.value override");
-
-  setLocKind(LiveSet, Var, LocKind::Val);
-  emitDbgValue(LocKind::Val, &DVI, &DVI);
+    VariableID Var = getVariableID(DebugVariable(DbgValue));
+    // We have no ID to create an Assignment with so we mark this assignment as
+    // NoneOrPhi. Note that the dbg.value still exists, we just cannot determine
+    // the assignment responsible for setting this value.
+    // This is fine; dbg.values are essentially interchangable with unlinked
+    // dbg.assigns, and some passes such as mem2reg and instcombine add them to
+    // PHIs for promoted variables.
+    Assignment AV = Assignment::makeNoneOrPhi();
+    addDbgDef(LiveSet, Var, AV);
+
+    LLVM_DEBUG(dbgs() << "processDbgValue on " << *DbgValue << "\n";);
+    LLVM_DEBUG(dbgs() << "   LiveLoc " << locStr(getLocKind(LiveSet, Var))
+                      << " -> Val, dbg.value override");
+
+    setLocKind(LiveSet, Var, LocKind::Val);
+    emitDbgValue(LocKind::Val, DbgValue, DbgValue);
+  };
+  if (isa<DPValue *>(DbgValueRecord))
+    return ProcessDbgValueImpl(cast<DPValue *>(DbgValueRecord));
+  return ProcessDbgValueImpl(cast<DbgValueInst *>(DbgValueRecord));
 }
 
 template <typename T> static bool hasZeroSizedFragment(T &DbgValue) {
@@ -1776,33 +1832,73 @@ void AssignmentTrackingLowering::processDbgInstruction(
     return;
 
   if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&I))
-    processDbgAssign(*DAI, LiveSet);
+    processDbgAssign(DAI, LiveSet);
   else if (auto *DVI = dyn_cast<DbgValueInst>(&I))
-    processDbgValue(*DVI, LiveSet);
+    processDbgValue(DVI, LiveSet);
+}
+void AssignmentTrackingLowering::processDPValue(
+    DPValue &DPV, AssignmentTrackingLowering::BlockInfo *LiveSet) {
+  // Ignore assignments to zero bits of the variable.
+  if (hasZeroSizedFragment(DPV))
+    return;
+
+  if (DPV.isDbgAssign())
+    processDbgAssign(&DPV, LiveSet);
+  else if (DPV.isDbgValue())
+    processDbgValue(&DPV, LiveSet);
 }
 
 void AssignmentTrackingLowering::resetInsertionPoint(Instruction &After) {
   assert(!After.isTerminator() && "Can't insert after a terminator");
-  auto R = InsertBeforeMap.find(After.getNextNode());
+  auto *R = InsertBeforeMap.find(getNextNode(&After));
+  if (R == InsertBeforeMap.end())
+    return;
+  R->second.clear();
+}
+void AssignmentTrackingLowering::resetInsertionPoint(DPValue &After) {
+  auto *R = InsertBeforeMap.find(getNextNode(&After));
   if (R == InsertBeforeMap.end())
     return;
   R->second.clear();
 }
 
 void AssignmentTrackingLowering::process(BasicBlock &BB, BlockInfo *LiveSet) {
+  // If the block starts with DPValues, we need to process those DPValues as
+  // their own frame without processing any instructions first.
+  bool ProcessedLeadingDPValues = !BB.begin()->hasDbgValues();
   for (auto II = BB.begin(), EI = BB.end(); II != EI;) {
     assert(VarsTouchedThisFrame.empty());
     // Process the instructions in "frames". A "frame" includes a single
     // non-debug instruction followed any debug instructions before the
     // next non-debug instruction.
-    if (!isa<DbgInfoIntrinsic>(&*II)) {
-      if (II->isTerminator())
-        break;
-      resetInsertionPoint(*II);
-      processNonDbgInstruction(*II, LiveSet);
-      assert(LiveSet->isValid());
-      ++II;
+
+    // Skip the current instruction if it has unprocessed DPValues attached (see
+    // comment above `ProcessedLeadingDPValues`).
+    if (ProcessedLeadingDPValues) {
+      // II is now either a debug intrinsic, a non-debug instruction with no
+      // attached DPValues, or a non-debug instruction with attached processed
+      // DPValues.
+      // II has not been processed.
+      if (!isa<DbgInfoIntrinsic>(&*II)) {
+        if (II->isTerminator())
+          break;
+        resetInsertionPoint(*II);
+        processNonDbgInstruction(*II, LiveSet);
+        assert(LiveSet->isValid());
+        ++II;
+      }
     }
+    // II is now either a debug intrinsic, a non-debug instruction with no
+    // attached DPValues, or a non-debug instruction with attached unprocessed
+    // DPValues.
+    if (II != EI && II->hasDbgValues()) {
+      for (DPValue &DPV : II->getDbgValueRange()) {
+        resetInsertionPoint(DPV);
+        processDPValue(DPV, LiveSet);
+        assert(LiveSet->isValid());
+      }
+    }
+    ProcessedLeadingDPValues = true;
     while (II != EI) {
       auto *Dbg = dyn_cast<DbgInfoIntrinsic>(&*II);
       if (!Dbg)
@@ -1812,6 +1908,10 @@ void AssignmentTrackingLowering::process(BasicBlock &BB, BlockInfo *LiveSet) {
       assert(LiveSet->isValid());
       ++II;
     }
+    // II is now a non-debug instruction either with no attached DPValues, or
+    // with attached processed DPValues. II has not been processed, and all
+    // debug instructions or DPValues in the frame preceding II have been
+    // processed.
 
     // We've processed everything in the "frame". Now determine which variables
     // cannot be represented by a dbg.declare.
@@ -1868,16 +1968,22 @@ AssignmentTrackingLowering::joinAssignment(const Assignment &A,
   // Here the same assignment ...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/78980


More information about the llvm-commits mailing list