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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

I think this looks reasonable, but am not familiar enough with the code to approve.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5480
       if (RFV.occupiesMultipleRegs()) {
-        unsigned Offset = 0;
-        for (auto RegAndSize : RFV.getRegsAndSizes()) {
-          Op = MachineOperand::CreateReg(RegAndSize.first, false);
-          auto FragmentExpr = DIExpression::createFragmentExpression(
-              Expr, Offset, RegAndSize.second);
-          if (!FragmentExpr)
-            continue;
-          FuncInfo.ArgDbgValues.push_back(
-              BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsDbgDeclare,
-                      Op->getReg(), Variable, *FragmentExpr));
-          Offset += RegAndSize.second;
-        }
+        splitMultiRegDbgValue(RFV.getRegsAndSizes());
         return true;
----------------
arsenm wrote:
> vsk wrote:
> > I'm struggling to understand why values in the ValueMap need separate handling. IOW I don't get why it's not possible to handle the 'ArgRegsAndSizes.size() > 1' case earlier and assert '!RFV.occupiesMultipleRegs()' here.
> RegsForValue expects a virtual register to exist already, and defined by some sequence somewhere. There's no corresponding live in register to the super-register in this case
Thanks for explaining. I wonder if this could be simplified by sinking the ValueMap lookup into getUnderlyingArgRegs?


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

https://reviews.llvm.org/D64645