[llvm] r280331 - [XRay][NFC] Promote isTailCall() as virtual in TargetInstrInfo.

Tue Mar 7 18:57:40 PST 2017

On Wed, Mar 8, 2017 at 1:52 PM Matthias Braun <mbraun at apple.com> wrote:

> PPC doesn't seem to have an TII::isTailCall() overload. You have to mark
> call and return instructions in llvm, even if the function is not called
> "call" or "return" in the instruction set. For example if you look up all
> those instructions that X86InstrInfo::isTailCall checks for, they are all
> marked with isReturn=1 and isCall=1 in the tablegen files.
>
>
Ah, right -- I keep forgetting this is at the LLVM level.

> The only thing I can find browsing around right now seems to be the
> Hexagon implementation os isTailCall() which looks suspicious to me. At the
> very least we should replace the default implementation of isTailCall()
> with "return MI.isReturn() && MI.isCall();" and state in the document that
> you only need to override this when your target is broken :) So people stop
> providing half-complete unnecessary implementations in their targets.
>
>
Sounds good to me. Testing should iron out the bugs if there are any anyway
if that default implementation doesn't quite do it.

Are you able to make that change, or can this wait?

> - Matthias
>
> On Mar 7, 2017, at 6:47 PM, Dean Michael Berris <dberris at google.com>
> wrote:
>
> As far as I can tell, on PowerPC there are special instructions that
> indicate a tail call which is neither a call nor a return. On X86, it's a
> normal jmp instruction, so it's neither a call nor a return either.
>
> On Wed, Mar 8, 2017 at 1:43 PM Matthias Braun <mbraun at apple.com> wrote:
>
> Do you have an example? Sounds more like some targets are broken and don't
> set the flags correctly to me...
>
> On Mar 7, 2017, at 6:42 PM, Dean Michael Berris <dberris at google.com>
> wrote:
>
> On some platforms, since they're special instructions, the check isn't as
> straight forward as that. At least I don't think it is. If it proves to be
> more efficient to do that instead of using something at a higher level to
> indicate the semantics then I'm fine with an alternative approach.
>
> On Wed, Mar 8, 2017 at 12:59 PM Matthias Braun <mbraun at apple.com> wrote:
>
> Just came accross this function, are you sure we need it? Shouldn't
> MachineInstr::isReturn() && MachineInstr::isCall() be the same as
> isTailCall()?
>
> - Matthias
>
> > On Aug 31, 2016, at 6:03 PM, Dean Michael Berris via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: dberris
> > Date: Wed Aug 31 20:03:22 2016
> > New Revision: 280331
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=280331&view=rev
> > Log:
> > [XRay][NFC] Promote isTailCall() as virtual in TargetInstrInfo.
> >
> > This change is broken out from D23986, where XRay detects tail call
> > exits.
> >
> > Modified:
> >    llvm/trunk/include/llvm/Target/TargetInstrInfo.h
> >    llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h
> >    llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
> >    llvm/trunk/lib/Target/X86/X86InstrInfo.h
> >
> > Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=280331&r1=280330&r2=280331&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)
> > +++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Wed Aug 31 20:03:22
> 2016
> > @@ -1481,6 +1481,11 @@ public:
> >     return None;
> >   }
> >
> > +  /// Determines whether |Inst| is a tail call instruction.
> > +  virtual bool isTailCall(const MachineInstr &Inst) const {
> > +    return false;
> > +  }
> > +
> > private:
> >   unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;
> >   unsigned CatchRetOpcode;
> >
> > Modified: llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h?rev=280331&r1=280330&r2=280331&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h (original)
> > +++ llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h Wed Aug 31 20:03:22
> 2016
> > @@ -340,7 +340,10 @@ public:
> >   bool isSignExtendingLoad(const MachineInstr &MI) const;
> >   bool isSolo(const MachineInstr &MI) const;
> >   bool isSpillPredRegOp(const MachineInstr &MI) const;
> > -  bool isTailCall(const MachineInstr &MI) const;
> > +
> > +  // Defined in Target.h.
> > +  bool isTailCall(const MachineInstr &MI) const override;
> > +
> >   bool isTC1(const MachineInstr &MI) const;
> >   bool isTC2(const MachineInstr &MI) const;
> >   bool isTC2Early(const MachineInstr &MI) const;
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=280331&r1=280330&r2=280331&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Wed Aug 31 20:03:22 2016
> > @@ -8061,6 +8061,29 @@ X86InstrInfo::getSerializableDirectMachi
> >   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::TAILJMPd64_REX:
> > +    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.
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.h?rev=280331&r1=280330&r2=280331&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrInfo.h (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrInfo.h Wed Aug 31 20:03:22 2016
> > @@ -541,6 +541,8 @@ public:
> >   ArrayRef<std::pair<unsigned, const char *>>
> >   getSerializableDirectMachineOperandTargetFlags() const override;
> >
> > +  bool isTailCall(const MachineInstr &Inst) const override;
> > +
> > protected:
> >   /// Commutes the operands in the given instruction by changing the
> operands
> >   /// order and/or changing the instruction's opcode and/or the
> immediate value
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170308/b2c9dc1d/attachment.html>