[PATCH] D57997: [SDAG] Support vector UMULO/SMULO

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1228
+SDValue VectorLegalizer::ExpandMULO(SDValue Op) {
+  auto Pair = TLI.expandMULO(Op.getNode(), DAG);
+  AddLegalizedOperand(Op.getValue(0), Pair.first);
----------------
Don't use auto


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1230
+  AddLegalizedOperand(Op.getValue(0), Pair.first);
+  AddLegalizedOperand(Op.getValue(1), Pair.second);
+  return Op.getResNo() ? Pair.second : Pair.first;
----------------
Would it be better to perform the unroll in this function instead of TLI.expandMULO? This matches existing behaviour 


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:8960
+    ResScalars.push_back(getUNDEF(ResEltVT));
+    OvScalars.push_back(getUNDEF(OvEltVT));
+  }
----------------
ResScalars.append(ResNE - NE, getUNDEF(ResEltVT));
OvScalars.append(ResNE - NE, getUNDEF(OvEltVT));


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:5526
 std::pair<SDValue, SDValue> TargetLowering::expandMULO(
     SDNode *Node, SelectionDAG &DAG) const {
   SDLoc dl(Node);
----------------
I still reckon you'd be better off returning a bool and the result/overflow SDValues by reference args - similar to what we do for the majority of the TargetLowering::expand* functions


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

https://reviews.llvm.org/D57997