[TII] Use optional destination and source pair as a return value (NFC)

[NikolaPrica]

Summary

Refactor usage of isCopyInstrImpl, isCopyInstr and isAddImmediate methods to return optional machine operand pair of destination and source registers.

[aprantl]

LGTM.

[vsk]

Lgtm, thanks for cleaning this up.

[RKSimon]

@NikolaPrica This is failing on MSVC builds - do you have a fix or should I revert?

[RKSimon]

Reverted in rG3842b94c4e7292de5f9e368bd60c64fc084c1bbe

[NikolaPrica]

Thanks @RKSimon !

DestSourcePair was not is_trivially_copyable with MSVC toolchain so I've updated its members to be pointers instead of references.

llvm/include/llvm/CodeGen/TargetInstrInfo.h

 class TargetRegisterInfo;
 class TargetSchedModel;
 class TargetSubtargetInfo;
 
 template <class T> class SmallVectorImpl;
 
 using ParamLoadedValue = std::pair<MachineOperand, DIExpression*>;
 
+struct DestSourcePair {
+  const MachineOperand *Destination;
+  const MachineOperand *Source;
+
+  DestSourcePair(const MachineOperand &Dest, const MachineOperand &Src)
+      : Destination(&Dest), Source(&Src) {}
+};
+
 //---------------------------------------------------------------------------
 ///
 /// TargetInstrInfo - Interface to description of machine instruction set
 ///
 class TargetInstrInfo : public MCInstrInfo {
 public:
   TargetInstrInfo(unsigned CFSetupOpcode = ~0u, unsigned CFDestroyOpcode = ~0u,
                   unsigned CatchRetOpcode = ~0u, unsigned ReturnOpcode = ~0u)
public:
   virtual void copyPhysReg(MachineBasicBlock &MBB,
                            MachineBasicBlock::iterator MI, const DebugLoc &DL,
                            unsigned DestReg, unsigned SrcReg,
                            bool KillSrc) const {
     llvm_unreachable("Target didn't implement TargetInstrInfo::copyPhysReg!");
   }
 
 protected:
-  /// Target-dependent implemenation for IsCopyInstr.
+  /// Target-dependent implementation for IsCopyInstr.
   /// 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 isCopyInstrImpl(const MachineInstr &MI,
-                               const MachineOperand *&Source,
-                               const MachineOperand *&Destination) const {
-    return false;
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  virtual Optional<DestSourcePair>
+  isCopyInstrImpl(const MachineInstr &MI) const {
+    return None;
   }
 
 public:
   /// 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.
-  /// For COPY-instruction the method naturally returns true, for all other
-  /// instructions the method calls target-dependent implementation.
-  bool isCopyInstr(const MachineInstr &MI, const MachineOperand *&Source,
-                   const MachineOperand *&Destination) const {
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  /// For COPY-instruction the method naturally returns destination and source
+  /// registers as machine operands, for all other instructions the method calls
+  /// target-dependent implementation.
+  Optional<DestSourcePair> isCopyInstr(const MachineInstr &MI) const {
     if (MI.isCopy()) {
-      Destination = &MI.getOperand(0);
-      Source = &MI.getOperand(1);
-      return true;
+      return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
     }
-    return isCopyInstrImpl(MI, Source, Destination);
+    return isCopyInstrImpl(MI);
   }
 
   /// If the specific machine instruction is an instruction that adds an
-  /// immediate value to its \c Source operand and stores it in \c Destination,
-  /// return true along with \c Destination and \c Source machine operand to
-  /// which \c Offset has been added.
-  virtual bool isAddImmediate(const MachineInstr &MI,
-                              const MachineOperand *&Destination,
-                              const MachineOperand *&Source,
+  /// immediate value to its source operand and stores it in destination,
+  /// return destination and source registers as machine operands along with
+  /// \c Offset which has been added.
+  virtual Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
                                                   int64_t &Offset) const {
-    return false;
+    return None;
   }
 
   /// 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,
                                    MachineBasicBlock::iterator MI,

llvm/lib/CodeGen/LiveDebugValues.cpp

 
 /// If \p MI is a register copy instruction, that copies a previously tracked
 /// value from one register to another register that is callee saved, we
 /// create new DBG_VALUE instruction  described with copy destination register.
 void LiveDebugValues::transferRegisterCopy(MachineInstr &MI,
                                            OpenRangesSet &OpenRanges,
                                            VarLocMap &VarLocIDs,
                                            TransferMap &Transfers) {
-  const MachineOperand *SrcRegOp, *DestRegOp;
 
-  if (!TII->isCopyInstr(MI, SrcRegOp, DestRegOp) || !SrcRegOp->isKill() ||
-      !DestRegOp->isDef())
+  auto DestSrc = TII->isCopyInstr(MI);
+  if (!DestSrc)
+    return;
+
+  const MachineOperand *DestRegOp = DestSrc->Destination;
+  const MachineOperand *SrcRegOp = DestSrc->Source;
+  if (!SrcRegOp->isKill() || !DestRegOp->isDef())
     return;
 
   auto isCalleeSavedReg = [&](unsigned Reg) {
     for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
       if (CalleeSavedRegs.test(*RAI))
         return true;
     return false;
   };

llvm/lib/CodeGen/TargetInstrInfo.cpp

bool TargetInstrInfo::hasLowDefLatency(const TargetSchedModel &SchedModel,
   int DefCycle = ItinData->getOperandCycle(DefClass, DefIdx);
   return (DefCycle != -1 && DefCycle <= 1);
 }
 
 Optional<ParamLoadedValue>
 TargetInstrInfo::describeLoadedValue(const MachineInstr &MI) const {
   const MachineFunction *MF = MI.getMF();
   DIExpression *Expr = DIExpression::get(MF->getFunction().getContext(), {});
-  const MachineOperand *SrcRegOp, *DestRegOp;
   int64_t Offset;
 
-  if (isCopyInstr(MI, SrcRegOp, DestRegOp)) {
-    return ParamLoadedValue(*SrcRegOp, Expr);
-  } else if (isAddImmediate(MI, DestRegOp, SrcRegOp, Offset)) {
+  if (auto DestSrc = isCopyInstr(MI)) {
+    return ParamLoadedValue(*DestSrc->Source, Expr);
+  } else if (auto DestSrc = isAddImmediate(MI, Offset)) {
     Expr = DIExpression::prepend(Expr, DIExpression::ApplyOffset, Offset);
-    return ParamLoadedValue(*SrcRegOp, Expr);
+    return ParamLoadedValue(*DestSrc->Source, Expr);
   }
 
   return None;
 }
 
 /// Both DefMI and UseMI must be valid.  By default, call directly to the
 /// itinerary. This may be overriden by the target.
 int TargetInstrInfo::getOperandLatency(const InstrItineraryData *ItinData,

llvm/lib/Target/AArch64/AArch64InstrInfo.h

public:
   bool shouldOutlineFromFunctionByDefault(MachineFunction &MF) const override;
   /// Returns true if the instruction has a shift by immediate that can be
   /// executed in one cycle less.
   static bool isFalkorShiftExtFast(const MachineInstr &MI);
   /// Return true if the instructions is a SEH instruciton used for unwinding
   /// on Windows.
   static bool isSEHInstruction(const MachineInstr &MI);
 
-  bool isAddImmediate(const MachineInstr &MI,
-                      const MachineOperand *&Destination,
-                      const MachineOperand *&Source,
+  Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
                                           int64_t &Offset) const override;
 
   Optional<ParamLoadedValue>
   describeLoadedValue(const MachineInstr &MI) const override;
 
 #define GET_INSTRINFO_HELPER_DECLS
 #include "AArch64GenInstrInfo.inc"
 
 protected:
-  /// 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.
-  bool isCopyInstrImpl(const MachineInstr &MI, const MachineOperand *&Source,
-                       const MachineOperand *&Destination) const override;
+  /// If the specific machine instruction is an instruction that moves/copies
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  Optional<DestSourcePair>
+  isCopyInstrImpl(const MachineInstr &MI) const override;
 
 private:
   /// Sets the offsets on outlined instructions in \p MBB which use SP
   /// so that they will be valid post-outlining.
   ///
   /// \param MBB A \p MachineBasicBlock in an outlined function.
   void fixupPostOutline(MachineBasicBlock &MBB) const;
 

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

MachineBasicBlock::iterator AArch64InstrInfo::insertOutlinedCall(
   return CallPt;
 }
 
 bool AArch64InstrInfo::shouldOutlineFromFunctionByDefault(
   MachineFunction &MF) const {
   return MF.getFunction().hasMinSize();
 }
 
-bool AArch64InstrInfo::isCopyInstrImpl(
-    const MachineInstr &MI, const MachineOperand *&Source,
-    const MachineOperand *&Destination) const {
+Optional<DestSourcePair>
+AArch64InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
 
   // AArch64::ORRWrs and AArch64::ORRXrs with WZR/XZR reg
   // and zero immediate operands used as an alias for mov instruction.
   if (MI.getOpcode() == AArch64::ORRWrs &&
       MI.getOperand(1).getReg() == AArch64::WZR &&
       MI.getOperand(3).getImm() == 0x0) {
-    Destination = &MI.getOperand(0);
-    Source = &MI.getOperand(2);
-    return true;
+    return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
   }
 
   if (MI.getOpcode() == AArch64::ORRXrs &&
       MI.getOperand(1).getReg() == AArch64::XZR &&
       MI.getOperand(3).getImm() == 0x0) {
-    Destination = &MI.getOperand(0);
-    Source = &MI.getOperand(2);
-    return true;
+    return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
   }
 
-  return false;
+  return None;
 }
 
-bool AArch64InstrInfo::isAddImmediate(const MachineInstr &MI,
-                                      const MachineOperand *&Destination,
-                                      const MachineOperand *&Source,
-                                      int64_t &Offset) const {
+Optional<DestSourcePair>
+AArch64InstrInfo::isAddImmediate(const MachineInstr &MI,
   int Sign = 1;
   switch (MI.getOpcode()) {
   default:
-    return false;
+    return None;
   case AArch64::SUBWri:
   case AArch64::SUBXri:
   case AArch64::SUBSWri:
   case AArch64::SUBSXri:
     Sign *= -1;
     LLVM_FALLTHROUGH;
   case AArch64::ADDSWri:
   case AArch64::ADDSXri:
   case AArch64::ADDWri:
   case AArch64::ADDXri: {
     // TODO: Third operand can be global address (usually some string).
     if (!MI.getOperand(0).isReg() || !MI.getOperand(1).isReg() ||
         !MI.getOperand(2).isImm())
-      return false;
-    Source = &MI.getOperand(1);
+      return None;
     Offset = MI.getOperand(2).getImm() * Sign;
     int Shift = MI.getOperand(3).getImm();
     assert((Shift == 0 || Shift == 12) && "Shift can be either 0 or 12");
     Offset = Offset << Shift;
   }
   }
-  Destination = &MI.getOperand(0);
-  return true;
+  return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
 }
 
 Optional<ParamLoadedValue>
 AArch64InstrInfo::describeLoadedValue(const MachineInstr &MI) const {
   switch (MI.getOpcode()) {
   case AArch64::MOVZWi:
   case AArch64::MOVZXi:
     if (!MI.getOperand(1).isImm())

llvm/lib/Target/ARM/ARMBaseInstrInfo.h

protected:
   ///
   /// Do not call this method for a non-commutable instruction or for
   /// non-commutable pair of operand indices OpIdx1 and OpIdx2.
   /// Even though the instruction is commutable, the method may still
   /// fail to commute the operands, null pointer is returned in such cases.
   MachineInstr *commuteInstructionImpl(MachineInstr &MI, bool NewMI,
                                        unsigned OpIdx1,
                                        unsigned OpIdx2) const override;
-
-  /// 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.
-  bool isCopyInstrImpl(const MachineInstr &MI, const MachineOperand *&Source,
-                       const MachineOperand *&Destination) const override;
+  /// If the specific machine instruction is an instruction that moves/copies
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  Optional<DestSourcePair>
+  isCopyInstrImpl(const MachineInstr &MI) const override;
 
 public:
   // Return whether the target has an explicit NOP encoding.
   bool hasNOP() const;
 
   // Return the non-pre/post incrementing version of 'Opc'. Return 0
   // if there is not such an opcode.
   virtual unsigned getUnindexedOpcode(unsigned Opc) const = 0;
unsigned getFramePred(const MachineInstr &MI) const {
     // 0 - frame size
     // 1 - arg of CALLSEQ_START/CALLSEQ_END
     // 2 - predicate code (like ARMCC::AL)
     // - added by predOps:
     // 3 - predicate reg
     return MI.getOperand(3).getReg();
   }
 
-  bool isAddImmediate(const MachineInstr &MI,
-                      const MachineOperand *&Destination,
-                      const MachineOperand *&Source,
+  Optional<DestSourcePair> isAddImmediate(const MachineInstr &MI,
                                           int64_t &Offset) const override;
 };
 
 /// Get the operands corresponding to the given \p Pred value. By default, the
 /// predicate register is assumed to be 0 (no register), but you can pass in a
 /// \p PredReg if that is not the case.
 static inline std::array<MachineOperand, 2> predOps(ARMCC::CondCodes Pred,
                                                     unsigned PredReg = 0) {
   return {{MachineOperand::CreateImm(static_cast<int64_t>(Pred)),

llvm/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::isCopyInstrImpl(const MachineInstr &MI,
-                                       const MachineOperand *&Src,
-                                       const MachineOperand *&Dest) const {
+Optional<DestSourcePair>
+ARMBaseInstrInfo::isCopyInstrImpl(const MachineInstr &MI) 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);
-  return true;
+    return None;
+  return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
 }
 
 const MachineInstrBuilder &
 ARMBaseInstrInfo::AddDReg(MachineInstrBuilder &MIB, unsigned Reg,
                           unsigned SubIdx, unsigned State,
                           const TargetRegisterInfo *TRI) const {
   if (!SubIdx)
     return MIB.addReg(Reg, State);
static const std::pair<unsigned, const char *> TargetFlags[] = {
       {MO_GOT, "arm-got"},
       {MO_SBREL, "arm-sbrel"},
       {MO_DLLIMPORT, "arm-dllimport"},
       {MO_SECREL, "arm-secrel"},
       {MO_NONLAZY, "arm-nonlazy"}};
   return makeArrayRef(TargetFlags);
 }
 
-bool ARMBaseInstrInfo::isAddImmediate(const MachineInstr &MI,
-                                      const MachineOperand *&Destination,
-                                      const MachineOperand *&Source,
-                                      int64_t &Offset) const {
+Optional<DestSourcePair>
+ARMBaseInstrInfo::isAddImmediate(const MachineInstr &MI,
   int Sign = 1;
   unsigned Opcode = MI.getOpcode();
 
   // We describe SUBri or ADDri instructions.
   if (Opcode == ARM::SUBri)
     Sign = -1;
   else if (Opcode != ARM::ADDri)
-    return false;
+    return None;
 
   // TODO: Third operand can be global address (usually some string). Since
   //       strings can be relocated we cannot calculate their offsets for
   //       now.
   if (!MI.getOperand(0).isReg() || !MI.getOperand(1).isReg() ||
       !MI.getOperand(2).isImm())
-    return false;
+    return None;
 
-  Destination = &MI.getOperand(0);
-  Source = &MI.getOperand(1);
   Offset = MI.getOperand(2).getImm() * Sign;
-  return true;
+  return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
 }
 
 bool llvm::registerDefinedBetween(unsigned Reg,
                                   MachineBasicBlock::iterator From,
                                   MachineBasicBlock::iterator To,
                                   const TargetRegisterInfo *TRI) {
   for (auto I = From; I != To; ++I)
     if (I->modifiesRegister(Reg, TRI))

llvm/lib/Target/Mips/Mips16InstrInfo.h

public:
 
   const MCInstrDesc& AddiuSpImm(int64_t Imm) const;
 
   void BuildAddiuSpImm
     (MachineBasicBlock &MBB, MachineBasicBlock::iterator I, int64_t Imm) const;
 
 protected:
   /// 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.
-  bool isCopyInstrImpl(const MachineInstr &MI, const MachineOperand *&Source,
-                       const MachineOperand *&Destination) const override;
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  Optional<DestSourcePair> isCopyInstrImpl(const MachineInstr &MI) const override;
 
 private:
   unsigned getAnalyzableBrOpc(unsigned Opc) const override;
 
   void ExpandRetRA16(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                    unsigned Opc) const;
 
   // Adjust SP by Amount bytes where bytes can be up to 32bit number.

llvm/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::isCopyInstrImpl(const MachineInstr &MI,
-                                      const MachineOperand *&Src,
-                                      const MachineOperand *&Dest) const {
-  if (MI.isMoveReg()) {
-    Dest = &MI.getOperand(0);
-    Src = &MI.getOperand(1);
-    return true;
-  }
-  return false;
+Optional<DestSourcePair>
+Mips16InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
+  if (MI.isMoveReg())
+    return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
+  return None;
 }
 
 void Mips16InstrInfo::storeRegToStack(MachineBasicBlock &MBB,
                                       MachineBasicBlock::iterator I,
                                       unsigned SrcReg, bool isKill, int FI,
                                       const TargetRegisterClass *RC,
                                       const TargetRegisterInfo *TRI,
                                       int64_t Offset) const {

llvm/lib/Target/Mips/MipsSEInstrInfo.h

public:
   /// non-NULL parameter, the last instruction is not emitted, but instead
   /// its immediate operand is returned in NewImm.
   unsigned loadImmediate(int64_t Imm, MachineBasicBlock &MBB,
                          MachineBasicBlock::iterator II, const DebugLoc &DL,
                          unsigned *NewImm) const;
 
 protected:
   /// 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.
-  bool isCopyInstrImpl(const MachineInstr &MI, const MachineOperand *&Source,
-                       const MachineOperand *&Destination) const override;
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  Optional<DestSourcePair>
+  isCopyInstrImpl(const MachineInstr &MI) const override;
 
 private:
   unsigned getAnalyzableBrOpc(unsigned Opc) const override;
 
   void expandRetRA(MachineBasicBlock &MBB, MachineBasicBlock::iterator I) const;
 
   void expandERet(MachineBasicBlock &MBB, MachineBasicBlock::iterator I) const;
 

llvm/lib/Target/Mips/MipsSEInstrInfo.cpp

case Mips::RDDSP_MM:
     break;
   }
   return true;
 }
 
 /// 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::isCopyInstrImpl(const MachineInstr &MI,
-                                      const MachineOperand *&Src,
-                                      const MachineOperand *&Dest) const {
+Optional<DestSourcePair>
+MipsSEInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
   bool isDSPControlWrite = false;
   // Condition is made to match the creation of WRDSP/RDDSP copy instruction
   // from copyPhysReg function.
   if (isReadOrWriteToDSPReg(MI, isDSPControlWrite)) {
-    if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != (1<<4))
-      return false;
+    if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != (1 << 4))
+      return None;
     else if (isDSPControlWrite) {
-      Src = &MI.getOperand(0);
-      Dest = &MI.getOperand(2);
+      return DestSourcePair{MI.getOperand(2), MI.getOperand(0)};
+
     } else {
-      Dest = &MI.getOperand(0);
-      Src = &MI.getOperand(2);
+      return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
     }
-    return true;
   } else if (MI.isMoveReg() || isORCopyInst(MI)) {
-    Dest = &MI.getOperand(0);
-    Src = &MI.getOperand(1);
-    return true;
+    return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
   }
-  return false;
+  return None;
 }
 
 void MipsSEInstrInfo::
 storeRegToStack(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                 unsigned SrcReg, bool isKill, int FI,
                 const TargetRegisterClass *RC, const TargetRegisterInfo *TRI,
                 int64_t Offset) const {
   DebugLoc DL;

llvm/lib/Target/X86/X86InstrInfo.h

protected:
   /// non-commutable operands.
   /// Even though the instruction is commutable, the method may still
   /// fail to commute the operands, null pointer is returned in such cases.
   MachineInstr *commuteInstructionImpl(MachineInstr &MI, bool NewMI,
                                        unsigned CommuteOpIdx1,
                                        unsigned CommuteOpIdx2) const override;
 
   /// 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.
-  bool isCopyInstrImpl(const MachineInstr &MI, const MachineOperand *&Source,
-                       const MachineOperand *&Destination) const override;
+  /// value from one register to another register return destination and source
+  /// registers as machine operands.
+  Optional<DestSourcePair>
+  isCopyInstrImpl(const MachineInstr &MI) const override;
 
 private:
   /// This is a helper for convertToThreeAddress for 8 and 16-bit instructions.
   /// We use 32-bit LEA to form 3-address code by promoting to a 32-bit
   /// super-register and then truncating back down to a 8/16-bit sub-register.
   MachineInstr *convertToThreeAddressWithLEA(unsigned MIOpc,
                                              MachineFunction::iterator &MFI,
                                              MachineInstr &MI,

llvm/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');
   report_fatal_error("Cannot emit physreg copy instruction");
 }
 
-bool X86InstrInfo::isCopyInstrImpl(const MachineInstr &MI,
-                                   const MachineOperand *&Src,
-                                   const MachineOperand *&Dest) const {
-  if (MI.isMoveReg()) {
-    Dest = &MI.getOperand(0);
-    Src = &MI.getOperand(1);
-    return true;
-  }
-  return false;
+Optional<DestSourcePair>
+X86InstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
+  if (MI.isMoveReg())
+    return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
+  return None;
 }
 
 static unsigned getLoadStoreRegOpcode(unsigned Reg,
                                       const TargetRegisterClass *RC,
                                       bool isStackAligned,
                                       const X86Subtarget &STI,
                                       bool load) {
   bool HasAVX = STI.hasAVX();