[PATCH] D125680: Correctly legalise stackmap operands

[Denis Antrushin via Phabricator via llvm-commits]

dantrushin added a comment.

In general looks OK to me (modulo few style nits).
But I'm not an ISEL type legalization expert. Would be nice to have LGTM from someone else as well.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:2920
+    Res = SoftPromoteHalfOp_STACKMAP(N, OpNo);
+    break;
   }
----------------
Nit: could you have it in single line as cases above (if it fits in 80 chars)?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1729
+    Res = PromoteIntOp_STACKMAP(N, OpNo);
+    break;
   }
----------------
Nit: same comment for a single line


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4669
+    Res = ExpandIntOp_STACKMAP(N, OpNo);
+    break;
   }
----------------
Nit: and here too


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4684
   ReplaceValueWith(SDValue(N, 0), Res);
+
   return false;
----------------
Nit: extra empty line


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:9423
+  // Add the STACKMAP operands, starting with DAG house-keeping.
+  std::vector<SDValue> Operands;
+  Operands.push_back(Chain);
----------------
Use existing `Ops` instead of introducing new vector?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp:490
+  case ISD::STACKMAP:
+    return "stackmap";
 
----------------
Single line again


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

https://reviews.llvm.org/D125680