[llvm-commits] [llvm] r169796 - in /llvm/trunk: include/llvm/CodeGen/FastISel.h lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Target/ARM/ARMFastISel.cpp lib/Target/X86/X86FastISel.cpp test/CodeGen/X86/tailcall-fastisel.ll

[Eli Friedman]

On Mon, Dec 10, 2012 at 4:18 PM, Chad Rosier <mcrosier at apple.com> wrote:
> Author: mcrosier
> Date: Mon Dec 10 18:18:02 2012
> New Revision: 169796
>
> URL: http://llvm.org/viewvc/llvm-project?rev=169796&view=rev
> Log:
> Fall back to the selection dag isel to select tail calls.
>
> This shouldn't affect codegen for -O0 compiles as tail call markers are not
> emitted in unoptimized compiles.  Testing with the external/internal nightly
> test suite reveals no change in compile time performance.  Testing with -O1,
> -O2 and -O3 with fast-isel enabled did not cause any compile-time or
> execution-time failures.  All tests were performed on my x86 machine.
> I'll monitor our arm testers to ensure no regressions occur there.
>
> In an upcoming clang patch I will be marking the objc_autoreleaseReturnValue
> and objc_retainAutoreleaseReturnValue as tail calls unconditionally.  While
> it's theoretically true that this is just an optimization, it's an
> optimization that we very much want to happen even at -O0, or else ARC
> applications become substantially harder to debug.
>
> Part of rdar://12553082
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/FastISel.h
>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>     llvm/trunk/lib/Target/ARM/ARMFastISel.cpp
>     llvm/trunk/lib/Target/X86/X86FastISel.cpp
>     llvm/trunk/test/CodeGen/X86/tailcall-fastisel.ll
>
> Modified: llvm/trunk/include/llvm/CodeGen/FastISel.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/FastISel.h?rev=169796&r1=169795&r2=169796&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/FastISel.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/FastISel.h Mon Dec 10 18:18:02 2012
> @@ -131,6 +131,10 @@
>    /// into the current block.
>    void recomputeInsertPt();
>
> +  /// removeDeadCode - Remove all dead instructions between the I and E.
> +  void removeDeadCode(MachineBasicBlock::iterator I,
> +                      MachineBasicBlock::iterator E);
> +
>    struct SavePoint {
>      MachineBasicBlock::iterator InsertPt;
>      DebugLoc DL;
> @@ -395,10 +399,6 @@
>
>    /// hasTrivialKill - Test whether the given value has exactly one use.
>    bool hasTrivialKill(const Value *V) const;
> -
> -  /// removeDeadCode - Remove all dead instructions between the I and E.
> -  void removeDeadCode(MachineBasicBlock::iterator I,
> -                      MachineBasicBlock::iterator E);
>  };
>
>  }
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=169796&r1=169795&r2=169796&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Mon Dec 10 18:18:02 2012
> @@ -5273,11 +5273,6 @@
>        !isInTailCallPosition(CS, CS.getAttributes().getRetAttributes(), TLI))
>      isTailCall = false;
>
> -  // If there's a possibility that fast-isel has already selected some amount
> -  // of the current basic block, don't emit a tail call.
> -  if (isTailCall && TM.Options.EnableFastISel)
> -    isTailCall = false;
> -
>    TargetLowering::
>    CallLoweringInfo CLI(getRoot(), RetTy, FTy, isTailCall, Callee, Args, DAG,
>                         getCurDebugLoc(), CS);
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=169796&r1=169795&r2=169796&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon Dec 10 18:18:02 2012
> @@ -1113,19 +1113,21 @@
>            }
>
>            bool HadTailCall = false;
> +          MachineBasicBlock::iterator SavedInsertPt = FuncInfo->InsertPt;
>            SelectBasicBlock(Inst, BI, HadTailCall);
>
> -          // Recompute NumFastIselRemaining as Selection DAG instruction
> -          // selection may have handled the call, input args, etc.
> -          unsigned RemainingNow = std::distance(Begin, BI);
> -          NumFastIselFailures += NumFastIselRemaining - RemainingNow;
> -
>            // If the call was emitted as a tail call, we're done with the block.
> +          // We also need to delete any previously emitted instructions.
>            if (HadTailCall) {
> +            FastIS->removeDeadCode(SavedInsertPt, FuncInfo->MBB->end());
>              --BI;
>              break;
>            }
>
> +          // Recompute NumFastIselRemaining as Selection DAG instruction
> +          // selection may have handled the call, input args, etc.
> +          unsigned RemainingNow = std::distance(Begin, BI);
> +          NumFastIselFailures += NumFastIselRemaining - RemainingNow;
>            NumFastIselRemaining = RemainingNow;
>            continue;
>          }
>
> Modified: llvm/trunk/lib/Target/ARM/ARMFastISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMFastISel.cpp?rev=169796&r1=169795&r2=169796&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMFastISel.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMFastISel.cpp Mon Dec 10 18:18:02 2012
> @@ -2284,6 +2284,9 @@
>    // Can't handle inline asm.
>    if (isa<InlineAsm>(Callee)) return false;
>
> +  // Allow SelectionDAG isel to handle tail calls.
> +  if (CI->isTailCall()) return false;

I'll just note that "isTailCall" doesn't precisely match the meaning
of "tail call" you want; it just means "that the callee function does
not access any allocas or varargs in the caller"
(http://llvm.org/docs/LangRef.html#call-instruction).  I mean, this
will probably work in practice, but you're abusing the semantics a
bit.

-Eli