On Jun 21, 2024, Christophe Lyon <christophe.l...@linaro.org> wrote:

>> How about mentioning Christophe's simplification in the commit log?

> For the avoidance of doubt: it's OK for me (but you don't need to
> mention my name in fact ;-)

Needing or not, I added it ;-)

>> > be accepted.  (int16_t)32768 >> (int16_t)16 must yield 0, not 1 (as
>> > before the fix).

>> This is OK, but you might wish to revisit this statement before
>> committing.

Oh, right, sorry, I messed it up.  uint16_t was what I should have put
in there.  int16_t would have overflown and invoked undefined behavior
to begin with, and I see it misled you far down the wrong path.

>> I think the original bug was that we were losing the cast to short

The problem was that the shift count saturated at 15.  AFAIK sign
extension was not relevant.  Hopefully the rewritten opening paragraph
below makes that clearer.  I will put it in later this week barring
objections or further suggestions of improvement.  Thanks,


[testsuite] [arm] [vect] adjust mve-vshr test [PR113281]

The test was too optimistic, alas.  We used to vectorize shifts by
clamping the shift counts below the bit width of the types (e.g. at 15
for 16-bit vector elements), but (uint16_t)32768 >> (uint16_t)16 is
well defined (because of promotion to 32-bit int) and 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 vectorization we no longer performed
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, and Christophe Lyon suggested a further
simplification.


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..03078de49c65e 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] >= (unsigned)(BITS))                          \
+       __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

Reply via email to