[PATCH] D128177: [DebugInfo][InstrRef][NFC] Represent DbgValues with multiple ops in LDV
Orlando Cazalet-Hyams via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 7 04:49:05 PDT 2022
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
More information about the llvm-commits
mailing list