This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Cortex-R82: remove crypto
ClosedPublic

Authored by SjoerdMeijer on Nov 23 2020, 1:06 PM.

Details

Summary

Remove target features crypto for Cortex-R82, because it doesn't have any, and add LSE which was missing.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Nov 23 2020, 1:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2020, 1:06 PM
SjoerdMeijer requested review of this revision.Nov 23 2020, 1:06 PM
MarkMurrayARM accepted this revision.Nov 30 2020, 1:09 AM

Looks OK to me, given releated upstreaming work I've done recently.

This revision is now accepted and ready to land.Nov 30 2020, 1:09 AM
chill added a subscriber: chill.Nov 30 2020, 1:30 AM

In the Arm Architecture Reference Manual Supplement - Armv8, for Armv8-R AArch64 architecture profile
"A1.4 Architecture extensions" lists among others

  • FEAT_SHA1 Advanced SIMD SHA1 instructions
  • FEAT_SHA256 Advanced SIMD SHA256 instructions
  • FEAT_AES Advanced SIMD AES instructions
  • FEAT_PMULL Advanced SIMD PMULL instructions

Is there a document, which says R82 does not implement those ?

chill edited reviewers, added: chill; removed: momchil.velikov.Nov 30 2020, 1:31 AM

In the Arm Architecture Reference Manual Supplement - Armv8, for Armv8-R AArch64 architecture profile
"A1.4 Architecture extensions" lists among others

  • FEAT_SHA1 Advanced SIMD SHA1 instructions
  • FEAT_SHA256 Advanced SIMD SHA256 instructions
  • FEAT_AES Advanced SIMD AES instructions
  • FEAT_PMULL Advanced SIMD PMULL instructions

Is there a document, which says R82 does not implement those ?

Yeah, it's architecturally supported, just not implemented in this core.
The best I can do is point you at a non-public doc offline (which I will do).

chill added a comment.Nov 30 2020, 1:44 AM

In the Arm Architecture Reference Manual Supplement - Armv8, for Armv8-R AArch64 architecture profile
"A1.4 Architecture extensions" lists among others

  • FEAT_SHA1 Advanced SIMD SHA1 instructions
  • FEAT_SHA256 Advanced SIMD SHA256 instructions
  • FEAT_AES Advanced SIMD AES instructions
  • FEAT_PMULL Advanced SIMD PMULL instructions

Is there a document, which says R82 does not implement those ?

... and on the next page

  • FEAT_SHA3 Advanced SIMD SHA3 instructions
  • FEAT_SHA512 Advanced SIMD SHA512 instructions
  • FEAT_SM3 Advanced SIMD SM3 instructions
  • FEAT_SM4 Advanced SIMD SM4 instructions
chill added inline comments.Nov 30 2020, 1:47 AM
llvm/include/llvm/Support/AArch64TargetParser.def
56

This change here modifies the architecture option. If a core does not completely implement an architecture, nevertheless
the -march=that-architecture option should be not be affected.

56

s/should be not be /should not be/

SjoerdMeijer added inline comments.Nov 30 2020, 1:55 AM
llvm/include/llvm/Support/AArch64TargetParser.def
56

Ah yes, this indeed doesn't look right. Will address this.

The short story is that this was "accidentally" getting things right.

The long story story, is that after discussing this offline, we want to this to be more aligned with GCC which enables a more minimal set of architecture extensions. This means that we don't always want to enable optional extensions, like crypto in this case. Additionally, none of the R-profile cores implement crypto, so enabling it might not be the best default for R-profile.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2020, 4:57 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript