This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add Neoverse V2 CPU support
ClosedPublic

Authored by david-arm on Sep 21 2022, 5:21 AM.

Details

Summary

Adds support for the Neoverse V2 CPU to the AArch64 backend.

Diff Detail

Event Timeline

david-arm created this revision.Sep 21 2022, 5:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 5:21 AM
david-arm requested review of this revision.Sep 21 2022, 5:21 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 21 2022, 5:21 AM
dmgreen added inline comments.Sep 21 2022, 6:34 AM
llvm/lib/Target/AArch64/AArch64.td
989

Please add FeatureFuseAES and FeatureLSLFast.

1167

HasV9_0aOps implies some of these features already.

llvm/lib/Target/AArch64/AArch64Subtarget.cpp
216

This can use the same block as NeoverseN2?

david-arm updated this revision to Diff 461898.Sep 21 2022, 8:08 AM
  • Changed lists of tuning features.
  • Removed redundant arch features from list.
  • Combined neoverse-v2 and neoverse-n2 cases together in AArch64Subtarget.
david-arm marked 3 inline comments as done.Sep 21 2022, 8:09 AM

Thanks for the quick review @dmgreen!

tschuett added a subscriber: tschuett.EditedSep 21 2022, 1:39 PM

VScaleForTuning is 1 for N2 and V2. It is 2 for V1. I though the V2 is more like the V1 than the N2?
Sorry. This is throughput right?

Thanks. Should RNG be added? And Why is Crypto no longer enabled?

Oh, A release note would be good to add too.

david-arm updated this revision to Diff 462124.Sep 22 2022, 3:58 AM
  • Added FEAT_RNG to the neoverse-v2 CPU.
  • Added message to release notes.

VScaleForTuning is 1 for N2 and V2. It is 2 for V1. I though the V2 is more like the V1 than the N2?
Sorry. This is throughput right?

Hi @tschuett, Neoverse V2 will have 128-bit SVE vector lengths, hence VScaleForTuning should be 1.

Matt added a subscriber: Matt.Sep 22 2022, 1:34 PM
Matt added inline comments.Sep 22 2022, 2:51 PM
llvm/lib/Target/AArch64/AArch64.td
1167

Shouldn't FeatureMTE (Enable Memory Tagging Extension) be present, too (as in NeoverseN2)?

david-arm added inline comments.Sep 23 2022, 1:19 AM
llvm/lib/Target/AArch64/AArch64.td
1167

It is already included in the patch, i.e. see above:

FeatureMTE, FeatureRandGen];
dmgreen added inline comments.Sep 23 2022, 1:27 AM
llvm/lib/Target/AArch64/AArch64.td
1167

Oh yeah so it is. Should we add AEK_MTE too then?

llvm/unittests/Support/TargetParserTest.cpp
1033

RAS is here twice.

tschuett added inline comments.Sep 23 2022, 1:29 AM
llvm/include/llvm/Support/AArch64TargetParser.def
238

MTE is implied by ARMV9A or missing?

david-arm updated this revision to Diff 462420.Sep 23 2022, 1:53 AM
  • Added AEK_MTE to target parser flags.
david-arm marked 3 inline comments as done.Sep 23 2022, 1:54 AM
dmgreen accepted this revision.Sep 23 2022, 1:55 AM

Thanks. LGTM

This revision is now accepted and ready to land.Sep 23 2022, 1:55 AM
Matt added inline comments.Sep 23 2022, 12:34 PM
llvm/include/llvm/Support/AArch64TargetParser.def
239

Should AEK_SVE2BITPERM be present? (Noticed that N2 has AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM).

llvm/lib/Target/AArch64/AArch64.td
1166

FeatureNEON may be redundant (note that it's in HasV8_3aOps).

OTOH, NeoverseV1 also has FeatureCrypto: is this no longer the case for NeoverseV2?

llvm/lib/Target/AArch64/AArch64Subtarget.cpp
204

Are CacheLineSize (= 0 by default) and MaxInterleaveFactor (= 2 by default) the same / correct for both N2 and V2?

david-arm updated this revision to Diff 462833.Sep 26 2022, 1:05 AM
  • Added SVE2BITPERM to AArch64TargetParser.def and updated the unit test.
david-arm marked 3 inline comments as done.Sep 26 2022, 1:20 AM
david-arm added inline comments.
llvm/include/llvm/Support/AArch64TargetParser.def
239

Good spot! I've made this consistent with the definition in AArch64.td.

llvm/lib/Target/AArch64/AArch64.td
1166

HasV8_3aOps does imply FeatureNEON, but only indirectly through FeatureComplxNum. I thought it was clearer to explicitly add it, since it doesn't do any harm.

With regards to FeatureCrypto, I am following the precedent set for Cortex-A510, Cortex-A710 and Cortex-X2 where it also wasn't enabled by default. The user can always enable crypto with -mcpu=neoverse-v2+crypto if required.

llvm/lib/Target/AArch64/AArch64Subtarget.cpp
204

I don't know the answer for neoverse-v2 I'm afraid, and neoverse-n2 isn't part of this patch. Performance tuning can be done in a later patch I think.

Matt added a comment.Sep 26 2022, 8:49 AM

OK, thanks!

paulwalker-arm accepted this revision.Sep 26 2022, 3:57 PM
paulwalker-arm added inline comments.
clang/docs/ReleaseNotes.rst
407

support for Neoverse V2 via the flag?

This revision was landed with ongoing or failed builds.Sep 27 2022, 12:56 AM
This revision was automatically updated to reflect the committed changes.
david-arm marked 3 inline comments as done.
david-arm marked an inline comment as done.Sep 27 2022, 12:57 AM