[PATCH] D123599: [DebugInfo][InstrRef] Describe value sizes when spilt to stack

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

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

LGTM. It's great to have the comprehensive test spelling out the expectations for all the combinations of fragment  / deref / indirect.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:879-880
+      unsigned DerefSize = ValueSize / 8;
+      if (Var.getFragment()) {
+        auto Fragment = Var.getFragment();
+        unsigned VariableSize = Fragment->SizeInBits / 8;
----------------
nit: `if (auto Fragment = Var.getFragment())` more idiomatic?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:888
+        }
+      }
+
----------------
nit: I think adding `InBits` / `InBytes` to these variable names would improve readability here


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir:269
+
+    ;; Fragment (32), Larger value (8), no stack value,
+    $rax = MOV64ri 0, debug-instr-number 13, debug-location !7
----------------



================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir:337
+
+    ;; Fragment (32), Larger value (8), no stack value,
+    $rax = MOV64ri 0, debug-instr-number 19, debug-location !7
----------------



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

https://reviews.llvm.org/D123599