[PATCH] D146632: [PowerPC] Fix the xxperm swap requirements

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

I realize that a lot of test cases have changed but it would be good to add the test case that is specific to this opportunity where the copy is removed.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10232
 
-    // if V2 is dead, then we swap V1 and V2 so we can
-    // use V2 as the destination instead.
-    if (!V1->hasOneUse() && V2->hasOneUse()) {
+    // // We want the single-use operand to be in V2 to prevent copying.
+    if (!V2->hasOneUse() && V1->hasOneUse()) {
----------------
nit:
Too many `//` .

Also, this comment needs to do a better job explaining why we want to avoid having the input with multiple uses as the second input to XXPERM.  ie. That we have one input that is also an output for XXPERM and so that input must be clobbered.


================
Comment at: llvm/test/CodeGen/PowerPC/pre-inc-disable.ll:140
 ; P9LE-NEXT:    add 5, 3, 4
-; P9LE-NEXT:    lfiwzx 0, 3, 4
+; P9LE-NEXT:    lxsiwzx 2, 3, 4
 ; P9LE-NEXT:    addis 3, 2, .LCPI1_0 at toc@ha
----------------
Here we are replacing `lfiwzx` with `lxsiwzx`. 
This could technically be a (very minor...) performance hit as we are now using an operation that is 5 cycles instead of 4. 
I guess the upside is that we have a wider range of result registers.


================
Comment at: llvm/test/CodeGen/PowerPC/pre-inc-disable.ll:182
 ; P9BE-AIX-NEXT:    add 5, 3, 4
-; P9BE-AIX-NEXT:    lfiwzx 0, 3, 4
+; P9BE-AIX-NEXT:    lxsiwzx 2, 3, 4
 ; P9BE-AIX-NEXT:    ld 3, L..C2(2) # %const.0
----------------
Here it is the same idea as above. We are replacing a 4 cycle with a 5 cycle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146632