[PATCH] D46129: [SelectionDAG] Improve selection of DBG_VALUE using a PHI node result

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.cpp:724
+  } else if (SD->getKind() == SDDbgValue::VREG) {
+    MIB.addReg(SD->getVReg(), RegState::Debug);
   } else if (SD->getKind() == SDDbgValue::CONST) {
----------------
aprantl wrote:
> Just to be sure: this does *not* allocate a new vreg, right?
No, we are just using the "unsigned" value for the register that was saved when creating the SDDbgValue.

Note that for this to work we must no change the vreg destination for the PHI node during SelectionDAG build/opt.
Since the PHI nodes aren't part of the SelectionDAG any "normal" use of the vreg would result in a CopyFromReg node that maps the PHI result to a SDValue. But since debug-info aren't supposed to affect code generation we can't create SDNodes for the debug-use. But I also assume that this means that the vreg defined by the already lowered PHI node will stay intact during the SelectionDAG build/opt.

An alternative approach that I considered was to save the Value*, for the PHI node in LLVM IR, similar to how we save a Value* when creating the SDDbgValue::CONST kind of nodes. And then we could use the FunctionLoweringInfo::ValueMap to do the translation into a vreg here in the InstrEmitter. However, then we need to expose the FunctionLoweringInfo to the InstrEmitter and pass that around while emitting things to be able to use it here. I'm not sure that it will make any difference really, and the current patch is smaller.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7313
 
+/// Constant
+SDDbgValue *SelectionDAG::getVRegDbgValue(DIVariable *Var,
----------------
aprantl wrote:
> dangling comment?
Thanks, I'll fix that.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7320
+         "Expected inlined-at fields to agree");
+  return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, VReg, IsIndirect, DL, O);
+}
----------------
Line longer than 80 columns, will be wrapped before landing this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D46129