Page MenuHomePhabricator

[AArch64] Add NVIDIA Carmel support
ClosedPublic

Authored by tambre on Apr 11 2020, 4:23 AM.

Details

Summary

NVIDIA's Carmel ARM64 cores are used in Tegra194 chips found in Jetson AGX Xavier, DRIVE AGX Xavier and DRIVE AGX Pegasus.

References:

Diff Detail

Event Timeline

tambre created this revision.Apr 11 2020, 4:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2020, 4:23 AM
tambre updated this revision to Diff 256763.Apr 11 2020, 4:34 AM

Remove cacheline size

tambre edited the summary of this revision. (Show Details)Apr 11 2020, 4:35 AM
tambre updated this revision to Diff 256764.Apr 11 2020, 5:37 AM

Fix formatting

sdesmalen added inline comments.Apr 14 2020, 3:01 PM
llvm/lib/Target/AArch64/AArch64.td
615

Not sure how accurate the link you referenced is, but it doesn't mention SVE or Crypto extensions. Is that an omission in the blog post?

llvm/unittests/Support/Host.cpp
271

nit: it probably doesn't matter much for the function getHostCPUNameForARM, but the feature list seems incomplete.

llvm/unittests/Support/TargetParserTest.cpp
981

nit: odd indentation here, have you used clang-format?

tambre updated this revision to Diff 258045.Apr 16 2020, 7:12 AM
tambre marked 6 inline comments as done.

Remove SVE, fix formatting

llvm/lib/Target/AArch64/AArch64.td
615

All the ARM vector extensions are a bit confusing. SVE indeed isn't supported, I've removed it.
I confirmed that AES, SHA1 and SHA2 extensions work on a real machine.

llvm/unittests/Support/Host.cpp
271

The output is from a Xavier system running the Linux variant provided by Nvidia.

llvm/unittests/Support/TargetParserTest.cpp
981

Fixed manually. Using clang-format results in unpleasant formatting similar to "apple-a10" and "apple-a11" above.

tambre edited the summary of this revision. (Show Details)Apr 18 2020, 5:50 AM
tambre updated this revision to Diff 258513.Apr 18 2020, 6:10 AM

Add cacheline size per the technical reference manual

sdesmalen: Could you please take another look?

It seems a NVIDIA Developer Program Membership is needed in order to download the TRM. I'm not sure if that is just a matter of creating an account, but without it I can't really verify that the architecture version and the features are correct and complete.

clang/test/Preprocessor/aarch64-target-features.c
183

This test references unsupported features.

llvm/include/llvm/Support/AArch64TargetParser.def
183

This still references AEK_SVE.

llvm/unittests/Support/TargetParserTest.cpp
981

Okay, that's fine, thanks for fixing.

1135

nit: It seems a bit arbitrary to only check for fp16 here?

tambre updated this revision to Diff 259653.Apr 23 2020, 11:38 AM
tambre marked 4 inline comments as done.

Remove SVE, add another check to parser test

It seems a NVIDIA Developer Program Membership is needed in order to download the TRM. I'm not sure if that is just a matter of creating an account, but without it I can't really verify that the architecture version and the features are correct and complete.

I was afraid that'd be the case. The "NVIDIA Developer Program Membership" should be free to join if you register and give them some of your information.

clang/test/Preprocessor/aarch64-target-features.c
183

I think only SVE was wrong here. Removed it.

llvm/include/llvm/Support/AArch64TargetParser.def
183

Fixed.

llvm/unittests/Support/TargetParserTest.cpp
1135

I based this off mostly of what was already here. I think most extensions are covered by architecture extension tests. I've added crypto here though.

sdesmalen: ping

Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2020, 12:50 AM
sdesmalen accepted this revision.Apr 30 2020, 1:56 AM

LGTM!

llvm/unittests/Support/TargetParserTest.cpp
1135

Cheers, that's should indeed be sufficient.

This revision is now accepted and ready to land.Apr 30 2020, 1:56 AM

LGTM!

Please land this for me as I lack commit privileges. :)

This revision was automatically updated to reflect the committed changes.