Page MenuHomePhabricator

[AArch64][GlobalISel] Teach AArch64CallLowering to handle basic sibling calls
ClosedPublic

Authored by paquette on Wed, Sep 4, 11:12 AM.

Details

Summary

This adds support for basic sibling call lowering in AArch64. The intent here is to only handle tail calls which do not change the ABI (hence, sibling calls.)

At this point, it is very restricted. It does not handle

  • Vararg calls
  • Calls with outgoing arguments
  • Calls whose calling conventions differ from the caller's calling convention
  • Tail/sibling calls with BTI enabled

This patch adds

  • AArch64CallLowering::isEligibleForTailCallOptimization, which is equivalent to the same function in AArch64ISelLowering.cpp (albeit with the restrictions above.)
  • mayTailCallThisCC and canGuaranteeTCO, which are identical to those in AArch64ISelLowering.cpp
  • getCallOpcode, which is exactly what it sounds like

Tail/sibling calls are lowered by checking if they pass target-independent tail call positioning checks, and checking if they satisfy isEligibleForTailCallOptimization. If they do, then a tail call instruction is emitted instead of a normal call. If we have a sibling call (which is always the case in this patch), then we do not emit any stack adjustment operations. When we go to lower a return, we check if we've already emitted a tail call. If so, then we skip the return lowering.

For testing, this patch

  • Adds test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll to test which tail calls we currently lower, which ones we don't, and which ones we shouldn't.
  • Updates test/CodeGen/AArch64/branch-target-enforcement-indirect-calls.ll to show that we fall back as expected.

Also updates test/CodeGen/AArch64/dllimport.ll, which now generates a tail call instead of a normal call (matching the other instruction selectors.)

Diff Detail

Event Timeline

paquette created this revision.Wed, Sep 4, 11:12 AM
thegameg added inline comments.
llvm/lib/Target/AArch64/AArch64CallLowering.cpp
563

Is this worth a remark?

paquette marked an inline comment as done.Wed, Sep 4, 11:40 AM
paquette added inline comments.
llvm/lib/Target/AArch64/AArch64CallLowering.cpp
563

Yeah, that's a good idea!

aemerson accepted this revision.Wed, Sep 4, 3:28 PM

Looks reasonable to me as a starting point.

llvm/lib/Target/AArch64/AArch64CallLowering.cpp
440

This bool really necessary?

This revision is now accepted and ready to land.Wed, Sep 4, 3:28 PM
paquette marked an inline comment as done.Wed, Sep 4, 3:34 PM
paquette added inline comments.
llvm/lib/Target/AArch64/AArch64CallLowering.cpp
440

Not right now, but there are a couple places it will need to be checked when the rest of the tail call stuff is implemented.

I'm going to commit this as-is and put up a separate review for remarks.

I figured I'd add them for all of the failure cases, and the success cases. (Basically turn the debug output into remark output.) That makes for a large-ish patch, so I'd like to do it separately.

I'm going to commit this as-is and put up a separate review for remarks.

I figured I'd add them for all of the failure cases, and the success cases. (Basically turn the debug output into remark output.) That makes for a large-ish patch, so I'd like to do it separately.

Sounds good to me.

This revision was automatically updated to reflect the committed changes.
RKSimon added a subscriber: RKSimon.Thu, Sep 5, 3:42 AM

@paquette I'm sorry but I had to revert rL370996 at rL371051 to fix the EXPENSIVE_CHECKS builds: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19511/

Is it possible for you to enable EXPENSIVE_CHECKS in all your local builds?

@RKSimon Sorry about that. I'll enable EXPENSIVE_CHECKS.

Fixed the issue. Issue was that I never tested returning something other than void. I disabled tail calling calls with non-void return types, and updated the test to reflect that.

Recommitted in r371114/0add2a8a5fd

https://reviews.llvm.org/rL371114