[PATCH] D52432: [PowerPC] Remove self-copies in pre-emit peephole

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

If the reviewers agree with my proposals for addressing the comments, I'll address them on the commit.



================
Comment at: lib/Target/PowerPC/PPCPreEmitPeephole.cpp:69
+          unsigned Opc = MI.getOpcode();
+          // Detect self copies.
+          if (Opc == PPC::OR || Opc == PPC::OR8 || Opc == PPC::FMR ||
----------------
hfinkel wrote:
> Please add a comment here stating that these can come out of the AADB.
Will add it on the commit. Thanks.


================
Comment at: lib/Target/PowerPC/PPCPreEmitPeephole.cpp:72
+              Opc == PPC::VOR || Opc == PPC::XXLOR || Opc == PPC::XXLORf ||
+              Opc == PPC::XSCPSGNDP) {
+            const MCInstrDesc &MCID = TII->get(Opc);
----------------
jsji wrote:
> How about PPC::MCRF, PPC::CROR,PPC::EVOR?  QPX copies if we still support QPX?
I didn't add the SPE and QPX ones because I can't test them. Of course, this should be a safe thing to do no matter what, but I am reluctant to add code I can't functionally test.
@hfinkel Do you think I should go ahead and add the following:
```
PPC::MCRF
PPC::QVFMR
PPC::QVFMRs
PPC::QVFMRb
PPC::CROR
PPC::EVOR
```


================
Comment at: lib/Target/PowerPC/PPCPreEmitPeephole.cpp:80
+              LLVM_DEBUG(MI.dump());
+              InstrsToErase.push_back(&MI);
+            }
----------------
jsji wrote:
> Maybe we should continue (skip the following check) once we identified that this is to be erased?
That's a good point. I'll update the patch to add a `continue` after either of the updates to `InstrsToErase` since we don't need to try to convert it to an R+I instruction if we know we're deleting it.


Repository:
  rL LLVM

https://reviews.llvm.org/D52432