Page MenuHomePhabricator

[AArch64][SVE] Add support for spilling/filling ZPR2/3/4
ClosedPublic

Authored by c-rhodes on Mar 11 2020, 6:43 AM.

Details

Summary

This patch enables the register allocator to spill/fill lists of 2, 3
and 4 SVE vectors registers to/from the stack. This is implemented with
pseudo instructions that get expanded to individual LDR_ZXI/STR_ZXI
instructions in AArch64ExpandPseudoInsts.

Patch by Sander de Smalen.

Diff Detail

Event Timeline

c-rhodes created this revision.Mar 11 2020, 6:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2020, 6:43 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
3111–3112

Nit: Should these be the None/default case of the switch above?

Seems wasteful to keep searching if we've already found our opcode in the switch.

c-rhodes added inline comments.Mar 12 2020, 3:04 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
3111–3112

That's seems like a sensible suggestion, it would be nicer if this was handled in the existing switch. I noticed a comment from @efriedma [1] on Sander's patch adding the same functionality for ZPR/PPR classes, where he mentions not depending on what getSpillSize returns. @efriedma do you recall why this was?

I've had a brief chat with Sander about this and as far as I'm aware we haven't formally decided on how the spill size is represented for scalable vectors, currently it's 128 for ZPR register class and 16 for PPR with an implicit vscale multiplier. Perhaps these could be moved to the relevant cases in that switch, i.e. 2 for PPR, 48 for ZPR3 etc.

[1] https://reviews.llvm.org/D70082?id=228687#inline-631144

efriedma added inline comments.Mar 12 2020, 1:44 PM
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
3111–3112

I think my concern was just that it's not clear what the return value of getSpillSize() means, if it's not the size in bytes of the required spill slot.

c-rhodes updated this revision to Diff 264202.Fri, May 15, 4:11 AM
c-rhodes edited the summary of this revision. (Show Details)

Use getSpillSize switch in storeRegToStackSlot/loadRegFromStackSlot for SVE predicate and vector registers.

c-rhodes marked an inline comment as done.Fri, May 15, 4:15 AM
c-rhodes added inline comments.
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
3111–3112

I've implemented @cameron.mcinally suggestion, I think we can be confident about what getSpillSize means as the spill size is explicitly set in bits for SVE registers, e.g.

def ZPR3  : RegisterClass<"AArch64", [untyped], 128, (add ZSeqTriples)> {
  let Size = 384;
}

and getSpillSize returns this in bytes.

This revision is now accepted and ready to land.Tue, May 26, 10:15 AM
This revision was automatically updated to reflect the committed changes.