| Message ID | ortthxfb6s.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [testsuite,arm,vect] adjust mve-vshr test [PR113281] | expand |
On 13/06/2024 10:23, Alexandre Oliva wrote: > > 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. > > Tested on x86_64-linux-gnu-x-arm-eabi. Also tested with gcc-13 > x-arm-vx7r2. Ok to install? > > > for gcc/testsuite/ChangeLog > > PR tree-optimization/113281 > * gcc.target/arm/simd/mve-vshr.c: Adjust expectations. > --- > gcc/testsuite/gcc.target/arm/simd/mve-vshr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c > index 8c7adef9ed8f1..8253427db6ef6 100644 > --- a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c > +++ b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c > @@ -56,9 +56,9 @@ FUNC_IMM(u, uint, 8, 16, >>, vshrimm) > /* MVE has only 128-bit vectors, so we can vectorize only half of the > functions above. */ > /* Vector right shifts use vneg and left shifts. */ > -/* { dg-final { scan-assembler-times {vshl.s[0-9]+\tq[0-9]+, q[0-9]+} 3 } } */ > -/* { dg-final { scan-assembler-times {vshl.u[0-9]+\tq[0-9]+, q[0-9]+} 3 } } */ > -/* { dg-final { scan-assembler-times {vneg.s[0-9]+\tq[0-9]+, q[0-9]+} 6 } } */ > +/* { dg-final { scan-assembler-times {vshl.s[0-9]+\tq[0-9]+, q[0-9]+} 1 } } */ > +/* { dg-final { scan-assembler-times {vshl.u[0-9]+\tq[0-9]+, q[0-9]+} 1 } } */ > +/* { dg-final { scan-assembler-times {vneg.s[0-9]+\tq[0-9]+, q[0-9]+} 2 } } */ > > > /* Shift by immediate. */ > > We know the range of the LHS of the shift operand (it comes from a T* array, so that isn't the issue. The problem comes from the RHS of the shift where the range can legitimately come from 0..31 (since the shift is conceptually performed at int precision). That can't be handled correctly as gimple only supports shifts on the number of bits in the type, itself. But we can inform the compiler that it needn't care about the larger range with a __builtin_unreachable(). It looks like adding if ((unsigned)b[i] >= 8*sizeof (TYPE##BITS##_t)) \ __builtin_unreachable(); \ to the shift-by-variable case is enough to tell the vectorizer that it's safe to vectorize this code without needing to handle any additional clamping. Since this test is primarily about testing the MVE vector operations, I think I'd rather go with a solution along those lines rather than nobbling the test. Thoughts? R.
diff --git a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c index 8c7adef9ed8f1..8253427db6ef6 100644 --- a/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c +++ b/gcc/testsuite/gcc.target/arm/simd/mve-vshr.c @@ -56,9 +56,9 @@ FUNC_IMM(u, uint, 8, 16, >>, vshrimm) /* MVE has only 128-bit vectors, so we can vectorize only half of the functions above. */ /* Vector right shifts use vneg and left shifts. */ -/* { dg-final { scan-assembler-times {vshl.s[0-9]+\tq[0-9]+, q[0-9]+} 3 } } */ -/* { dg-final { scan-assembler-times {vshl.u[0-9]+\tq[0-9]+, q[0-9]+} 3 } } */ -/* { dg-final { scan-assembler-times {vneg.s[0-9]+\tq[0-9]+, q[0-9]+} 6 } } */ +/* { dg-final { scan-assembler-times {vshl.s[0-9]+\tq[0-9]+, q[0-9]+} 1 } } */ +/* { dg-final { scan-assembler-times {vshl.u[0-9]+\tq[0-9]+, q[0-9]+} 1 } } */ +/* { dg-final { scan-assembler-times {vneg.s[0-9]+\tq[0-9]+, q[0-9]+} 2 } } */ /* Shift by immediate. */