[PATCH] D64645: DAG: Handle dbg_value for arguments split into multiple subregs

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

This looks good to me, and I'm sort of familiar enough with the debug bits to approve. I've one concern about the change to getUnderlyingArgRegs though: previously it would fail-safe and return 0/$noreg if it encountered a node it didn't recognise. However now, if there's anything unrecognised in the middle of a vector/pair, the element will be silently skipped. That could lead to splitMultiRegDbgValue producing DBG_VALUEs with the wrong fragment offset, as it's unaware of the skipped element.

(I'm not aware of any guarantees about what SDNodes Arguments are translated into).



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5315
+// getUnderlyingArgRegs - Find underlying registers used for a truncated,
+// bitcasted, or split argument.
+void getUnderlyingArgRegs(SmallVectorImpl<std::pair<unsigned, unsigned>> &Regs,
----------------
Document that the output pairs are <Reg,Size>?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5334
+  case ISD::CONCAT_VECTORS:
+    for (SDValue Op : N.op_values())
+      getUnderlyingArgRegs(Regs, Op);
----------------
NB, this didn't compile for me (N needs to be dereferenced to get the SDNode inside).


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5435
+
     if (Reg && TargetRegisterInfo::isVirtualRegister(Reg)) {
       MachineRegisterInfo &RegInfo = MF.getRegInfo();
----------------
Reg.isVirtual()? As you're moving this code to use Register :)


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

https://reviews.llvm.org/D64645