[PATCH] D123605: [WIP][Sema][SVE] Move/simplify Sema testing for SVE ACLE builtins

Paul Walker via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 28 03:42:15 PDT 2022


paulwalker-arm added inline comments.


================
Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2_imm_n.cpp:25
+{
+  // expected-error-re at +1 3 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  EXPAND_XZM_FUNC(SVE_ACLE_FUNC(svqshlu,_n_s8,,), pg, svundef_s8(), -1);
----------------
RosieSumpter wrote:
> paulwalker-arm wrote:
> > I've not seen this before, presumably it's short hand instead of needing to repeat multiple identical `expected-error` check lines?  If so, is it worth using this throughout the test files and essentially only require one `expected-error` per function or does this only work here because the `EXPAND...` macro emits its three function calls on the same line?
> Yes it lets you specify how many times you expect the diagnostic to appear, but as you said it only works when the diagnostics are emitted on the same line so I'm not sure there's a way to reduce the number of `expected-error` lines any more than this
OK, thanks for checking.  To be honest I'm not sure why we need the `EXPAND_XZM_FUNC` macro given `SVE_ACLE_FUNC` worked fine before.  To my eye it kind of ruins the flow, but hey-ho I'll not worry about it.

Assuming I've not screwed up I think you're missing tests for `SVE_ACLE_FUNC(svrshrnb,_n_s16,,)` and `SVE_ACLE_FUNC(svrshrnt,_n_s16,,)`.


================
Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2_imm_rotation.cpp:17-40
+  // expected-error at +1 {{argument should be the value 90 or 270}}
+  SVE_ACLE_FUNC(svcadd,_s8,,)(svundef_s8(), svundef_s8(), 0);
+  // expected-error at +1 {{argument should be the value 90 or 270}}
+  SVE_ACLE_FUNC(svcadd,_u8,,)(svundef_u8(), svundef_u8(), 0);
+  // expected-error at +1 {{argument should be the value 90 or 270}}
+  SVE_ACLE_FUNC(svcadd,_s16,,)(svundef_s16(), svundef_s16(), 0);
+  // expected-error at +1 {{argument should be the value 90 or 270}}
----------------
I know we cannot test every number but `180` seems like a reasonable mistake for people to make given the other complex number instructions so perhaps alternate between `0` and `180` to give a little more coverage without increasing the number of lines.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123605/new/

https://reviews.llvm.org/D123605



More information about the cfe-commits mailing list