Change TII isCopyInstr way of returning arguments(NFC)

[NikolaPrica]

Summary

Make TII isCopyInstr() return MachineOperands through pointer to pointer instead via reference.

[NikolaPrica]

Ping.

[arsenm]

Why? This looks worse to me

[NikolaPrica]

Yes. I agree. It would look prettier with reference. But we can't construct default MachineOperand like "MachineOperand MO;" and pass it to this isCopyInstr. We could construct it like "MachineOperand MO = MachineInstr::CreateReg();" but that is unnecessary object construction. Better way would be without need for object construction. Is my reasoning correct or am I missing something?

[arsenm]

In D47364#1122117, @NikolaPrica wrote:

Yes. I agree. It would look prettier with reference. But we can't construct default MachineOperand like "MachineOperand MO;" and pass it to this isCopyInstr. We could construct it like "MachineOperand MO = MachineInstr::CreateReg();" but that is unnecessary object construction. Better way would be without need for object construction. Is my reasoning correct or am I missing something?

I would expect this to be a reference to pointer, not a pointer to pointer

[NikolaPrica]

Updated isCopyInstr to take pointer to reference of MachineOperand.

[arsenm]

LGTM

include/llvm/CodeGen/TargetInstrInfo.h

virtual void copyPhysReg(MachineBasicBlock &MBB,
                            unsigned DestReg, unsigned SrcReg,
                            bool KillSrc) const {
     llvm_unreachable("Target didn't implement TargetInstrInfo::copyPhysReg!");
   }
 
   /// If the specific machine instruction is a instruction that moves/copies
   /// value from one register to another register return true along with
   /// @Source machine operand and @Destination machine operand.
-  virtual bool isCopyInstr(const MachineInstr &MI, MachineOperand &Source,
-                           MachineOperand &Destination) const {
+  virtual bool isCopyInstr(const MachineInstr &MI,
+                           const MachineOperand *&SourceOpNum,
+                           const MachineOperand *&Destination) const {
     return false;
   }
 
   /// Store the specified register of the given register class to the specified
   /// stack frame index. The store instruction is to be added to the given
   /// machine basic block before the specified machine instruction. If isKill
   /// is true, the register operand is the last use and must be marked kill.
   virtual void storeRegToStackSlot(MachineBasicBlock &MBB,

lib/Target/ARM/ARMBaseInstrInfo.h

public:
   void copyFromCPSR(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                     unsigned DestReg, bool KillSrc,
                     const ARMSubtarget &Subtarget) const;
 
   void copyPhysReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                    const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
                    bool KillSrc) const override;
 
-  bool isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                   MachineOperand &Dest) const override;
+  bool isCopyInstr(const MachineInstr &MI, const MachineOperand *&Src,
+                   const MachineOperand *&Dest) const override;
 
   void storeRegToStackSlot(MachineBasicBlock &MBB,
                            MachineBasicBlock::iterator MBBI,
                            unsigned SrcReg, bool isKill, int FrameIndex,
                            const TargetRegisterClass *RC,
                            const TargetRegisterInfo *TRI) const override;
 
   void loadRegFromStackSlot(MachineBasicBlock &MBB,

lib/Target/ARM/ARMBaseInstrInfo.cpp

if (Opc == ARM::MOVr)
       Mov = Mov.add(condCodeOp());
   }
   // Add implicit super-register defs and kills to the last instruction.
   Mov->addRegisterDefined(DestReg, TRI);
   if (KillSrc)
     Mov->addRegisterKilled(SrcReg, TRI);
 }
 
-bool ARMBaseInstrInfo::isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                                   MachineOperand &Dest) const {
+bool ARMBaseInstrInfo::isCopyInstr(const MachineInstr &MI,
+                                   const MachineOperand *&Src,
+                                   const MachineOperand *&Dest) const {
   // VMOVRRD is also a copy instruction but it requires
   // special way of handling. It is more complex copy version
   // and since that we are not considering it. For recognition
   // of such instruction isExtractSubregLike MI interface fuction
   // could be used.
   // VORRq is considered as a move only if two inputs are
   // the same register.
   if (!MI.isMoveReg() ||
       (MI.getOpcode() == ARM::VORRq &&
        MI.getOperand(1).getReg() != MI.getOperand(2).getReg()))
     return false;
-  Dest = MI.getOperand(0);
-  Src = MI.getOperand(1);
+  Dest = &MI.getOperand(0);
+  Src = &MI.getOperand(1);
   return true;
 }
 
 const MachineInstrBuilder &
 ARMBaseInstrInfo::AddDReg(MachineInstrBuilder &MIB, unsigned Reg,
                           unsigned SubIdx, unsigned State,
                           const TargetRegisterInfo *TRI) const {
   if (!SubIdx)

lib/Target/Mips/Mips16InstrInfo.h

public:
   /// any side effects other than storing to the stack slot.
   unsigned isStoreToStackSlot(const MachineInstr &MI,
                               int &FrameIndex) const override;
 
   void copyPhysReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                    const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
                    bool KillSrc) const override;
 
-  bool isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                   MachineOperand &Dest) const override;
+  bool isCopyInstr(const MachineInstr &MI, const MachineOperand *&Src,
+                   const MachineOperand *&Dest) const override;
 
   void storeRegToStack(MachineBasicBlock &MBB,
                        MachineBasicBlock::iterator MBBI,
                        unsigned SrcReg, bool isKill, int FrameIndex,
                        const TargetRegisterClass *RC,
                        const TargetRegisterInfo *TRI,
                        int64_t Offset) const override;
 

lib/Target/Mips/Mips16InstrInfo.cpp

void Mips16InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
 
   if (DestReg)
     MIB.addReg(DestReg, RegState::Define);
 
   if (SrcReg)
     MIB.addReg(SrcReg, getKillRegState(KillSrc));
 }
 
