[PATCH] D25980: [PowerPC] Improvements for BUILD_VECTOR Vol. 2

[Kit Barton via llvm-commits]

kbarton added a comment.

Aside from some nits about comments and formatting this LGTM.



================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10521
 
+/// combineBVOfFpToIntToFpToIntOfBV - If this node is a build_vector of
+/// fp-to-int conversions, reduce it to a fp-to-int of a build_vector
----------------
Please convert this to a doxygen-style comment.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10527
+SDValue
+PPCTargetLowering::combineBVOfFpToIntToFpToIntOfBV(SDNode *N,
+                                                   DAGCombinerInfo &DCI) const {
----------------
I know this name is descriptive of what the function does, but it's very difficult to read. 
Is there another name possible? Maybe something containing transpose, exchange, swap or refactor?


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10552
+    EVT TargetVT = N->getValueType(0);
+    for (int i = 0, e = N->getNumOperands(); i < e; i++) {
+      if (N->getOperand(i).getOpcode() != PPCISD::MFVSR)
----------------
Use ++i not i++


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10569
+    // Now that we know we have the right type of node, get its operands
+    for (int i = 0, e = N->getNumOperands(); i < e; i++) {
+      SDValue In = N->getOperand(i).getOperand(0);
----------------
++i


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10586
+    unsigned Opcode;
+    if(FirstConversion == PPCISD::FCTIDZ ||
+       FirstConversion == PPCISD::FCTIWZ)
----------------
space between if (


Repository:
  rL LLVM

https://reviews.llvm.org/D25980