Page MenuHomePhabricator

[Clang][ARM][AArch64] Add support for Armv9-A, Armv9.1-A and Armv9.2-A
ClosedPublic

Authored by vhscampos on Sep 9 2021, 9:01 AM.

Details

Summary

armv9-a, armv9.1-a and armv9.2-a can be targeted using the -march option
both in ARM and AArch64.

  • Armv9-A maps to Armv8.5-A.
  • Armv9.1-A maps to Armv8.6-A.
  • Armv9.2-A maps to Armv8.7-A.
  • The SVE2 extension is enabled by default on these architectures.
  • The cryptographic extensions are disabled by default on these architectures.

The Armv9-A architecture is described in the Arm® Architecture Reference
Manual Supplement Armv9, for Armv9-A architecture profile
(https://developer.arm.com/documentation/ddi0608/latest).

Diff Detail

Event Timeline

vhscampos created this revision.Sep 9 2021, 9:01 AM
vhscampos requested review of this revision.Sep 9 2021, 9:01 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 9 2021, 9:01 AM

Some first comments after just looking at the plumbing for these new options. Didn't check yet the architecture extensions for the different version.

clang/lib/Driver/ToolChains/Arch/AArch64.cpp
81–82

This comment needs to be updated for v9a?

413–425

How about +v9a?

clang/test/Preprocessor/aarch64-target-features.c
179–180

Comment out of date?

llvm/lib/Target/AArch64/AArch64.td
471

Is this an unrelated change? Can this be done separately?

llvm/unittests/Support/TargetParserTest.cpp
497

I haven't looked, but in these target parser tests, do we also not need to check the architecture descriptions?
Copied this for example from the target parser def file:

(ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT | ARM::AEK_HWDIVARM |
 ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
 ARM::AEK_DOTPROD)
vhscampos updated this revision to Diff 372473.Sep 14 2021, 6:43 AM
vhscampos marked 3 inline comments as done.
  1. Enable the SVE2 extension as default.
  2. Remove out of date comments in tests.
  3. Remove unrelated change.
vhscampos added inline comments.Sep 14 2021, 6:44 AM
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
413–425

Since v9a maps to v8.5a, and the latter was skipped in the original code, I did not include v9a here.

llvm/unittests/Support/TargetParserTest.cpp
497

If I understand it correctly, we only check architecture extensions for CPUs, not for the architectures themselves.

vhscampos edited the summary of this revision. (Show Details)Sep 16 2021, 6:26 AM
SjoerdMeijer accepted this revision.Sep 17 2021, 12:23 AM

Thanks, looks reasonable to me.

llvm/unittests/Support/TargetParserTest.cpp
497

Ah yeah, I confused it with that.

This revision is now accepted and ready to land.Sep 17 2021, 12:23 AM
vhscampos updated this revision to Diff 373943.Sep 21 2021, 8:22 AM
  1. Fix bug in "+sve2" feature position in the target features list. It was being inserted at the end, which made it impossible to disable it using +nosve2, as the positive option would always be placed after the negative one.
  2. Add tests to the bug fix above.
vhscampos requested review of this revision.Sep 21 2021, 8:24 AM

Sorry @SjoerdMeijer , I found a bug in the implementation (as described in the latest comment). Therefore I kindly ask another round of review, please.

SjoerdMeijer accepted this revision.Sep 22 2021, 12:30 AM

LGTM

clang/lib/Driver/ToolChains/Arch/AArch64.cpp
419

Nit: some .s missing at the end of this sentence and the next below.

This revision is now accepted and ready to land.Sep 22 2021, 12:30 AM
vhscampos updated this revision to Diff 374162.Sep 22 2021, 2:14 AM

Add missing . to end of sentences in comments.

vhscampos marked an inline comment as done.Sep 22 2021, 2:15 AM
vhscampos updated this revision to Diff 376228.Thu, Sep 30, 8:08 AM
vhscampos edited the summary of this revision. (Show Details)
  1. Disable the cryptographic extensions by default.
  2. Small fix in TargetParserTest.cpp to include different spellings of the -march values.

To the relevant persons I have just added to the review: @srhines @nickdesaulniers @llozano

The cryptographic extensions will NOT be enabled by default on Armv9-A and on Armv9-A ARM CPUs.

Matt added a subscriber: Matt.Tue, Oct 5, 12:59 PM
This revision was landed with ongoing or failed builds.Mon, Oct 11, 9:45 AM
This revision was automatically updated to reflect the committed changes.