This is an archive of the discontinued LLVM Phabricator instance.

[SVE] Don't use LocalStackAllocation for SVE objects
ClosedPublic

Authored by david-arm on Jul 15 2020, 1:12 AM.

Details

Summary

[SVE] Don't use LocalStackAllocation for SVE objects

I have introduced a new TargetFrameLowering query function:

isStackIdSafeForLocalArea

that queries whether or not it is safe for objects of a given stack
id to be bundled into the local area. The default behaviour is to
always bundle regardless of the stack id, however for AArch64 this is
overriden so that it's only safe for fixed-size stack objects.
There is future work here to extend this algorithm for multiple local
areas so that SVE stack objects can be bundled together and accessed
from their own virtual base-pointer.

Diff Detail

Event Timeline

david-arm created this revision.Jul 15 2020, 1:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
david-arm updated this revision to Diff 278109.Jul 15 2020, 1:49 AM
paulwalker-arm added inline comments.Jul 15 2020, 4:33 AM
llvm/lib/Target/AArch64/AArch64FrameLowering.h
107

To me the name implies something that is safe either way, whereas you're making the change to fix a bug. Would isStackIdSafeForLocalArea be a better fit?

llvm/test/CodeGen/AArch64/sve-localstackalloc.mir
2

I forget the exact requirement but does the use of -debug-only mean this tests needs

REQUIRES: Asserts

or something. It is not possible to test the expected frame/stack layout instead, rather the rely on debug?

david-arm updated this revision to Diff 278514.Jul 16 2020, 9:14 AM
david-arm edited the summary of this revision. (Show Details)

When generating the test mir file did you use -simplify-mir. If not then I recommend regenerating it to remove redundant fields and thus have a simpler test input.

llvm/test/CodeGen/AArch64/sve-localstackalloc.mir
2

You can use -run-only= here instead of the start/stop combo.

paulwalker-arm accepted this revision.Jul 17 2020, 2:08 AM
paulwalker-arm added inline comments.
llvm/test/CodeGen/AArch64/sve-localstackalloc.mir
19

Perhaps worth adding a CHECK for the name line, just in case somebody adds additional tests to this file.

This revision is now accepted and ready to land.Jul 17 2020, 2:08 AM
This revision was automatically updated to reflect the committed changes.