[PATCH] D129268: Legalise patchpoint arguments.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:5526-5527
+
+  if (Op->getOpcode() == ISD::Constant) {
+    ConstantSDNode *CN = cast<ConstantSDNode>(Op);
+    EVT Ty = Op.getValueType();
----------------
vext01 wrote:
> arsenm wrote:
> > dyn_cast to ConstantSDNode
> Forgive me. I'm still fairly new to C++, am I doing this right?
> 
> ```diff
> diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> index 4ee4ece7d8ef..89c0738f24b0 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> @@ -5547,8 +5547,7 @@ SDValue DAGTypeLegalizer::ExpandIntOp_STACKMAP(SDNode *N, unsigned OpNo) {
>    for (unsigned I = 0; I < OpNo; I++)
>      NewOps.push_back(N->getOperand(I));
>  
> -  if (Op->getOpcode() == ISD::Constant) {
> -    ConstantSDNode *CN = cast<ConstantSDNode>(Op);
> +  if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Op)) {
>      EVT Ty = Op.getValueType();
>      if (CN->getConstantIntValue()->getValue().getActiveBits() < 64) {
>        NewOps.push_back(
> @@ -5587,8 +5586,7 @@ SDValue DAGTypeLegalizer::ExpandIntOp_PATCHPOINT(SDNode *N, unsigned OpNo) {
>    for (unsigned I = 0; I < OpNo; I++)
>      NewOps.push_back(N->getOperand(I));
>  
> -  if (Op->getOpcode() == ISD::Constant) {
> -    ConstantSDNode *CN = cast<ConstantSDNode>(Op);
> +  if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Op)) {
>      EVT Ty = Op.getValueType();
>      if (CN->getConstantIntValue()->getValue().getActiveBits() < 64) {
>        NewOps.push_back(
> diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> index 25105ee2dbde..0dae6a06ef72 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> @@ -2201,12 +2201,11 @@ void SelectionDAGISel::pushStackMapLiveVariable(SmallVectorImpl<SDValue> &Ops,
>    // nodes at DAG-construction time.
>    assert(OpNode->getOpcode() != ISD::FrameIndex);
>  
> -  if (OpNode->getOpcode() == ISD::Constant) {
> +  if (ConstantSDNode *CN = dyn_cast<ConstantSDNode>(OpNode)) {
>      Ops.push_back(
>          CurDAG->getTargetConstant(StackMaps::ConstantOp, DL, MVT::i64));
>      Ops.push_back(
> -        CurDAG->getTargetConstant(cast<ConstantSDNode>(OpNode)->getZExtValue(),
> -                                  DL, OpVal.getValueType()));
> +        CurDAG->getTargetConstant(CN->getZExtValue(), DL, OpVal.getValueType()));
>    } else {
>      Ops.push_back(OpVal);
>    }
> ```
> 
> This makes tests fail, so can't be equivalent.
The slight difference is ConstantSDNode accepts Constant and TargetConstant but I would expect consistency at this point


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

https://reviews.llvm.org/D129268