This is an archive of the discontinued LLVM Phabricator instance.

[Sema][SVE] Move/simplify Sema testing for SVE ACLE builtins
ClosedPublic

Authored by RosieSumpter on May 4 2022, 7:32 AM.

Details

Summary

Currently for SVE ACLE builtins, single tests are used to verify both
clang code generation (when the feature is available) and semantic
error/warning messages (when the feature is unavailable). This
patch moves the semantic testing into dedicated Sema tests.

Diff Detail

Event Timeline

RosieSumpter created this revision.May 4 2022, 7:32 AM
Herald added a project: Restricted Project. · View Herald Transcript
RosieSumpter requested review of this revision.May 4 2022, 7:32 AM
  • Make operand names more descriptive
sdesmalen accepted this revision.May 9 2022, 7:44 AM
sdesmalen added inline comments.
clang/test/Sema/aarch64-sve-intrinsics/acle_sve_imm.cpp
278

I think these are strictly already tested with the above tests which tests their range, but I guess it can't hurt to keep them.

414

This isn't strictly an immediate test, but it has to live somewhere so I'm okay with it being here.

434

nit: can you add a comment here that says // Test type checks on svpattern and svprfop enums.

437

nit: maybe wrap this in SVE_ACLE_FUNC as well to match the rest of this file, or remove the SVE_ACLE_FUNC from the test-case below.

This revision is now accepted and ready to land.May 9 2022, 7:44 AM
sdesmalen added inline comments.May 9 2022, 7:51 AM
clang/test/Sema/aarch64-sve-intrinsics/acle_sve_bfloat.cpp
11

Can you wrap these in SVE_ACLE_FUNC and add an extra RUN line? (similar to my request on D124850)

RosieSumpter marked 2 inline comments as done.

Addressed nits

clang/test/Sema/aarch64-sve-intrinsics/acle_sve_bfloat.cpp
11

Does it make sense to test the overloaded forms here? They weren't tested in the original files, and since the test would look like e.g. SVE_ACLE_FUNC(svcreate2,_bf16,,)(bf16, bf16), if the overload run line has +sve but not +bf16 then the overloaded form svcreate2(bf16, bf16) doesn't give an error about the intrinsic being undeclared, it gives errors about the params of test_bfloat() not being known types

sdesmalen added inline comments.May 10 2022, 3:12 AM
clang/test/Sema/aarch64-sve-intrinsics/acle_sve_bfloat.cpp
11

Good point, I didn't realise that the bfloat types aren't available when the +bf16 attribute isn't passed in. In that case, I agree it's best to keep it as-is.

This revision was landed with ongoing or failed builds.May 10 2022, 5:23 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2022, 5:23 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_svcnt.c