This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Extend LD1RQ ISel patterns to cover missing addressing modes
ClosedPublic

Authored by MattDevereau on Aug 2 2022, 9:19 AM.

Details

Summary

Add some missing patterns for ld1rq's scalar + scalar addressing mode. Also, adds the scalar + imm and scalar + scalar addressing modes for the patterns added in https://reviews.llvm.org/D130010

Diff Detail

Event Timeline

MattDevereau created this revision.Aug 2 2022, 9:19 AM
MattDevereau requested review of this revision.Aug 2 2022, 9:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2022, 9:19 AM
Matt added a subscriber: Matt.Aug 2 2022, 9:55 AM
paulwalker-arm added inline comments.Aug 5 2022, 4:25 AM
llvm/lib/Target/AArch64/SVEInstrFormats.td
6918 ↗(On Diff #449306)

What side effects are you worried about here?

8600–8613 ↗(On Diff #449306)

Classes like this that only contain isel patterns (i.e. are not used to define instructions) are best kept in AArch64SVEInstrInfo.td, typically next to the place they are used.

Is it possible to merge the classes like how multiclass pred_load is implemented? I think you'll also be able to reuse complex patterns like am_sve_regreg_lsl# rather than explicitly matching against add and shifts.

MattDevereau added inline comments.Aug 5 2022, 4:37 AM
llvm/lib/Target/AArch64/SVEInstrFormats.td
6918 ↗(On Diff #449306)

Checks in N2-sve-instructions.s fail the HasSideEffects check without this. The failing ones have the same scalar for the base and index, i.e
# CHECK-NEXT: 1 6 0.33 * U ld1rqb { z0.b }, p0/z, [x0, x0]
instead outputs
# CHECK-NEXT: 1 6 0.33 * ld1rqb { z0.b }, p0/z, [x0, x0]

Accepting changes to N2-sve-instructions.s as valid

Added class LD1RQPat to match the other set of scalar variant definitions

paulwalker-arm added inline comments.Aug 19 2022, 4:19 AM
llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
2267–2283

Can these be moved into LD1RQPat to mirror the three forms use by sve_ld1rq_pat.

I guess there's also a shout for all the patterns to exist within a single multiclass given they're related but that's up to you.

llvm/test/CodeGen/AArch64/sve-intrinsics-loads.ll
20–21

You shouldn't be regressing existing code. You likely need to wrap the immediate variants with AddedComplexity, see pred_load so the immediate forms are matched first then the register ones only match when necessary.

Multiclassed more LD1RQ patterns into ld1rq_pat
Added AddedComplexity to immediate patterns to prevent scalar + immediate cases regressing to scalar + scalar(mov)

paulwalker-arm accepted this revision.Aug 23 2022, 8:04 AM
This revision is now accepted and ready to land.Aug 23 2022, 8:04 AM
This revision was landed with ongoing or failed builds.Aug 25 2022, 6:07 AM
This revision was automatically updated to reflect the committed changes.