[PowerPC] [NFC] Cleanup xxpermdi peephole optimization

[vddvss]

Summary

Following on from rG884351547da2, this patch cleans up the logic for xxpermdi
peephole optimizations by converting two layers of nested ifs to early breaks
and simplifying the logic.

[steven.zhang]

Please attach the test if it is NOT NFC patch, otherwise, adding a tag [NFC] in the title, thank you.

[vddvss]

In D71170#1774445, @steven.zhang wrote:

Please attach the test if it is NOT NFC patch, otherwise, adding a tag [NFC] in the title, thank you.

Oops, sorry about that. Yes, it is an NFC patch covered by existing tests.

[steven.zhang]

LGTM, as early return makes the code more clear. Thank you for the clean up.

[vddvss]

In D71170#1774538, @steven.zhang wrote:

LGTM, as early return makes the code more clear. Thank you for the clean up.

Thanks! Could you land it for me please?

[jsji]

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

350

Looks like this might have a problem with Stack slots? We used to only handle virtual registers,
but now, we might also let throughg stack slots (and Not a register), which might cause build break?

//   0           Not a register, can be used as a sentinel.
//   [1;2^30)    Physical registers assigned by TableGen.
//   [2^30;2^31) Stack slots. (Rarely used.)
//   [2^31;2^32) Virtual registers assigned by MachineRegisterInfo.

[vddvss]

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

350

Oh, I didn't know about that. I had used isPhysicalRegister based on the pattern in D69497. Should I change this or both of them to !isVirtualRegister?

[jsji]

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

350

I think so, it is the simplest NFC change. Or else you should go through following code to double check, may sure it won't break anything.

[vddvss]

This addresses @jsji's note about the potential for stack slots being let through by using isPhysicalRegister. It changes this pattern and the one from D69497 to use !isVirtualRegister.

[jsji]

Thanks. I will commit it for you.

[steven.zhang]

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

350

Ah, good catch. ! isVirtualRegister != isPhysicalRegister ...

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

