This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Make -mcpu=generic schedule for an in-order core
ClosedPublic

Authored by dmgreen on Sep 30 2021, 6:38 AM.

Details

Summary

We would like to start pushing -mcpu=generic towards enabling the set of features that improves performance for some CPUs, without hurting any others. A blend of the performance options hopefully beneficial to all CPUs. The largest part of that is enabling in-order scheduling using the Cortex-A55 schedule model. This is similar to the Arm backend change from eecb353d0e25ba which made -mcpu=generic perform inorder scheduling using the cortex-a8 schedule model.

The idea is that in-order cpu's require the most help in instruction scheduling, whereas out-of-order cpus can for the most part out-of-order schedule around different codegen. Our benchmarking suggests that hypothesis holds. When running on an in-order core this improved performance by 3.8% geomean on a set of DSP workloads, 2% geomean on some other embedded benchmark and betwee 1% and 1.8% on a set of singlecore and multicore workloads, all running on a Cortex-A55 cluster.

On an out-of-order cpu the results are a lot more noisy but show flat performance or an improvement. On the set of DSP and embedded benchmarks, run on a Cortex-A78 there was a very noisy 1% speed improvement. Using the most detailed results I could find, SPEC2006 runs on a Neoverse N1 show a small increase in instruction count (+0.127%), but a decrease in cycle counts (-0.155%, on average). The instruction count is very low noise, the cycle count is more noisy with a 0.15% decrease not being significant. SPEC2k17 shows a small decrease (-0.2%) in instruction count leading to a -0.296% decrease in cycle count. These results are within noise margins but tend to show a small improvement in general.

When specifying an Apple target, clang will set "-target-cpu apple-a7" on the command line, so should not be affected by this change when running from clang. This also doesn't enable more runtime unrolling like -mcpu=cortex-a55 does, only changing the schedule used.

A lot of existing tests have updated (they are unlikely to all show given how Phabricator shows changes). This is a summary of the important differences:

  • Most changes are the same instructions in a different order.
  • Sometimes this leads to very minor inefficiencies, such as requiring an extra mov to move variables into r0/v0 for the return value of a test function.
  • misched-fusion.ll was no longer fusing the pairs of instructions it should, as per D110561.
  • neon-mla-mls.ll now uses "mul; sub" as opposed to "neg; mla" due to the different latencies. This seems fine to me.
  • Some SVE tests do not always remove movprfx where they did before. I haven't looked into why yet.
  • The tests argument-blocks-array-of-struct.ll and arm64-windows-calls.ll produce two LDR where they previously produced an LDP due to store-pair-suppress kicking in.
  • arm64-ldp.ll and arm64-neon-copy.ll are missing pre/postinc on LPD.
  • Some tests such as arm64-neon-mul-div.ll and ragreedy-local-interval-cost.ll have more, less or just different spilling.
  • In aarch64_generated_funcs.ll.generated.expected one part of the function is no longer outlined. Interestingly if I switch this to use any other scheduled even less is outlined.

Some of these are expected to happen, such as differences in outlining or register spilling. There will be places where these result in worse codegen, places where they are better, with the SPEC instruction counts suggesting it is not a decrease overall, on average. The LDP inefficiencies I will look into improving.

Diff Detail

Event Timeline

dmgreen created this revision.Sep 30 2021, 6:38 AM
dmgreen requested review of this revision.Sep 30 2021, 6:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2021, 6:38 AM

This sounds like a good idea to me.
I was wondering if a mail to the dev list would be helpful here as it may impact people that are not added as a reviewer?

Matt added a subscriber: Matt.Sep 30 2021, 7:07 PM
asl added a subscriber: asl.Oct 4 2021, 12:50 AM

This sounds good to me too.

Some of the changes might need checking with the appropriate people, but the change in generic target makes sense.

The more obscure cases (like the speculation one) we could even handle offline, after merge.

For speculation-hardening-sls.ll: the approach taken in the ARM backend version of the similar test is probably a lot more robust, and I'd guess that if the test was adapted to follow the approach there ( see https://github.com/llvm/llvm-project/blob/566690b067c8175314fa657b899c99bccf96821c/llvm/test/CodeGen/ARM/speculation-hardening-sls.ll#L343), the compiler would still use the x16 register.

For speculation-hardening-sls.ll: the approach taken in the ARM backend version of the similar test is probably a lot more robust, and I'd guess that if the test was adapted to follow the approach there ( see https://github.com/llvm/llvm-project/blob/566690b067c8175314fa657b899c99bccf96821c/llvm/test/CodeGen/ARM/speculation-hardening-sls.ll#L343), the compiler would still use the x16 register.

Thanks for the suggestion, I will take a look.

On a side note, it looks like llvm/test/CodeGen/ARM/speculation-hardening-sls.ll doesn't contain many of the check lines I would expect any more, after e497b12a69604b6d691312a30f6b86da4f18f7f8. Is that expected, or should I make a patch to undo that?

On a side note, it looks like llvm/test/CodeGen/ARM/speculation-hardening-sls.ll doesn't contain many of the check lines I would expect any more, after e497b12a69604b6d691312a30f6b86da4f18f7f8. Is that expected, or should I make a patch to undo that?

That's definitely not expected by me. It looks like the test has lost it's most valuable checks.
If you have the time to make a patch to undo losing the most valuable checks there, that'd be great!

That's definitely not expected by me. It looks like the test has lost it's most valuable checks.
If you have the time to make a patch to undo losing the most valuable checks there, that'd be great!

Luckily it looks really easy to update, only a single character has changed in the checked lines since before the checks were regenerated. D111074 resets that back to how it was before.

mnadeem added a subscriber: mnadeem.Oct 4 2021, 5:06 PM

Thanks for raising this on the mailing list. Just copying the link for completeness here:

https://lists.llvm.org/pipermail/llvm-dev/2021-October/153071.html

This LGTM, but please give it a few more days before committing this to give folks a chance to reply.

SjoerdMeijer accepted this revision.Oct 5 2021, 3:35 AM
This revision is now accepted and ready to land.Oct 5 2021, 3:35 AM
dmgreen updated this revision to Diff 377240.Oct 5 2021, 7:54 AM
dmgreen edited the summary of this revision. (Show Details)

Thanks all.

This is just a rebase, now without the speculation-hardening-sls.ll change, with a couple more instruction reorders from new tests.

This revision was landed with ongoing or failed builds.Oct 9 2021, 7:58 AM
This revision was automatically updated to reflect the committed changes.