[PATCH] [PPC64LE] Remove unnecessary swaps from lane-insensitive vector computations

Bill Schmidt wschmidt at linux.vnet.ibm.com
Mon Mar 30 14:15:09 PDT 2015


I'll look at the backend option as a temporary measure, although someday I'd like to see a better solution.  (That said, I can't volunteer to design it myself right now, so I have no right to complain...)


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:110
@@ +109,3 @@
+  // their swap entries.  The key is the address of the MI.
+  std::map<MachineInstr*, int> SwapMap;
+
----------------
echristo wrote:
> DenseMap?
Yes, that looks appropriate.  I'll move to that.

================
Comment at: lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:204
@@ +203,3 @@
+
+  const int InitialVectorSize(256);
+  SwapVector.clear();
----------------
echristo wrote:
> wschmidt wrote:
> > echristo wrote:
> > > 256?
> > Hey, a magic constant.  That's why I gave it a name. :)  Picking an initial vector size to avoid too many reallocations while keeping the size reasonable.  Yes, it's a wet thumb to the breeze...
> Did you use science to figure it out (i.e. see what numbers would generally work etc) or just WAG?
Semi-science, yes.  I've spent some time on this in GCC as well and gotten a pretty good feel for what happens.  256 is sufficiently large to prevent any reallocations in the usual case.  For larger functions, we will see a reallocation or two.  My testing on LLVM indicated that the code in projects/test-suite used no reallocations except for three of the benchmarks (I know this because my implementation was broken, so I noticed failures when the reallocations occurred).  So I feel this is a good choice.

================
Comment at: lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:251
@@ +250,3 @@
+      switch(MI->getOpcode()) {
+      default:
+        // Unless noted otherwise, an instruction is considered
----------------
echristo wrote:
> wschmidt wrote:
> > echristo wrote:
> > > What instructions not listed are you thinking here?
> > A vast number of lane-insensitive instructions.  All of the vector math, logical, select, etc.  Everything that's true SIMD.
> Comment then? Just because there's another long list below so...
Yep, I can do that.

================
Comment at: lib/Target/PowerPC/PPCVSXSwapRemoval.cpp:614
@@ +613,3 @@
+
+        for (MachineInstr &UseMI : MRI->use_nodbg_instructions(DefReg)) {
+          int UseIdx = SwapMap[&UseMI];
----------------
echristo wrote:
> wschmidt wrote:
> > echristo wrote:
> > > Good point, what _does_ this do for debug info? :)
> > It doesn't do anything, but it doesn't need to.  When the load or store is initially expanded, its location information goes on the LXVD2X and XXPERMDI instructions, or on the XXPERMDI and STXVD2X instructions.  This pass removes the XXPERMDI instructions, but the location information remains with the LXVD2X or STXVD2X.
> That's what I thought, just making sure.
I can add a comment to this effect.

http://reviews.llvm.org/D8565

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list