This is an archive of the discontinued LLVM Phabricator instance.

ARM, AArch64, X86: Check preserved registers for tail calls.
ClosedPublic

Authored by MatzeB on Mar 31 2016, 7:53 PM.

Details

Summary

We can only perform a tail call to a callee that preserves all the
registers that the caller needs to preserve.

This situation happens with calling conventions like preserver_mostcc or
cxx_fast_tls. It was explicitely handled for the fast_tls case and failing for preserve_most. This patch generalizes
the check to any calling convention.

Related to rdar://24207743

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 52323.Mar 31 2016, 7:53 PM
MatzeB retitled this revision from to ARM, AArch64, X86: Check preserved registers for tail calls..
MatzeB updated this object.
MatzeB added reviewers: t.p.northover, qcolombet.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added subscribers: llvm-commits, manmanren.
qcolombet added inline comments.Apr 1 2016, 10:07 AM
lib/CodeGen/TargetRegisterInfo.cpp
397

No braces.

lib/Target/AArch64/AArch64ISelLowering.cpp
2882

Isn’t sufficient to be inclusive?
I.e., CallerCC C= CalleeCC

lib/Target/ARM/ARMISelLowering.cpp
2151

Ditto

test/CodeGen/AArch64/tailcall-ccmismatch.ll
6

Add a comment on what you are testing here.

test/CodeGen/ARM/cxx-tlscc.ll
130

Ditto

MatzeB updated this revision to Diff 52453.Apr 1 2016, 9:15 PM

Thanks for the review! Addressed the comments as discussed.

MatzeB updated this revision to Diff 52454.Apr 1 2016, 9:16 PM

upload the correct patch this time.

Overall, I think the change makes sense in that it should keep the original intention intact and make it more generic and not less safe (but I'm not an expert).

lib/CodeGen/TargetRegisterInfo.cpp
395

IIUC, you need to check that all reserved registers on the left are also on the right, but not necessarily all on the right to be on the left, right? In that case, this check would be too restrictive, mostly matching recursive tail calls only, no?

MatzeB added inline comments.Apr 4 2016, 11:11 AM
lib/CodeGen/TargetRegisterInfo.cpp
395

That's what the check is doing, isn't it?

Bit0Bit1Bit0&Bit1 == Bit0
00/11
100
111

Is there a clearer way to write the code?

rengolin added inline comments.Apr 4 2016, 11:41 AM
lib/CodeGen/TargetRegisterInfo.cpp
395

Dang! It is, sorry, brain fart.

qcolombet accepted this revision.Apr 4 2016, 11:46 AM
qcolombet edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Apr 4 2016, 11:46 AM
This revision was automatically updated to reflect the committed changes.