[PATCH] D54409: PowerPC/SPE: Fix load/store handling for SPE

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: wuzish, MaskRay.

LGTM other than a few minor nits.



================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:2239
+    if (MemSDNode *Memop = dyn_cast<MemSDNode>(*UI)) {
+      if (Memop->getMemoryVT() == MVT::f64) {
+          Base = N.getOperand(0);
----------------
Am I reading this correctly? If this node is used for any memory operation that is accessing an `f64`, we will never use r+i addressing. Is that the desired behaviour?

So if I'm not mistaken, we have the following semantics:
- If we are accessing an `MVT::f64`, always use r+r
- If we are accessing any other type, use r+i if the offset is a 16-bit signed value
  - Otherwise use r+r addressing


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:2263
+      // SPE load/store can only handle 8-bit offsets.
+      if (SelectAddressEVXRegReg(N, Base, Index, DAG))
+        return true;
----------------
We can probably combine this condition with the one above into a single `if`, but that can be done on the commit - no need for another review.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54409