This is an archive of the discontinued LLVM Phabricator instance.

[Clang][SVE2.1] Add clang support for prototypes using svcount_t
ClosedPublic

Authored by CarolineConcatto on May 19 2023, 3:15 AM.

Details

Summary

In this patch it is used for the prototype:

Patch by: Sander de Smalen <sander.desmalen@arm.com>

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 3:15 AM
CarolineConcatto requested review of this revision.May 19 2023, 3:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2023, 3:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
CarolineConcatto edited the summary of this revision. (Show Details)May 22 2023, 1:15 AM
This revision is now accepted and ready to land.May 23 2023, 5:51 AM
sdesmalen added inline comments.May 23 2023, 6:27 AM
clang/include/clang/Basic/Builtins.def
42

Can we make 'Q' something to mean 'target type' and then use a second letter to clarify which exact target type it is, e.g.

// Q -> target builtin type, followed by a character to distinguish the builtin type
//   Qa -> AArch64 svcount_t builtin type.

That should make it more extensible for other target types in the future.

Matt added a subscriber: Matt.May 23 2023, 3:16 PM
CarolineConcatto marked an inline comment as done.
CarolineConcatto edited the summary of this revision. (Show Details)

-Address review's comments about Q target type.

hassnaa-arm added inline comments.May 30 2023, 8:53 AM
clang/include/clang/Basic/Builtins.def
42

Is that suggestion because maybe other targets don't support svcount ?

sdesmalen added inline comments.May 30 2023, 8:59 AM
clang/include/clang/Basic/Builtins.def
42

Correct, svcount_t is an AArch64 specific type.

hassnaa-arm added inline comments.May 30 2023, 9:01 AM
clang/include/clang/Basic/Builtins.def
42

Thanks.

sdesmalen added inline comments.May 31 2023, 5:27 AM
clang/include/clang/Basic/arm_sve.td
16

The typespec modifier in this file can remain Q rather than Qa. Can you change it back?

hassnaa-arm added inline comments.May 31 2023, 5:31 AM
clang/include/clang/Basic/arm_sve.td
16

what if there is a new type that is target-specific ?
At that case we will have to find a new character.
But if we now use Qa, we can use Qb for the new type.
Is that correct ?

  • Remove Qa from arm_sve.td and its dependencies
CarolineConcatto marked 2 inline comments as done.May 31 2023, 6:54 AM
CarolineConcatto added inline comments.
clang/include/clang/Basic/arm_sve.td
16

This is now removed. I thought that should reflect what is in Builtins.def, but it does not.
Because Q is being defined in sve target we do not the 'a' in 'Qa'

sdesmalen accepted this revision.May 31 2023, 7:05 AM
This revision was automatically updated to reflect the committed changes.
CarolineConcatto marked an inline comment as done.