[PATCH] D48277: [DebugInfo] Keep DBG_VALUE undef in LiveDebugVariables

[Adrian Prantl via Phabricator via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Looks good after adding an additional comment explaining why the undef values have to be treated differently. Thanks!



================
Comment at: lib/CodeGen/LiveDebugVariables.cpp:1095
     DbgValueLocation Loc = I.value();
+    // Undef values are not placed in the LocNoMap so skip over them.
+    if (Loc.isUndef())
----------------
Please add a comment explaining why they aren't placed in the LocNoMap so future readers understand this.


================
Comment at: lib/CodeGen/LiveDebugVariables.cpp:1097
+    if (Loc.isUndef())
+      continue;
     unsigned NewLocNo = LocNoMap[Loc.locNo()];
----------------
bjope wrote:
> aprantl wrote:
> > Just for my understanding: What is preventing you from adding the undef location into the map?
> The reason afaik: locNo() is ~0U for isUndef() (that is actually how undef is identified). So adding it to the LocNoMap, which actually is implemented as a vector and not a map, would make that vector very long.
> 
> Maybe that can be solved somehow :
> - chopping off another bit in DbgValueLocation to describe IsUndef similar to WasIndirect
> - or by reserving locNo 0 for isUndef instead of using ~0U)
> - or is it possible to use a map instead of a vector to implement the LocNoMap
Got it. The current patch doesn't look to bad either.


https://reviews.llvm.org/D48277