[PATCH] D29014: [SelDag] Implement FREEZE node

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:385
+  SDValue V = GetPromotedInteger(N->getOperand(0));
+  return DAG.getNode(N->getOpcode(), SDLoc(N),
+                     V.getValueType(), V);
----------------
Just use ISD::FREEZE instead of N->getOpcode()


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4021
 
+SDValue DAGTypeLegalizer::PromoteIntOp_FREEZE(SDNode *N) {
+  SDValue Op = GetPromotedInteger(N->getOperand(0));
----------------
I think this should be with the other PromoteIntOp functions? I think that's how this file is sorted.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4021
 
+SDValue DAGTypeLegalizer::PromoteIntOp_FREEZE(SDNode *N) {
+  SDValue Op = GetPromotedInteger(N->getOperand(0));
----------------
craig.topper wrote:
> I think this should be with the other PromoteIntOp functions? I think that's how this file is sorted.
Does this function even get called? The input type and the output type are the same right? So PromoteIntRes should have been called first.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4025
+}
+SDValue DAGTypeLegalizer::ExpandIntOp_FREEZE(SDNode *N) {
+  SDValue InL, InH;
----------------
Same with this. We should expand the result and then we'll never have to expand the input separately.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3105
   SDValue Op = getValue(I.getOperand(0));
+  if (I.getOperand(0)->getType()->isAggregateType()) {
+    /// Create a MERGE_VALUES node from the given operands.
----------------
Would we better off just handling freeze on its own instead of adding all this aggregate code to the generic visitUnary?


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

https://reviews.llvm.org/D29014