[PATCH] D125680: Correctly legalise stackmap operands

[Edd Barrett via Phabricator via llvm-commits]

vext01 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2314
+  SmallVector<SDValue> NewOps(N->ops().begin(), N->ops().end());
+  NewOps[OpNo] = ZExtPromotedInteger(N->getOperand(OpNo));
+  return SDValue(DAG.UpdateNodeOperands(N, NewOps), 0);
----------------
arsenm wrote:
> I'm a bit surprised this can unconditionally zero extend. I'd expect ANY_EXTEND or to have to consider some ABI property
Perhaps not some ABI property, as a call to `llvm.experimental.stackmap` isn't really a call, so there is no ABI to speak of.

I'll look into `ANY_EXTEND`. I'm not familiar.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2274-2277
+      Ops.push_back(
+          CurDAG->getTargetConstant(StackMaps::ConstantOp, DL, MVT::i64));
+      O = CurDAG->getTargetConstant(
+          cast<ConstantSDNode>(OpNode)->getZExtValue(), DL, It->getValueType());
----------------
arsenm wrote:
> You're still fixing this into TargetConstant at selection time instead of upfront when lowering from the IR
This code is selecting the stackmap live variables.

The (non-frameindex) live variables can't be emitted to target constants at DAG-build time or they won't get legalized and that's the problem that this change is trying to address.

Correct me if I'm wrong though.


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

https://reviews.llvm.org/D125680