This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Allocate emergency spillslot when using fixed-length SVE.
AbandonedPublic

Authored by sdesmalen on Mar 7 2023, 5:42 AM.

Details

Summary

The SVE reg+imm addressing modes require the immediate to be a multiple
of VL. When it is not a multiple of VL, it can use 0 for the immediate
and use an add instruction to materialise the base-register.

When using SVE instructions to access fixed-length vectors on the stack
that are not VL-aligned, the frame should anticipate that it might require
an emergency spill-slot to accomodate for an extra 'add' to materialize
the base-address of the load when there are no free registers available.

Fixes https://github.com/llvm/llvm-project/issues/60918

Diff Detail

Unit TestsFailed

Event Timeline

sdesmalen created this revision.Mar 7 2023, 5:42 AM
sdesmalen requested review of this revision.Mar 7 2023, 5:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 7 2023, 5:42 AM
peterwaller-arm accepted this revision.Mar 7 2023, 7:21 AM

Makes sense to me, one minor suggestion. I note that this introduces a few loads/stores to the stack for fixed length vectorization, but having it work correctly is the priority, those may be possible to remove later.

llvm/test/CodeGen/AArch64/framelayout-sve-scavengingslot.mir
30

Since this test lacks check lines I presume it is meant to check only that llc exits successfully. Could you have checklines or a comment explaining what failure it is meant to catch?

This revision is now accepted and ready to land.Mar 7 2023, 7:21 AM
sdesmalen updated this revision to Diff 503041.Mar 7 2023, 7:45 AM

Added missing CHECK lines

sdesmalen added inline comments.Mar 7 2023, 7:46 AM
llvm/test/CodeGen/AArch64/framelayout-sve-scavengingslot.mir
30

Sorry I forgot to put in the CHECK lines. Thanks for catching this!

paulwalker-arm added inline comments.Mar 7 2023, 10:06 AM
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
2979

Is this initialisation required?

2985–2986

This seems a bit "belt and braces". You sure there's no other solution?

2992

Is the StackID of this frame index also relevant? As in, you only care about accesses to the non-scalable stack region?

Does using an SVE instruction to access a fixed stack slot ever work? I mean, I guess if you get lucky, the fixed offset is zero, but that should happen rarely. Maybe we'd get better code if we expand out the arithmetic earlier. (Doesn't need to block this patch, just something to think about.)

Matt added a subscriber: Matt.Mar 7 2023, 11:38 AM
sdesmalen abandoned this revision.Mar 16 2023, 7:02 AM

Abandoning since the alternative fix has now landed (D146056)