TargetInstrInfo: Provide default implementation of isTailCall().

[MatzeB]

Summary

In fact this default implementation should be the only implementation,
keep it virtual for now to accomodate targets that don't model flags
correctly.

[MatzeB]

I did not touch HexagonInstrInfo::isTailCall() here as it wasn't immediately obvious whether the flags are correct there and the default implementation would work. Some target maintainer should double check and if so maybe we can get rid of the complete callback.

[kparzysz]

In D30747#695716, @MatzeB wrote:

I did not touch HexagonInstrInfo::isTailCall() here as it wasn't immediately obvious whether the flags are correct there and the default implementation would work. Some target maintainer should double check and if so maybe we can get rid of the complete callback.

Hexagon has over 100 different opcodes for branches, and each of them can be used as a tail call. We are not going to invent 100+ phony opcodes just to put flags on them.

[MatzeB]

In D30747#695741, @kparzysz wrote:

In D30747#695716, @MatzeB wrote:

I did not touch HexagonInstrInfo::isTailCall() here as it wasn't immediately obvious whether the flags are correct there and the default implementation would work. Some target maintainer should double check and if so maybe we can get rid of the complete callback.

Hexagon has over 100 different opcodes for branches, and each of them can be used as a tail call. We are not going to invent 100+ phony opcodes just to put flags on them.

Yes I somewhat expected this answer and that we will end up in a similar discussion as in http://llvm.org/PR31960. So let's keep the callback (and the comment about wrong flags).

[dberris]

LGTM -- I suspect this doesn't break the compiler-rt tests for XRay?

include/llvm/Target/TargetInstrInfo.h

1499–1501 ↗

