[PATCH] D54583: PowerPC: Optimize SPE double parameter calling setup

[Justin Hibbits via Phabricator via llvm-commits]

jhibbits marked 6 inline comments as done.
jhibbits added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:7840
+
+  assert(Op0.getNumOperands() > 0 && "WTF?");
+  if (Op->getConstantOperandVal(1) == 0)
----------------
nemanjai wrote:
> A more descriptive assert message is probably in order.
> Also, perhaps it would be clearer if this was rewritten to:
> - Assert that the constant operand value is less than 2
> - Use a ternary operator to select the opcode (or just have one opcode - see above)
This assert was during some debugging, and I forgot to remove it.  However, it does make sense to have an assert along the lines of your suggestion.  I"ll make such a change.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:7842
+  if (Op->getConstantOperandVal(1) == 0)
+    return DAG.getNode(PPCISD::EXTRACT_SPE_LO,  dl, MVT::i32, Op0.getOperand(0));
+
----------------
nemanjai wrote:
> Line too long?
Yeah, just a hair (one character).  Reformatting with clang-format will fix that.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.h:203
+      /// Extract high SPE register component
+      EXTRACT_SPE_HI,
+
----------------
nemanjai wrote:
> Why not just have `EXTRACT_SPE_OP` and have it take a constant operand that determines Hi/Lo? Also, for both build and extract, it would be good to add a comment that these correspond almost exactly to `BUILD_PAIR` and `EXTRACT_VECTOR_ELT` nodes except that the input types are floating point since `i64` isn't a legal type for the target.
I'm not sure how to pass a constant through to the tablegen'd layer.  These two pseudo-ops are just light wrappers to EVMERGEHI and MR.  If there is a way to pass a constant and do the switch down in that layer, then that's acceptable as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54583