Hi,
On Thu, 20 Jun 2024 at 12:51, Alexandre Oliva <ol...@adacore.com> wrote:
>
> On Jun 19, 2024, "Richard Earnshaw (lists)" <richard.earns...@arm.com> wrote:
>
> > It looks like adding
>
> > if ((unsigned)b[i] >= 8*sizeof (TYPE##BITS##_t)) \
> > __builtin_unreachable(); \
>
> Ah, yes, nice, good idea, thanks!
>
> Here's the patch that implements that, co-attributed to you, as IMHO it
> should be; please let me know if you find otherwise. The commit message
> is mostly unchanged from v1, except for the very end. Tested on
> x86_64-linux-gnu-x-arm-eabi. Ok to install?
>
>
> The test was too optimistic, alas. We used to vectorize shifts
> involving 8-bit and 16-bit integral types by clamping the shift count
> at the highest in-range shift count, but that was not correct: such
> narrow shifts expect integral promotion, so larger shift counts should
> be accepted. (int16_t)32768 >> (int16_t)16 must yield 0, not 1 (as
> before the fix).
>
> Unfortunately, in the gimple model of vector units, such large shift
> counts wouldn't be well-defined, so we won't vectorize such shifts any
> more, unless we can tell they're in range or undefined.
>
> So the test that expected the incorrect clamping we no longer perform
> needs to be adjusted. Instead of nobbling the test, Richard Earnshaw
> suggested annotating the test with the expected ranges so as to enable
> the optimization.
>
>
> Co-Authored-By: Richard Earnshaw <richard.earns...@arm.com>
>
> for gcc/testsuite/ChangeLog
>
> PR tree-optimization/113281
> * gcc.target/arm/simd/mve-vshr.c: Add expected ranges.
> ---
> gcc/testsuite/gcc.target/arm/simd/mve-vshr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c
> b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c
> index 8c7adef9ed8f1..35cd0e75be5dc 100644
> --- a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c
> +++ b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c
> @@ -9,6 +9,8 @@
> void test_ ## NAME ##_ ## SIGN ## BITS ## x ## NB (TYPE##BITS##_t *
> __restrict__ dest, TYPE##BITS##_t *a, TYPE##BITS##_t *b) { \
> int i; \
> for (i=0; i<NB; i++) { \
> + if ((unsigned)b[i] >= __CHAR_BIT__ * sizeof (TYPE##BITS##_t)) \
Maybe using
if ((unsigned)b[i] >= BITS) \
would be clearer?
Thanks,
Christophe
> + __builtin_unreachable(); \
> dest[i] = a[i] OP b[i]; \
> } \
> }
>
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