for (MachineInstr &MI : MBB) {
         break;
       }
       case PPC::XXPERMDI: {
         // Perform simplifications of 2x64 vector swaps and splats.
         // A swap is identified by an immediate value of 2, and a splat
         // is identified by an immediate value of 0 or 3.
         int Immed = MI.getOperand(3).getImm();
 
-        if (Immed != 1) {
+        if (Immed == 1)
+          break;
 
-          // For each of these simplifications, we need the two source
-          // regs to match.  Unfortunately, MachineCSE ignores COPY and
-          // SUBREG_TO_REG, so for example we can see
-          //   XXPERMDI t, SUBREG_TO_REG(s), SUBREG_TO_REG(s), immed.
-          // We have to look through chains of COPY and SUBREG_TO_REG
-          // to find the real source values for comparison.
-          unsigned TrueReg1 =
-            TRI->lookThruCopyLike(MI.getOperand(1).getReg(), MRI);
-          unsigned TrueReg2 =
-            TRI->lookThruCopyLike(MI.getOperand(2).getReg(), MRI);
 
-          if (TrueReg1 == TrueReg2 && Register::isVirtualRegister(TrueReg1)) {
-            MachineInstr *DefMI = MRI->getVRegDef(TrueReg1);
-            unsigned DefOpc = DefMI ? DefMI->getOpcode() : 0;
+        if (!(TrueReg1 == TrueReg2 && Register::isVirtualRegister(TrueReg1)))
+          break;
+
+
+        if (!DefMI)
+          break;
+
+        unsigned DefOpc = DefMI->getOpcode();
 
-            // If this is a splat fed by a splatting load, the splat is
-            // redundant. Replace with a copy. This doesn't happen directly due
-            // to code in PPCDAGToDAGISel.cpp, but it can happen when converting
-            // a load of a double to a vector of 64-bit integers.
-            auto isConversionOfLoadAndSplat = [=]() -> bool {
-              if (DefOpc != PPC::XVCVDPSXDS && DefOpc != PPC::XVCVDPUXDS)
-                return false;
-              unsigned DefReg =
-                TRI->lookThruCopyLike(DefMI->getOperand(1).getReg(), MRI);
-              if (Register::isVirtualRegister(DefReg)) {
-                MachineInstr *LoadMI = MRI->getVRegDef(DefReg);
-                if (LoadMI && LoadMI->getOpcode() == PPC::LXVDSX)
-                  return true;
-              }
-              return false;
-            };
-            if (DefMI && (Immed == 0 || Immed == 3)) {
-              if (DefOpc == PPC::LXVDSX || isConversionOfLoadAndSplat()) {
-                LLVM_DEBUG(dbgs() << "Optimizing load-and-splat/splat "
-                                     "to load-and-splat/copy: ");
-                LLVM_DEBUG(MI.dump());
-                BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(PPC::COPY),
-                        MI.getOperand(0).getReg())
-                    .add(MI.getOperand(1));
-                ToErase = &MI;
-                Simplified = true;
-              }
-            }
+          unsigned FeedReg1 =
+          if (Register::isVirtualRegister(FeedReg1)) {
+            MachineInstr *LoadMI = MRI->getVRegDef(FeedReg1);
+        if ((Immed == 0 || Immed == 3) &&
+            (DefOpc == PPC::LXVDSX || isConversionOfLoadAndSplat())) {
 
-            // If this is a splat or a swap fed by another splat, we
-            // can replace it with a copy.
-            if (DefOpc == PPC::XXPERMDI) {
-              unsigned DefReg1 = DefMI->getOperand(1).getReg();
-              unsigned DefReg2 = DefMI->getOperand(2).getReg();
-              unsigned DefImmed = DefMI->getOperand(3).getImm();
 
-              // If the two inputs are not the same register, check to see if
-              // they originate from the same virtual register after only
-              // copy-like instructions.
-              if (DefReg1 != DefReg2) {
-                unsigned FeedReg1 = TRI->lookThruCopyLike(DefReg1, MRI);
-                unsigned FeedReg2 = TRI->lookThruCopyLike(DefReg2, MRI);
 
-                if (FeedReg1 != FeedReg2 ||
-                    Register::isPhysicalRegister(FeedReg1))
-                  break;
-              }
+            if (!(FeedReg1 == FeedReg2 &&
+                  Register::isVirtualRegister(FeedReg1)))
 
-              if (DefImmed == 0 || DefImmed == 3) {
-                LLVM_DEBUG(dbgs() << "Optimizing splat/swap or splat/splat "
-                                     "to splat/copy: ");
-                LLVM_DEBUG(MI.dump());
-                BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(PPC::COPY),
-                        MI.getOperand(0).getReg())
-                    .add(MI.getOperand(1));
-                ToErase = &MI;
-                Simplified = true;
-              }
 
-              // If this is a splat fed by a swap, we can simplify modify
-              // the splat to splat the other value from the swap's input
-              // parameter.
-              else if ((Immed == 0 || Immed == 3) && DefImmed == 2) {
-                LLVM_DEBUG(dbgs() << "Optimizing swap/splat => splat: ");
-                LLVM_DEBUG(MI.dump());
-                MI.getOperand(1).setReg(DefReg1);
-                MI.getOperand(2).setReg(DefReg2);
-                MI.getOperand(3).setImm(3 - Immed);
-                Simplified = true;
-              }
 
-              // If this is a swap fed by a swap, we can replace it
-              // with a copy from the first swap's input.
-              else if (Immed == 2 && DefImmed == 2) {
-                LLVM_DEBUG(dbgs() << "Optimizing swap/swap => copy: ");
-                LLVM_DEBUG(MI.dump());
-                BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(PPC::COPY),
-                        MI.getOperand(0).getReg())
-                    .add(DefMI->getOperand(1));
-                ToErase = &MI;
-                Simplified = true;
-              }
-            } else if ((Immed == 0 || Immed == 3) && DefOpc == PPC::XXPERMDIs &&
-                       (DefMI->getOperand(2).getImm() == 0 ||
-                        DefMI->getOperand(2).getImm() == 3)) {
-              // Splat fed by another splat - switch the output of the first
-              // and remove the second.
-              DefMI->getOperand(0).setReg(MI.getOperand(0).getReg());
-              ToErase = &MI;
-              Simplified = true;
-              LLVM_DEBUG(dbgs() << "Removing redundant splat: ");
-              LLVM_DEBUG(MI.dump());
-            }
-          }
-        }
         break;
       }
       case PPC::VSPLTB:
       case PPC::VSPLTH:
       case PPC::XXSPLTW: {
         unsigned MyOpcode = MI.getOpcode();
         unsigned OpNo = MyOpcode == PPC::XXSPLTW ? 1 : 2;
         unsigned TrueReg =