This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Implement tail calls
ClosedPublic

Authored by arsenm on Mar 21 2021, 9:59 AM.

Details

Summary

Or at least the sibling call cases which the DAG already handles.

Diff Detail

Event Timeline

arsenm created this revision.Mar 21 2021, 9:59 AM
arsenm requested review of this revision.Mar 21 2021, 9:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2021, 9:59 AM
Herald added a subscriber: wdng. · View Herald Transcript

It would be nice if we can put the common code with AArch64 into some generic place instead of duplicating it.
Other than that, looks good.

llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
960–963

Checking the register mask seems pretty cheap, while checking argument assignment looks more expensive.
Does it make sense to do this check first? That should bail out faster if the calling conventions don’t match.

1217

Can this use getStackAlignment() instead of hardcoding 16?

1229

Same about getStackAlignment()

llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-sibling-call.ll
372–374

The parent does not use byval. Is this comment correct?

arsenm updated this revision to Diff 343261.May 5 2021, 6:25 PM

Call common function, rebase on top of patch to fix split arguments

I left a nit inline. Apart from that, LGTM.

llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
1165–1167

Can this and the assert below use getStackAlignment() instead of hardcoding 16?
Or does this have a different meaning?

arsenm updated this revision to Diff 344943.May 12 2021, 1:31 PM
arsenm marked an inline comment as done.

Don't hardcode stack alignment

Flakebi accepted this revision.May 13 2021, 2:27 AM
Flakebi added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
1177

The assert should also use getStackAlignment().

This revision is now accepted and ready to land.May 13 2021, 2:27 AM