[Bug c/106470] New: Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[Bug c/106470] New: Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[vt at altlinux dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

            Bug ID: 106470
           Summary: Subscribed access to __m256i casted to (uint16_t *)
                    produces garbage or a warning
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vt at altlinux dot org
  Target Milestone: ---

It's appeared in liboqs in code imported from PQClean[1]. There is minimized
reproducer:
1.
```
$ cat test1.c
#include <immintrin.h>
#include <stdio.h>
#include <stdint.h>
#include <string.h>
int main(void)
{
        __m256i tmp = _mm256_set_epi16(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
13, 14, 15, 16);
        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", ((uint16_t *)&tmp)[i]);
        }
        printf("\n");
        return 0;
}
$ gcc -O2 -Wall -mavx2 -o a test1.c
$ ./a
 c040 69a1 558e 0000 68da b7f6 7fb1 0000 87d0 b7f6 7fb1 0000 0000 0000 0001
0000
```
This should been printing:
 0010 000f 000e 000d 000c 000b 000a 0009 0008 0007 0006 0005 0004 0003 0002
0001

2. If we add `#pragma GCC unroll 16` it compiles with the warning, still
printing incorrect values.
```
$ cat test2.c
#include <immintrin.h>
#include <stdio.h>
#include <stdint.h>
#include <string.h>
int main(void)
{
        __m256i tmp = _mm256_set_epi16(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
13, 14, 15, 16);
#pragma GCC unroll 16
        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", ((uint16_t *)&tmp)[i]);
        }
        printf("\n");
        return 0;
}

$ gcc -O2 -Wall -mavx2 -o b test2.c
test2.c: In function ‘main’:
test2.c:10:51: warning: ‘tmp’ is used uninitialized [-Wuninitialized]
   10 |                 printf(" %04x", ((uint16_t *)&tmp)[i]);
      |                                 ~~~~~~~~~~~~~~~~~~^~~
test2.c:7:17: note: ‘tmp’ was declared here
    7 |         __m256i tmp = _mm256_set_epi16(1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
11, 12, 13, 14, 15, 16);
      |                 ^~~
$ ./b
 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000
0000
```

In comparison, clang produces correct output.


[1] https://github.com/open-quantum-safe/liboqs/issues/1244

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
You are violating C aliasing rules.
You need a uint16_t type which is marked as may_alias. Or use memcpy or use 
-fno-strict-aliasing etc.

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The easiest way to fix this is to use GNU-C vectors like:


        for (size_t i = 0; i < 16; i++) {
                typedef __attribute__((vector_size(sizeof(__m256i)) )) uint16_t
myvector_t;
                myvector_t t;
                t = (myvector_t)tmp;
                printf(" %04x", t[i]);
        }

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The other fix is to use _mm256_extract_epi16.
E.g.


inline unsigned short extract_epi16(__m256i v, int pos) {
    switch(pos){
        case 0: return _mm256_extract_epi16(v, 0);
        case 1: return _mm256_extract_epi16(v, 1);
        case 2: return _mm256_extract_epi16(v, 2);
        case 3: return _mm256_extract_epi16(v, 3);
        case 4: return _mm256_extract_epi16(v, 4);
        case 5: return _mm256_extract_epi16(v, 5);
        case 6: return _mm256_extract_epi16(v, 6);
        case 7: return _mm256_extract_epi16(v, 7);
        case 8: return _mm256_extract_epi16(v, 8);
        case 9: return _mm256_extract_epi16(v, 9);
        case 10: return _mm256_extract_epi16(v, 10);
        case 11: return _mm256_extract_epi16(v, 11);
        case 12: return _mm256_extract_epi16(v, 12);
        case 13: return _mm256_extract_epi16(v, 13);
        case 14: return _mm256_extract_epi16(v, 14);
        case 15: return _mm256_extract_epi16(v, 15);
    }
    return 0;
}

...

        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", extract_epi16(tmp, i));
        }

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[vt at altlinux dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #4 from Vitaly Chikunov <vt at altlinux dot org> ---
Andrew, thanks for the quick answer and example! I wish that warning was a bit
more enlightening, and in the first case it is not quietly compiles producing
incorrect code (which is only found by testing).

I also found two variants that's producing correct output:

        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", ((__v16hu)tmp)[i]);
        }

(`__v16hu` is defined in `avxintrin.h`) and similar to how `__v16hu` is defined
there:

        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", ((uint16_t __attribute__ ((__vector_size__
(32)))) tmp)[i] );
        }

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[vt at altlinux dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #5 from Vitaly Chikunov <vt at altlinux dot org> ---
I experimented with `_mm256_extract_epi16` too and loop liek this worked too:

```
# pragma GCC unroll 16
        for (size_t i = 0; i < 16; i++) {
                printf(" %04x", _mm256_extract_epi16(tmp, i));
        }
```

That's why I started to experiment with `#pragma GCC unroll 16` for the 2nd
test case.

Curiously, in `liboqs` adding the pragma removes the `‘tmp’ is used
uninitialized` warning (while w/o pragma it compiles with warning), but in
reproducer it acts contrary - with the pragma it causes warning and w/o pragma
it compiles quietly.

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amonakov at gcc dot gnu.org

--- Comment #6 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
Andrew, surely the bogus -Wuninitialized warning is a GCC bug here?

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Alexander Monakov from comment #6)
> Andrew, surely the bogus -Wuninitialized warning is a GCC bug here?

No. It is just exposing the undefined behavior.

[Bug middle-end/106470] Subscribed access to __m256i casted to (uint16_t *) produces garbage or a warning

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470

--- Comment #8 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
But that's the point of many warnings, isn't it? To help the user understand
what's wrong when the code is bad? And bogus warnings just confuse more.