[PATCH] D133291: [Assignment Tracking][8/*] Add DIAssignID merging utilities

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 7 03:54:38 PDT 2022


jmorse added a comment.

LGTM



================
Comment at: llvm/include/llvm/IR/Instruction.h:529
+  /// instruction in \p SourceInstructions needs to have DIAssignID
+  /// metadata. If none of them do then nothing happens.
+  void mergeDIAssignID(ArrayRef<const Instruction *> SourceInstructions);
----------------
Worth adding the rider that if this instruction has no DIAssignID, then it is given one, no? (I think that's what the 3rd unit test is doing?)


================
Comment at: llvm/lib/IR/DebugInfo.cpp:843
+  Function *Fn = getFunction();
+  assert(Fn && "Rethink this approach");
+  // Collect up the DIAssignID tags.
----------------
"Uninserted instruction merged"?


================
Comment at: llvm/lib/IR/DebugInfo.cpp:861
+  DIAssignID *MergeID = IDs[0];
+  for (auto It = IDs.begin() + 1, End = IDs.end(); It != End; It++)
+    at::RAUW(*It, MergeID);
----------------
(Style guide says ++It)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133291/new/

https://reviews.llvm.org/D133291



More information about the llvm-commits mailing list