This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add support for the Cortex-X3 CPU
ClosedPublic

Authored by vhscampos on Oct 24 2022, 3:16 AM.

Details

Summary

Cortex-X3 is an Armv9-A AArch64 CPU.

This patch introduces support for Cortex-X3.

Technical Reference Manual: https://developer.arm.com/documentation/101593/latest

Diff Detail

Event Timeline

vhscampos created this revision.Oct 24 2022, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2022, 3:16 AM
vhscampos requested review of this revision.Oct 24 2022, 3:16 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 24 2022, 3:16 AM
dmgreen added a subscriber: dmgreen.
dmgreen added inline comments.
clang/docs/ReleaseNotes.rst
473

This should be in the "Arm and AArch64 Support in Clang" section.

llvm/docs/ReleaseNotes.rst
105

Can you add a reference to neoverse-v2 too.

llvm/include/llvm/Support/AArch64TargetParser.def
228

Remove this space
I believe Crypto should be removed.

llvm/lib/Target/AArch64/AArch64.td
805

Please add FeatureFuseAdrpAdd and FeatureLSLFast.

Can you also add a test to clang/test/Driver/aarch64-mcpu.c and the right code to llvm/lib/Support/Host.cpp if you can find it.

vhscampos updated this revision to Diff 470449.Oct 25 2022, 5:36 AM
vhscampos marked 2 inline comments as done.

Comments addressed

llvm/docs/ReleaseNotes.rst
105

I could do this in another patch, but I prefer to restrict this one to X3.

vhscampos added inline comments.Oct 25 2022, 5:37 AM
llvm/docs/ReleaseNotes.rst
105

I meant I prefer to restrict this patch to X3 only.

vhscampos planned changes to this revision.Oct 27 2022, 3:31 AM

We found an issue in the list of target features for v9-A. This patch will be affected.

dmgreen added inline comments.Oct 28 2022, 9:56 AM
clang/docs/ReleaseNotes.rst
746–752

Please combine these two points together

llvm/lib/Target/AArch64/AArch64.td
1274

Oh - this can use NeoverseN2Model.

vhscampos updated this revision to Diff 472001.Oct 31 2022, 7:33 AM

Combining release notes of Cortex-X3 and Neoverse V2.

Make cortex-x3 use neoverse-v2 model.

vhscampos marked 2 inline comments as done.Oct 31 2022, 7:33 AM

Thanks. What was wrong with the v9-A features?

llvm/unittests/Support/TargetParserTest.cpp
1096

Should SSBS be included in here? As far as I understand it is enabled by default in 8.5-A. That should mean it's included automatically, but that can be said for a lot of the other features here too.
Same for AEK_FLAGM, which I noticed as a difference between A715 and X3.

tschuett added inline comments.
clang/docs/ReleaseNotes.rst
748–751

What happened with native detection?

dmgreen added inline comments.
clang/docs/ReleaseNotes.rst
748–751

I think I prefer the new wording, FWIW, as we add more CPUs. The native detection should just be automatically implied.

tschuett added inline comments.Nov 1 2022, 7:02 AM
clang/docs/ReleaseNotes.rst
748–751

But the word native disappeared.

vhscampos updated this revision to Diff 472567.Nov 2 2022, 3:56 AM

Added AEK_FLAM to the list of features for Cortex-X3

vhscampos marked an inline comment as done.Nov 2 2022, 4:06 AM
vhscampos added inline comments.
clang/docs/ReleaseNotes.rst
748–751

I've looked at https://reviews.llvm.org/D134352 and also in code related to -mcpu=native implementation and I couldn't find anything related to Neoverse V2. Can you please point me to where that release note relates to?

llvm/unittests/Support/TargetParserTest.cpp
1096

SSBS is an optional feature for all Armv8-A architectures.

But FlagM should be present here as it's mandatory in v8.4-A. Fixed.

vhscampos marked an inline comment as done.Nov 2 2022, 4:13 AM
dmgreen added inline comments.Nov 2 2022, 4:32 AM
clang/docs/ReleaseNotes.rst
748–751

I think the comment just meant that -mcpu=native on a neoverse-v2 would correctly turn into -mcpu=neoverse-v2 because the code added to Host.cpp. That should pretty much always be true, so I'm not sure it deserves to be called out.
It sounds fine either way though, if you do think it is important.

llvm/unittests/Support/TargetParserTest.cpp
1096

But if the CPU has SSBS, should it not be present? It seems to be mentioned in the TRM.

According to the reference I have it became mandatory in 8.5-A, so it should be enabled automatically and I'm not sure adding the AEK feature will have much effect. It sounds better to add it for consistency though, if we can.

vhscampos updated this revision to Diff 472583.Nov 2 2022, 5:10 AM

Added SSBS

vhscampos marked an inline comment as done.Nov 2 2022, 5:11 AM
dmgreen accepted this revision.Nov 2 2022, 5:36 AM

Thanks. LGTM if there are not other comments

This revision is now accepted and ready to land.Nov 2 2022, 5:36 AM
vhscampos added a comment.EditedNov 8 2022, 7:38 AM

@tschuett Do you still want me to put the note about 'native' detection for neoverse-v2 back?

@tschuett Do you still want me to put the note about 'native' detection for neoverse-v2 back?

Up to you. No worries.

vhscampos updated this revision to Diff 474029.Nov 8 2022, 9:23 AM

Rebasing and conflict resolution

This revision was landed with ongoing or failed builds.Nov 9 2022, 3:34 AM
This revision was automatically updated to reflect the committed changes.