-bool Mips16InstrInfo::isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                                  MachineOperand &Dest) const {
+bool Mips16InstrInfo::isCopyInstr(const MachineInstr &MI,
+                                  const MachineOperand *&Src,
+                                  const MachineOperand *&Dest) const {
   if (MI.isMoveReg()) {
-    Dest = MI.getOperand(0);
-    Src = MI.getOperand(1);
+    Dest = &MI.getOperand(0);
+    Src = &MI.getOperand(1);
     return true;
   }
   return false;
 }
 
 void Mips16InstrInfo::storeRegToStack(MachineBasicBlock &MBB,
                                       MachineBasicBlock::iterator I,
                                       unsigned SrcReg, bool isKill, int FI,

lib/Target/Mips/MipsSEInstrInfo.h

public:
   /// any side effects other than storing to the stack slot.
   unsigned isStoreToStackSlot(const MachineInstr &MI,
                               int &FrameIndex) const override;
 
   void copyPhysReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                    const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
                    bool KillSrc) const override;
 
-  bool isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                   MachineOperand &Dest) const override;
+  bool isCopyInstr(const MachineInstr &MI, const MachineOperand *&Src,
+                   const MachineOperand *&Dest) const override;
 
   void storeRegToStack(MachineBasicBlock &MBB,
                        MachineBasicBlock::iterator MI,
                        unsigned SrcReg, bool isKill, int FrameIndex,
                        const TargetRegisterClass *RC,
                        const TargetRegisterInfo *TRI,
                        int64_t Offset) const override;
 

lib/Target/Mips/MipsSEInstrInfo.cpp

switch (MI.getOpcode()) {
     default:
      return false;
   }
 }
 
 /// We check for the common case of 'or', as it's MIPS' preferred instruction
 /// for GPRs but we have to check the operands to ensure that is the case.
 /// Other move instructions for MIPS are directly identifiable.
-bool MipsSEInstrInfo::isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                                  MachineOperand &Dest) const {
+bool MipsSEInstrInfo::isCopyInstr(const MachineInstr &MI,
+                                  const MachineOperand *&Src,
+                                  const MachineOperand *&Dest) const {
   bool isDSPControlWrite = false;
   // Condition is made to match the creation of WRDSP/RDDSP copy instruction
   // from copyPhysReg function.
   if (isReadOrWritToDSPReg(MI, isDSPControlWrite)) {
-    if (!MI.getOperand(1).isImm() || !(MI.getOperand(1).getImm() == (1<<4)))
+    if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != (1<<4))
       return false;
     else if (isDSPControlWrite) {
-      Src = MI.getOperand(0);
-      Dest = MI.getOperand(2);
+      Src = &MI.getOperand(0);
+      Dest = &MI.getOperand(2);
     } else {
-      Dest = MI.getOperand(0);
-      Src = MI.getOperand(2);
+      Dest = &MI.getOperand(0);
+      Src = &MI.getOperand(2);
     }
     return true;
   } else if (MI.isMoveReg() || isORCopyInst(MI)) {
-    Dest = MI.getOperand(0);
-    Src = MI.getOperand(1);
+    Dest = &MI.getOperand(0);
+    Src = &MI.getOperand(1);
     return true;
   }
   return false;
 }
 
 void MipsSEInstrInfo::
 storeRegToStack(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                 unsigned SrcReg, bool isKill, int FI,

lib/Target/X86/X86InstrInfo.h

bool canInsertSelect(const MachineBasicBlock &, ArrayRef<MachineOperand> Cond,
                        unsigned, unsigned, int &, int &, int &) const override;
   void insertSelect(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                     const DebugLoc &DL, unsigned DstReg,
                     ArrayRef<MachineOperand> Cond, unsigned TrueReg,
                     unsigned FalseReg) const override;
   void copyPhysReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
                    const DebugLoc &DL, unsigned DestReg, unsigned SrcReg,
                    bool KillSrc) const override;
-  bool isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                   MachineOperand &Dest) const override;
+  bool isCopyInstr(const MachineInstr &MI, const MachineOperand *&Src,
+                   const MachineOperand *&Dest) const override;
   void storeRegToStackSlot(MachineBasicBlock &MBB,
                            MachineBasicBlock::iterator MI, unsigned SrcReg,
                            bool isKill, int FrameIndex,
                            const TargetRegisterClass *RC,
                            const TargetRegisterInfo *TRI) const override;
 
   void storeRegToAddr(MachineFunction &MF, unsigned SrcReg, bool isKill,
                       SmallVectorImpl<MachineOperand> &Addr,

lib/Target/X86/X86InstrInfo.cpp

if (SrcReg == X86::EFLAGS || DestReg == X86::EFLAGS) {
     report_fatal_error("Unable to copy EFLAGS physical register!");
   }
 
   LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to "
                     << RI.getName(DestReg) << '\n');
   llvm_unreachable("Cannot emit physreg copy instruction");
 }
 
-bool X86InstrInfo::isCopyInstr(const MachineInstr &MI, MachineOperand &Src,
-                               MachineOperand &Dest) const {
+bool X86InstrInfo::isCopyInstr(const MachineInstr &MI,
+                               const MachineOperand *&Src,
+                               const MachineOperand *&Dest) const {
   if (MI.isMoveReg()) {
-    Dest = MI.getOperand(0);
-    Src = MI.getOperand(1);
+    Dest = &MI.getOperand(0);
+    Src = &MI.getOperand(1);
     return true;
   }
   return false;
 }
 
 static unsigned getLoadStoreRegOpcode(unsigned Reg,
                                       const TargetRegisterClass *RC,
                                       bool isStackAligned,