This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen][AArch64] Don't split functions with a red zone on AArch64
ClosedPublic

Authored by dhoekwater on Aug 4 2023, 12:03 PM.

Details

Summary

Because unconditional branch relaxation on AArch64 grows the stack to
spill a register, splitting a function would cause the red zone to be
overwritten. Explicitly disable MFS for such functions.

Diff Detail

Unit TestsFailed

Event Timeline

dhoekwater created this revision.Aug 4 2023, 12:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 12:03 PM
dhoekwater requested review of this revision.Aug 4 2023, 12:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 12:03 PM
This revision was not accepted when it landed; it landed in state Needs Review.Aug 4 2023, 12:30 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 12:30 PM
Herald added subscribers: Restricted Project, Enna1, abrachet, phosek. · View Herald Transcript
leonardchan reopened this revision.Aug 4 2023, 4:55 PM
leonardchan added a subscriber: leonardchan.

Sorry I accidentally closed the revision. I think just re-opening it should be fine then when you commit it, you can still use this differential revision.

I think perhaps you can re-upload your diff to the original one.

dhoekwater updated this revision to Diff 547428.Aug 4 2023, 5:54 PM
dhoekwater added a comment.EditedAug 4 2023, 5:54 PM

I think perhaps you can re-upload your diff to the original one.

All good, that worked.

dhoekwater updated this revision to Diff 547876.
dhoekwater updated this revision to Diff 548779.EditedAug 9 2023, 3:01 PM

Use an LLVM IR test instead of an MIR test since those are easier to read. Removes dependency on https://reviews.llvm.org/D157063.

dhoekwater edited the summary of this revision. (Show Details)Aug 9 2023, 3:01 PM

It would be great to reuse the existing corpus of splitting tests for ARM. They would cover basic functionality along with interactions with different flavours of FDO. Can you create a separate patch to move the existing tests in test/CodeGen/X86/machine-function-splitter.ll to test/CodeGen/Generic/machine-function-splitter.ll? Then we can add the triple and appropriate CHECKs for x86 and ARM.

One precedent I can find is: test/CodeGen/Generic/cfi-sections.ll which has x86 and ARM triples (and REQUIRES the x86 and ARM registered targets).

Rebase onto D157565. Remove a few reviewers since I added too many initially.

Add tests into Generic

Rebase onto main and remove the dependency

dhoekwater edited the summary of this revision. (Show Details)

Update the revision description

mingmingl accepted this revision.Aug 22 2023, 1:51 PM
This revision is now accepted and ready to land.Aug 22 2023, 1:51 PM