[PATCH] D125953: [DebugInfo][InstrRef] Handle joins PHI+Def values in LiveDebugValues

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

IIRC this is sound, from our discussion elsewhere the problem is that we do not eliminate a PHI if the incoming values agree, but one value comes from a PHI and the other does not. IMO: this should also get a unit test entry to ease the identification of errors in the future. Additionally, the test file should be MIR and only run livedebugvalues, to reduce the surface of things being tested.

(I think I still need to convince myself that there's not some subtle ordering on PHI elimination that we affect by doing this -- that'll take a little time, but I'm pretty confident already).



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:2504-2505
 
+    if (V.second->ID != ValueIDNum::EmptyValue && V.second->ID == FirstVal.ID)
+      continue; // No disagreement.
+
----------------
IMO this needs a more precise (but still short comment), indicating _what_ about the agreement is acceptable here. i.e., "The difference is only that one value comes from a DBG_PHI, the other from a defining instruction or similar", or something in that vein.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125953