[PATCH] D110173: [DebugInfo][InstrRef] Use correct (and existing) PHI placement utilities for machine locations
Stephen Tozer via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 23 06:10:25 PDT 2021
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
More information about the llvm-commits
mailing list