[PATCH] D25912: [PowerPC] Improvements for BUILD_VECTOR Vol. 1

[Nemanja Ivanovic via llvm-commits]

nemanjai added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:674-676
+      if (Subtarget.hasDirectMove())
+        setOperationAction(ISD::BUILD_VECTOR, MVT::v2i64, Custom);
+      setOperationAction(ISD::BUILD_VECTOR, MVT::v2f64, Custom);
----------------
amehsan wrote:
> I have added a similar logic in one of the patches that I committed recently. Please make sure that we do not do this twice when you commit your code. (mine is a little bit earlier in the file IIRC)
OK, thanks for mentioning that. I'll double check.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:7214-7217
+    auto haveEfficientPattern = [&](BuildVectorSDNode *V) -> bool {
+      bool IsSplat = true;
+      bool IsLoad = false;
+      SDValue Op0 = V->getOperand(0);
----------------
amehsan wrote:
> Do we need a lambda that is called only once? Since this function is very long already, I think creating a separate function for this is reasonable to avoid making this function too long. 
I only added a lambda for two reasons:
1. The logic is very close to where it's used
2. The logic isn't needed anywhere else

But I really don't feel strongly about that reasoning and would be happy to change it to a function if that's what is preferred. What do the others think?


Repository:
  rL LLVM

https://reviews.llvm.org/D25912