This is an archive of the discontinued LLVM Phabricator instance.

Update generic scheduling to use A510 scheduling model
ClosedPublic

Authored by harviniriawan on Aug 1 2023, 7:47 AM.

Details

Summary

Refresh of the generic scheduling model to use A510 instead of A55.
Main benefits are to the little core, and introducing SVE scheduling information.
Changes tested on various OoO cores, no performance degradation is seen.

Diff Detail

Event Timeline

harviniriawan created this revision.Aug 1 2023, 7:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 7:47 AM
harviniriawan requested review of this revision.Aug 1 2023, 7:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 7:47 AM
Matt added a subscriber: Matt.Aug 1 2023, 2:28 PM

Sounds great. The change certainly sounds like a good idea.

There are a lot of test updates needed though, and some of them might be better not-auto-generated, or could otherwise do with a bit of a cleanup. I remember when changing to the A55 scheduling model that I just manually updated some of the tests, it might be better to do the same here. I haven't gone through everything, but have marked some of the tests that could do with some cleanup.

llvm/test/CodeGen/AArch64/128bit_load_store.ll
1

I'm not sure why this needed updating, but the check lines OK here.

llvm/test/CodeGen/AArch64/2s-complement-asm.ll
1

This one isn't needed, and seems to not have updated the actual tests.

llvm/test/CodeGen/AArch64/GlobalISel/swifterror.ll
1

I think this is one that is best left manually updated.

llvm/test/CodeGen/AArch64/aarch64-2014-08-11-MachineCombinerCrash.ll
1

Probably not needed.

llvm/test/CodeGen/AArch64/fptoui-sat-vector.ll
2277

There is an extra spill here, but that kind of things is expected given enough code.

llvm/test/CodeGen/AArch64/logical_shifted_reg.ll
261

This looks like it no longer decides to ifcvt. Given the test that sound fine I think.

llvm/test/CodeGen/AArch64/memcpy-scoped-aa.ll
137–138

Things like this can be removed too, so long as there are check lines elsewhere.

llvm/test/CodeGen/AArch64/misched-detail-resource-booking-01.mir
1

This doesn't look right. Should it be using -mcpu=cortex-a55 instead?

llvm/test/CodeGen/AArch64/misched-fusion-lit.ll
1

This is likely best checked manually.

llvm/test/CodeGen/AArch64/named-vector-shuffles-neon.ll
114

Some of these have less movs thanks to the lower latencies, which is nice.

llvm/test/CodeGen/AArch64/wineh-bti.ll
1

This has test CHECK line elsewhere in the file.

harviniriawan marked 11 inline comments as done.
harviniriawan added inline comments.Aug 2 2023, 9:09 AM
llvm/test/CodeGen/AArch64/misched-detail-resource-booking-01.mir
1

shouldn't it be a510 as it's the default scheduling policy now?

Thanks. If you can clean up these tests, then this looks good to me.

llvm/test/CodeGen/AArch64/aarch64-dynamic-stack-layout.ll
86

This looks like it would be better with the manual check lines below.

llvm/test/CodeGen/AArch64/arm64-ld1.ll
2

This doesn't look like it has generated properly.

llvm/test/CodeGen/AArch64/arm64-promote-const-complex-initializers.ll
12–19

This doesn't look like it is checking everything it did in the past.

llvm/test/CodeGen/AArch64/arm64_32.ll
17–34

This file might be better manually generated. Some of the check lines and comments are deliberately placed.

llvm/test/CodeGen/AArch64/atomic-ops-lse.ll
36

It looks like ths RUN line for CHECK-REG is trying to check that there are no invalid register formed. You might be able to change it to --check-prefixes=CHECK,CHECK-REG

llvm/test/CodeGen/AArch64/bfis-in-loop.ll
13–14

This doesn't look right

llvm/test/CodeGen/AArch64/f16-instructions.ll
13

This one needs to be done manually.

llvm/test/CodeGen/AArch64/fmlal-loreg.ll
3 ↗(On Diff #546739)

I'm not sure why these blank lines were removed? I think they can be kept in.

llvm/test/CodeGen/AArch64/misched-detail-resource-booking-01.mir
29

This file can specify -mcpu=cortex-a55 to keep the test as it was before.

llvm/test/CodeGen/AArch64/nontemporal-load.ll
5

These newlines and blank ; lines look funny.

llvm/test/CodeGen/AArch64/shrink-wrapping-vla.ll
18

Manual update might be better.

llvm/test/CodeGen/AArch64/sme2-intrinsics-min.ll
140 ↗(On Diff #546739)

This doesn't look right.

llvm/test/CodeGen/AArch64/sme2-intrinsics-rshl.ll
53

This doesn't look right.

llvm/test/CodeGen/AArch64/sme2-intrinsics-sqdmulh.ll
54

This doesn't look right.

llvm/test/CodeGen/AArch64/speculation-hardening-loads.ll
5

Manual update might be better.

llvm/test/CodeGen/AArch64/split-vector-insert.ll
13–56

This was testing the debug output before.

llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
1 ↗(On Diff #546739)

Can you explain this one? Are the two versions now too different to keep the same?

llvm/test/CodeGen/AArch64/sve-fixed-length-build-vector.ll
56

Keep this

llvm/test/CodeGen/AArch64/sve-split-int-pred-reduce.ll
25

This is nice :)

llvm/test/CodeGen/AArch64/swift-return.ll
7

Manually update this one may be best.

llvm/test/CodeGen/AArch64/vec-combine-compare-to-bitmask.ll
10–25

These need to keep checking the constant pools

llvm/test/CodeGen/AArch64/vec-combine-compare-truncate-store.ll
5–6

These too.

llvm/test/CodeGen/AArch64/zext-to-tbl.ll
5 ↗(On Diff #546739)

These too.

harviniriawan marked 23 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2023, 8:16 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
harviniriawan added inline comments.Aug 3 2023, 8:16 AM
llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
1 ↗(On Diff #546739)

Yes indeed. the ones with CHECK-ADD annoyingly is scheduled slightly differently due to it being ADD instead of LDR

dmgreen accepted this revision.Aug 3 2023, 1:11 PM

Thanks. Looks good. You may need to rebase over new tests, make sure everything is still passing when you do.

LGTM

llvm/test/CodeGen/AArch64/misched-detail-resource-booking-01.mir
1

Sorry I forgot to reply to the old comment. AFAIU this is testing some of the info in the scheduling model, not which model is being used. So pinning it to the A55 makes sense for keeping the old testing in place.

This revision is now accepted and ready to land.Aug 3 2023, 1:11 PM
harviniriawan marked an inline comment as done.Aug 21 2023, 3:09 AM
harviniriawan closed this revision.Aug 21 2023, 4:27 AM
harviniriawan updated this revision to Diff 551978.
aeubanks added inline comments.
llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
1 ↗(On Diff #551978)

this test is broken, without the split-file the other files won't exist in a clean build, please fix or revert if it takes time to fix