[PATCH] D19825: Power9 - Add exploitation of vector load and store that do not require swaps

[Kit Barton via llvm-commits]

kbarton added a comment.

Regarding the testing, if I understand the patch correctly, we should also be able to do: -mcpu=pwr9 -mattr=-P9Vector, and get the same codegen as we would expect with -mcpu=pwr8 (i.e., generate the additional swap instructions). If that's true, can you please add these runsteps as well to ensure we get the right codegen?


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10633
@@ -10632,2 +10632,3 @@
     // For little endian, VSX stores require generating xxswapd/lxvd2x.
+    // Not needed on ISA 3.0 based CPUs since we have a non-permuting store.
     EVT VT = N->getOperand(1).getValueType();
----------------
Here the comments refer to ISA3_0. Does this imply P9Vector? What about hasVSX()?

================
Comment at: lib/Target/PowerPC/PPCInstrInfo.cpp:1008
@@ -1005,2 +1007,3 @@
   } else if (PPC::VSRCRegClass.hasSubClassEq(RC)) {
-    NewMIs.push_back(addFrameReference(BuildMI(MF, DL, get(PPC::STXVD2X))
+    unsigned Op = Subtarget.isISA3_0() ? PPC::STXVX : PPC::STXVD2X;
+    NewMIs.push_back(addFrameReference(BuildMI(MF, DL, get(Op))
----------------
Here we check for ISA3_0. Again, does this imply P9Vector()? Are the LXVX and STXVX instructions guarded by ISA3_0 or P9Vector?

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:95
@@ -94,2 +94,3 @@
 def IsBigEndian : Predicate<"!PPCSubTarget->isLittleEndian()">;
+def HasOnlySwappingMemOps : Predicate<"!PPCSubTarget->hasP9Vector()">;
 
----------------
Here we're checking for P9Vector only, no ISA3_0. 

================
Comment at: lib/Target/PowerPC/PPCSubtarget.h:279
@@ +278,3 @@
+  bool needsSwapsForVSXMemOps() const {
+    return hasVSX() && isLittleEndian() && !isISA3_0();
+  }
----------------
Here we're checking hasVSX() and ISA3_0, but no mention of P9Vector. 


Repository:
  rL LLVM

http://reviews.llvm.org/D19825