[PATCH] D128177: [DebugInfo][InstrRef][NFC] Represent DbgValues with multiple ops in LDV

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

LGTM with some minor comments. I'd feel more confident if someone else (/ @jmorse ) took a quick look too, but given this is NFC and conceptually straightforward, I'm happy to approve it.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:174
+
+namespace llvm {
+    using namespace LiveDebugValues;
----------------
has this been `clang-format`ed (IIRC we don't usually indent for the namespace scope)?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:334
+    } ID;
+    uint32_t RawID;
+  };
----------------
It looks like you use this union for type punning. IIRC this is UB in C++ but I am unsure. Is this a case that is permitted for some reason, do you know?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:425
+// around.
+#define MAX_DBG_OPS 8
+
----------------
IIRC there is at least one other place in the compiler where there's a limit for number of debug operands (SCEV or salvaging or both?). Have you considered using a single value for all the limits? Or, if not, that then maybe updating the other limits if they're greater than 8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128177