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

[Ehsan Amiri via llvm-commits]

amehsan added inline comments.


================
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;
----------------
amehsan wrote:
> amehsan wrote:
> > nemanjai wrote:
> > > amehsan wrote:
> > > > Can't we just replace uses of MI.getOperand(0) with MI.getOperand(1) instead of creating a new copy instruction? 
> > > This is safe to do. The COPY will be removed if not needed. I did something similar before and it caused a problem when the operand defined by the removed instruction had another use. Please see https://reviews.llvm.org/D25493.
> > > I suppose I could traverse the use list for MI.getOperand(0) and see if it can be replaced as well, but I don't think that's any cleaner.
> > We don't know that the COPY is guaranteed to be removed. AFAIK, We should not rely on downstream optimizations if we can generate better code. Yes, all uses should be replaced. There is a utility method for that: Check: include/llvm/CodeGen/MachineRegisterInfo.h for method:
> >   void replaceRegWith(unsigned FromReg, unsigned ToReg)
> Looking at this method again, it has differences with replaceAllUsesWith. First I thought it is the same. Some comment from people with more experience will be helpful.
@hfinkel your comment here will be helpful as well. 


Repository:
  rL LLVM

https://reviews.llvm.org/D26066