(On Diff #91048)

Can we make this comment more direct? Something like:

"Override this method for targets where tail calls are not always both return and call instructions."

[MatzeB]

In D30747#702333, @dberris wrote:

LGTM -- I suspect this doesn't break the compiler-rt tests for XRay?

Can I even run them on a mac? I also don't have PPC hardware available. But we surely have buildbots for that (and if not I blame someone else for the lack of bots).

include/llvm/Target/TargetInstrInfo.h

1499–1501 ↗

(On Diff #91048)

Ok how about

"Override this method on targets that do not properly set MCID::Return and MCID::Call on tail call instructions."

to not give the impression that this is recommended.

[dberris]

In D30747#702343, @MatzeB wrote:

In D30747#702333, @dberris wrote:

LGTM -- I suspect this doesn't break the compiler-rt tests for XRay?

Can I even run them on a mac? I also don't have PPC hardware available. But we surely have buildbots for that (and if not I blame someone else for the lack of bots).

Nope, the XRay runtime doesn't work in MacOS yet.

There are bots that run these. I'm not sure if you want to wait to find out if they break. :)

include/llvm/Target/TargetInstrInfo.h

1499–1501 ↗

(On Diff #91048)

That's better I think.

[MatzeB]

In D30747#702358, @dberris wrote:

In D30747#702343, @MatzeB wrote:

In D30747#702333, @dberris wrote:

LGTM -- I suspect this doesn't break the compiler-rt tests for XRay?

Can I even run them on a mac? I also don't have PPC hardware available. But we surely have buildbots for that (and if not I blame someone else for the lack of bots).

Nope, the XRay runtime doesn't work in MacOS yet.

There are bots that run these. I'm not sure if you want to wait to find out if they break. :)

Unless you run them for me that's all I can do. Reverts are cheap (breaking the build isn't so nice but as long as we don't have pre-commit checking in llvm...).

llvm/trunk/include/llvm/Target/TargetInstrInfo.h

public:
   ///
   /// MIR Serialization is able to serialize only the target flags that are
   /// defined by this method.
   virtual ArrayRef<std::pair<unsigned, const char *>>
   getSerializableBitmaskMachineOperandTargetFlags() const {
     return None;
   }
 
-  /// Determines whether |Inst| is a tail call instruction.
+  /// Determines whether \p Inst is a tail call instruction. Override this
+  /// method on targets that do not properly set MCID::Return and MCID::Call on
+  /// tail call instructions."
   virtual bool isTailCall(const MachineInstr &Inst) const {
-    return false;
+    return Inst.isReturn() && Inst.isCall();
   }
 
   /// True if the instruction is bound to the top of its basic block and no
   /// other instructions shall be inserted before it. This can be implemented
   /// to prevent register allocator to insert spills before such instructions.
   virtual bool isBasicBlockPrologue(const MachineInstr &MI) const {
     return false;
   }

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h

public:
   bool isLdStPairSuppressed(const MachineInstr &MI) const;
 
   /// Return true if this is an unscaled load/store.
   bool isUnscaledLdSt(unsigned Opc) const;
 
   /// Return true if this is an unscaled load/store.
   bool isUnscaledLdSt(MachineInstr &MI) const;
 
-  bool isTailCall(const MachineInstr &Inst) const override;
-
   static bool isPairableLdStInst(const MachineInstr &MI) {
     switch (MI.getOpcode()) {
     default:
       return false;
     // Scaled instructions.
     case AArch64::STRSui:
     case AArch64::STRDui:
     case AArch64::STRQui:

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp

case AArch64::LDURSHWi:
     return true;
   }
 }
 
 bool AArch64InstrInfo::isUnscaledLdSt(MachineInstr &MI) const {
   return isUnscaledLdSt(MI.getOpcode());
 }
 
-bool AArch64InstrInfo::isTailCall(const MachineInstr &Inst) const
-{
-  switch (Inst.getOpcode()) {
-  case AArch64::TCRETURNdi:
-  case AArch64::TCRETURNri:
-    return true;
-  default:
-    return false;
-  }
-}
-
 // Is this a candidate for ld/st merging or pairing?  For example, we don't
 // touch volatiles or load/stores that have a hint to avoid pair formation.
 bool AArch64InstrInfo::isCandidateToMergeOrPair(MachineInstr &MI) const {
   // If this is a volatile load/store, don't mess with it.
   if (MI.hasOrderedMemoryRef())
     return false;
 
   // Make sure this is a reg+imm (as opposed to an address reloc).

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.h

 public:
   // Return whether the target has an explicit NOP encoding.
   bool hasNOP() const;
 
   virtual void getNoopForElfTarget(MCInst &NopInst) const {
     getNoopForMachoTarget(NopInst);
   }
 
-  bool isTailCall(const MachineInstr &Inst) const override;
-
   // 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;
 
   MachineInstr *convertToThreeAddress(MachineFunction::iterator &MFI,
                                       MachineInstr &MI,
                                       LiveVariables *LV) const override;
 

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), get(ARM::FCONSTD), DReg)
       .add(predOps(ARMCC::AL));
   MI.addRegisterKilled(DReg, TRI, true);
 }
 
 bool ARMBaseInstrInfo::hasNOP() const {
   return Subtarget.getFeatureBits()[ARM::HasV6KOps];
 }
 
-bool ARMBaseInstrInfo::isTailCall(const MachineInstr &Inst) const
-{
-  switch (Inst.getOpcode()) {
-  case ARM::TAILJMPd:
-  case ARM::TAILJMPr:
-  case ARM::TCRETURNdi:
-  case ARM::TCRETURNri:
-    return true;
-  default:
-    return false;
-  }
-}
-
 bool ARMBaseInstrInfo::isSwiftFastImmShift(const MachineInstr *MI) const {
   if (MI->getNumOperands() < 4)
     return true;
   unsigned ShOpVal = MI->getOperand(3).getImm();
   unsigned ShImm = ARM_AM::getSORegOffset(ShOpVal);
   // Swift supports faster shifts for: lsl 2, lsl 1, and lsr 1.
   if ((ShImm == 1 && ARM_AM::getSORegShOp(ShOpVal) == ARM_AM::lsr) ||
       ((ShImm == 1 || ShImm == 2) &&

llvm/trunk/lib/Target/X86/X86InstrInfo.h

MachineInstr *optimizeLoadInstr(MachineInstr &MI,
                                   MachineInstr *&DefMI) const override;
 
   std::pair<unsigned, unsigned>
   decomposeMachineOperandsTargetFlags(unsigned TF) const override;
 
   ArrayRef<std::pair<unsigned, const char *>>
   getSerializableDirectMachineOperandTargetFlags() const override;
 
-  bool isTailCall(const MachineInstr &Inst) const override;
-
   unsigned getOutliningBenefit(size_t SequenceSize,
                                size_t Occurrences,
                                bool CanBeTailCall) const override;
 
   bool isFunctionSafeToOutlineFrom(MachineFunction &MF) const override;
 
   llvm::X86GenInstrInfo::MachineOutlinerInstrType
   getOutliningType(MachineInstr &MI) const override;

llvm/trunk/lib/Target/X86/X86InstrInfo.cpp

static const std::pair<unsigned, const char *> TargetFlags[] = {
       {MO_DARWIN_NONLAZY, "x86-darwin-nonlazy"},
       {MO_DARWIN_NONLAZY_PIC_BASE, "x86-darwin-nonlazy-pic-base"},
       {MO_TLVP, "x86-tlvp"},
       {MO_TLVP_PIC_BASE, "x86-tlvp-pic-base"},
       {MO_SECREL, "x86-secrel"}};
   return makeArrayRef(TargetFlags);
 }
 
-bool X86InstrInfo::isTailCall(const MachineInstr &Inst) const {
-  switch (Inst.getOpcode()) {
-    case X86::TCRETURNdi:
-    case X86::TCRETURNmi:
-    case X86::TCRETURNri:
-    case X86::TCRETURNdi64:
-    case X86::TCRETURNmi64:
-    case X86::TCRETURNri64:
-    case X86::TAILJMPd:
-    case X86::TAILJMPm:
-    case X86::TAILJMPr:
-    case X86::TAILJMPd64:
-    case X86::TAILJMPm64:
-    case X86::TAILJMPr64:
-    case X86::TAILJMPm64_REX:
-    case X86::TAILJMPr64_REX:
-      return true;
-    default:
-      return false;
-  }
-}
-
 namespace {
   /// Create Global Base Reg pass. This initializes the PIC
   /// global base register for x86-32.
   struct CGBR : public MachineFunctionPass {
     static char ID;
     CGBR() : MachineFunctionPass(ID) {}
 
     bool runOnMachineFunction(MachineFunction &MF) override {