This is an archive of the discontinued LLVM Phabricator instance.

[SVE] Generate overloaded functions for ACLE intrinsics.
ClosedPublic

Authored by sdesmalen on Mar 9 2020, 9:55 AM.

Details

Summary

The SVE ACLE allows using a short-form for the intrinsics, e.g.
the following two declarations generate the same code:

  
svuint32_t svld1(svbool_t, uint32_t const *);
svuint32_t svld1_u32(svbool_t, uint32_t const *);

using the attribute:

__clang_arm_builtin_alias

so that any call to svld1(svbool_t, uint32_t const *) will
map to __builtin_sve_svld1_u32.

Diff Detail

Event Timeline

sdesmalen created this revision.Mar 9 2020, 9:55 AM
Herald added a project: Restricted Project. · View Herald Transcript
sdesmalen updated this revision to Diff 249370.Mar 10 2020, 7:32 AM
sdesmalen edited the summary of this revision. (Show Details)
  • Rebased patch on top of D75850.
  • Removed __clang_arm_sve_alias in favour of __clang_arm_builtin_alias
miyuki added inline comments.Mar 10 2020, 7:44 AM
clang/lib/Sema/SemaDeclAttr.cpp
5002

I would suggest splitting the check into Arm-specific and AArch64-specific checks to speed things up (if it is easy to get the current target arch in this context).

sdesmalen updated this revision to Diff 249404.Mar 10 2020, 9:00 AM
  • Refactored condition to check for valid builtins (split up checks for Arm and AArch64)
sdesmalen marked an inline comment as done.Mar 10 2020, 9:01 AM

The clang attribute part LGTM, but I want someone else to check the rest.

SjoerdMeijer added inline comments.Mar 10 2020, 1:35 PM
clang/include/clang/Basic/Attr.td
362

nit: don't thin we use underscores in names, and looking at examples below, I guess this should be TargetAnyARM

clang/utils/TableGen/SveEmitter.cpp
105

nit: add a comment here too (for consistency)

634

Unrelated to this change, I just spotted it because it looked a bit messy, but is this the old copyright header? While we are at it, replace this with the new one? Looks like e.g. in arm_mve.h, we do use the new one.

SjoerdMeijer accepted this revision.Mar 11 2020, 4:24 AM

With the nit addressed, this LGTM

clang/utils/TableGen/SveEmitter.cpp
634

Let's ignore this for now, we can fix this later if needed.

This revision is now accepted and ready to land.Mar 11 2020, 4:24 AM
This revision was automatically updated to reflect the committed changes.
sdesmalen marked 4 inline comments as done.

Addressed review comments before commititng. Thanks for the review @miyuki and @SjoerdMeijer!