This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE][Clang] Fix crash for incorrect svptrue and svcnt parameters
ClosedPublic

Authored by MattDevereau on Mar 9 2022, 7:53 AM.

Details

Summary

Giving an int parameter to SVE intrinsics svptrue and svcnt caused Clang to crash on compilation. Changing their parameter types to void instead of omitting args results in a diagnostic error message instead.

Diff Detail

Event Timeline

MattDevereau created this revision.Mar 9 2022, 7:53 AM
MattDevereau requested review of this revision.Mar 9 2022, 7:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2022, 7:53 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This is missing tests for svundef, svrdffr, svsetffr and svpfalse?

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_svptrue.c
3

Is it possible to use the update_cc_test_checks.py script for these tests?

MattDevereau added a comment.EditedMar 9 2022, 8:22 AM

This is missing tests for svundef, svrdffr, svsetffr and svpfalse?

@sdesmalen Only svcnt and svptrue cause the crash. This might be because of extra values added by the [IsAppendSVALL] TypeFlag in arm_sve.td. We decided to change other intrinsics with 0 parameters as there seems to be no drawbacks to doing so and it might catch further future errors.

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_svptrue.c
3

Deleting // expected-error-re@+1... and running update_cc_test_checks.py did not generate anything in its place in either of the tests

sdesmalen accepted this revision.Mar 9 2022, 9:28 AM

This is missing tests for svundef, svrdffr, svsetffr and svpfalse?

@sdesmalen Only svcnt and svptrue cause the crash. This might be because of extra values added by the [IsAppendSVALL] TypeFlag in arm_sve.td. We decided to change other intrinsics with 0 parameters as there seems to be no drawbacks to doing so and it might catch further future errors.

Fair enough. Since you're adding the void to the prototype for a reason (the diagnostic behaviour now changes), I figured you may as well want to test it.

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_svptrue.c
3

Okay I wasn't sure, so thanks for confirming.

This revision is now accepted and ready to land.Mar 9 2022, 9:28 AM