[PATCH] D139691: [PowerPC] add a peephole to remove redundant swap instructions after vector splats on P8

Thu Dec 15 20:59:59 PST 2022

shchenz accepted this revision as: shchenz.
shchenz added a comment.
This revision is now accepted and ready to land.

LGTM. Maybe wait for some days for other reviews.

================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:647
           LLVM_DEBUG(MI.dump());
+        } else if (Immed == 2 &&
+                   (DefOpc == PPC::VSPLTB || DefOpc == PPC::VSPLTH ||
----------------
tingwang wrote:
> shchenz wrote:
> > tingwang wrote:
> > > shchenz wrote:
> > > > Can this branch be merged to the above one at line 627? Splat a double word and the double word is a splat of a small unit should also can be removed?
> > > Sure, I can do the merge to reuse logic. 
> > > 
> > > Are you referring to instructions like xxspltib? I think those are introduced in v3.0, and they will not appear on P8, so I did not include those.
> > Oh, I meant the `XXPERMDI` instruction can be removed like the above branch(at like 627) does. Not referring to any other instructions.
> IMHO, we saw these additional swaps on P8 because of stores, and that's why only saw Immed == 2. I'm not sure of any origin that can produce splat (i.e. Immed == 0 || Immed == 3 like 627) and reach peephole. If there is any such origin, it maybe more interesting to fix in the origin. To me, it is a little bit  confused why need to handle splat here?
OK, I see. I did some hacks and seems indeed XXPERMDI (as a splat) is never be fed by another splat. So let's keep as it is. And continuously improve this when a case is found.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139691