[PATCH] D110173: [DebugInfo][InstrRef] Use correct (and existing) PHI placement utilities for machine locations

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

First pass looking through this, haven't gone through the unit test yet - I think I understand this roughly, and don't have any complaints, but will continue to review - 1 nit and 1 question enclosed.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:1756-1758
+    // If we've already eliminated a PHI here, do no further checking, just
+    // propagate the first live-in value into this block.
+    if (InLocs[Idx.asU64()] != ValueIDNum(MBB.getNumber(), 0, Idx)) {
----------------
My understanding of this is incomplete, under what circumstances would we have eliminated the PHI without having already assigned `InLocs[Idx.asU64()] = FirstVal`?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:760
   /// from \p InLocs, and assigning the newly computed live ins back into
   /// \p InLocs. \returns two bools -- the first indicates whether a change
   /// was made, the second whether a lattice downgrade occurred. If the latter
----------------
Return value documentation needs updating


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110173