[PATCH] D26066: [PowerPC] Improvements for BUILD_VECTOR Vol. 4

Ehsan Amiri via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 28 11:20:50 PDT 2016


amehsan added a comment.

In https://reviews.llvm.org/D26066#581855, @nemanjai wrote:

> Here are some examples of terrible code gen currently (rather than providing code, I'm just illustrating these with pseudo-SDAG nodes):


Could you also comment on what is your process for identifying problematic patterns that have been addressed in these patches?



================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:151-154
+                BuildMI(MBB, &MI, MI.getDebugLoc(),
+                        TII->get(PPC::COPY), MI.getOperand(0).getReg())
+                  .addOperand(MI.getOperand(1));
+                ToErase = &MI;
----------------
Can't we just replace uses of MI.getOperand(0) with MI.getOperand(1) instead of creating a new copy instruction? 


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:244-246
+          (MyOpcode == PPC::XXSPLTW && DefOpcode == PPC::LXVWSX) ||
+          (MyOpcode == PPC::XXSPLTW && DefOpcode == PPC::MTVSRWS)||
+          (MyOpcode == PPC::XXSPLTW && isConvertOfSplat());
----------------
Given the name of variable being defined 'SameOpcode' these three lines that you have added requires some comment explaining what's going on. 


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:304-334
+            if (P1->getOpcode() == PPC::FRSP &&
+                MRI->hasOneNonDBGUse(P1->getOperand(0).getReg())) {
+              unsigned ConvReg1 = P1->getOperand(1).getReg();
+              unsigned FRSP1Defines = P1->getOperand(0).getReg();
+              for (MachineInstr &I : MRI->use_instructions(FRSP1Defines)) {
+                for (int i = 0, e = I.getNumOperands(); i < e; i++)
+                  if (I.getOperand(i).isReg() &&
----------------
I think you can factor out the common logic here either in a lambda or a helper function. You are doing very similar things for P1 and P2.


Repository:
  rL LLVM

https://reviews.llvm.org/D26066





More information about the llvm-commits mailing list