This is an archive of the discontinued LLVM Phabricator instance.

[ARM,CDE] Generalize MVE intrinsics infrastructure to support CDE
ClosedPublic

Authored by miyuki on Mar 9 2020, 8:15 AM.

Details

Summary

This patch generalizes the existing code to support CDE intrinsics
which will share some properties with existing MVE intrinsics
(some of the intrinsics will be polymorphic and accept/return values
of MVE vector types).
Specifically the patch:

  • Adds new tablegen backends -gen-arm-cde-builtin-def, -gen-arm-cde-builtin-codegen, -gen-arm-cde-builtin-sema, -gen-arm-cde-builtin-aliases, -gen-arm-cde-builtin-header based on existing MVE backends.
  • Renames the 'clang_arm_mve_alias' attribute into 'clang_arm_builtin_alias' (it will be used with CDE intrinsics as well as MVE intrinsics)
  • Implements semantic checks for the coprocessor argument of the CDE intrinsics as well as the existing coprocessor intrinsics.
  • Adds one CDE intrinsic __arm_cx1 to test the above changes

Diff Detail

Event Timeline

miyuki created this revision.Mar 9 2020, 8:15 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 9 2020, 8:15 AM

Renames the 'clang_arm_mve_alias' attribute into 'clang_arm_builtin_alias' (it will be used with CDE intrinsics as well as MVE intrinsics)

You might talk to @sdesmalen, who mentioned introducing an arm_sve_alias attribute in a comment on D75470. Probably better if we all use the same one :-)

miyuki updated this revision to Diff 249140.Mar 9 2020, 9:45 AM

Fix linter and formatter warnings

Renames the 'clang_arm_mve_alias' attribute into 'clang_arm_builtin_alias' (it will be used with CDE intrinsics as well as MVE intrinsics)

You might talk to @sdesmalen, who mentioned introducing an arm_sve_alias attribute in a comment on D75470. Probably better if we all use the same one :-)

Thanks for pointing out! I added arm_sve_alias in D75861, but am happy to update my patch to reuse the generic __clang_arm_builtin_alias!

miyuki added a comment.EditedMar 9 2020, 10:13 AM

but am happy to update my patch to reuse the generic __clang_arm_builtin_alias!

Please do. It would be nicer to have a single attribute than 2 or 3 distinct ones which serve essentially the same goal.

simon_tatham accepted this revision.Mar 10 2020, 5:24 AM

LGTM with a couple of tiny spelling nitpicks.

clang/include/clang/Basic/DiagnosticSemaKinds.td
642

Was this identifier meant to say "invalid" twice?

clang/include/clang/Sema/Sema.h
11768

Another "Corpoc" here.

This revision is now accepted and ready to land.Mar 10 2020, 5:24 AM
miyuki updated this revision to Diff 249362.Mar 10 2020, 6:42 AM

Address review comments

miyuki marked 2 inline comments as done.Mar 10 2020, 6:48 AM
This revision was automatically updated to reflect the committed changes.
nickdesaulniers added inline comments.
clang/lib/Sema/SemaChecking.cpp
2096

I get a warning that this is unused in Release builds.

miyuki marked an inline comment as done.Mar 10 2020, 10:29 AM

Should compile without warnings now.

Should compile without warnings now.

LGTM, thanks