[Bug target/53192] New: Incorrect arguments to AVX2's gather intrinsics

[Bug target/53192] New: Incorrect arguments to AVX2's gather intrinsics

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

             Bug #: 53192
           Summary: Incorrect arguments to AVX2's gather intrinsics
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: kirill.yukhin@intel.com


Hello,
Seems Intel's latest Spec contains a bug, which was reproduced in avx2intrin.h

_mm_i32gather_epi64 (long long int const *base,
                     __m128i index, const int scale)

This has led to an incompatibility between Intel and Gnu compilers. The Intel
version of immintrin.h specifies the type as __int64 const *. The type __int64
is a non-standard MS invention, which apparently is compatible with the
standard type int64_t (inttypes.h). The GCC version of avx2intrin.h specifies
the same parameter as long long int const *. 
Unfortunately, these two types are incompatible under 64-bit Linux.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2012-05-02
     Ever Confirmed|0                           |1

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2012-05-02 15:56:55 UTC ---
Please provide a testcase to show the problem.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

Yukhin Kirill <kirill.yukhin at intel dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |areg.melikadamyan at gmail
                   |                            |dot com

--- Comment #3 from Yukhin Kirill <kirill.yukhin at intel dot com> 2012-05-22 08:23:56 UTC ---
(In reply to comment #1)
> Please provide a testcase to show the problem.

I have no idea, which kind of test it should be.
These is just MS-ICC-GCC incompatibility issue

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[kirill.yukhin at intel dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

--- Comment #2 from Yukhin Kirill <kirill.yukhin at intel dot com> 2012-05-22 08:22:12 UTC ---
(In reply to comment #1)
> Please provide a testcase to show the problem.

I have no idea, which kind of test it should be.
These is just MS-ICC-GCC incompatibility issue

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-22 11:18:50 UTC ---
Can you explain how are the two incompatible?  You mean you get a warning if
you pass an address of int64_t to this intrinsic?  Even if this weirdo __int64
is compatible with long rather than long long in LP64, both are layed out the
same.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53192

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-22 11:52:12 UTC ---
BTW, to avoid that warning, you could use in C:
extern __inline __m128i
__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
_mm_i32gather_epi64 (
#ifdef __cplusplus
                     long long int const *base,
#else
                     union __attribute__((__transparent_union__)) { long long
int const *__ll; long int const *__l; } base,
#endif
                     __m128i index, const int scale)
and for C++ add another overload (of course, for _LP64 only).

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |diagnostic, rejects-valid
             Target|x86-64                      |x86-64 (LP64)
             Status|WAITING                     |NEW

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
Here is a testcase which compiles just fine at -O0 but fails at -O1 and above:
```
#include <immintrin.h>

__m128i f(long *base, __m128i vindx, int scale)
{
        return _mm_i32gather_epi64 (base, vindx, 1);
}
__m128i g(long long *base, __m128i vindx, int scale)
{
        return _mm_i32gather_epi64 (base, vindx, 1);
}
```

Note clang's version is to always use a macro here.
ICC directly defines these internally as real intrinsics and accepts the above
without an error.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> BTW, to avoid that warning, you could use in C:
> extern __inline __m128i
> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> _mm_i32gather_epi64 (
> #ifdef __cplusplus
>                      long long int const *base,
> #else
>                      union __attribute__((__transparent_union__)) { long
> long int const *__ll; long int const *__l; } base,
> #endif
>                      __m128i index, const int scale)
> and for C++ add another overload (of course, for _LP64 only).

The other option is to change how intrinsics work on x86 and use resolve
overloads inside the backend like how aarch64, arm and rs6000 backends all
handle intrinsics these days.

That would be a huge rewrite of the intrinsics but it might resolve some of the
other issues dealing the builtins and such too.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Looking at other intrinsics with {,unsigned }__int64{, const} * arguments, I
see
void _mm_maskstore_epi64 (__int64* mem_addr, __m128i mask, __m128i a)
void _mm256_maskstore_epi64 (__int64* mem_addr, __m256i mask, __m256i a)
unsigned __int64 _mulx_u64 (unsigned __int64 a, unsigned __int64 b, unsigned
__int64* hi)
int _rdrand64_step (unsigned __int64* val)
unsigned char _addcarry_u64 (unsigned char c_in, unsigned __int64 a, unsigned
__int64 b, unsigned __int64 * out)
unsigned char _addcarryx_u64 (unsigned char c_in, unsigned __int64 a, unsigned
__int64 b, unsigned __int64 * out)
int _rdseed64_step (unsigned __int64 * val)
unsigned char _subborrow_u64 (unsigned char c_in, unsigned __int64 a, unsigned
__int64 b, unsigned __int64 * out)
__m128i _mm_i32gather_epi64 (__int64 const* base_addr, __m128i vindex, const
int scale)
__m128i _mm_mask_i32gather_epi64 (__m128i src, __int64 const* base_addr,
__m128i vindex, __m128i mask, const int scale)
__m256i _mm256_i32gather_epi64 (__int64 const* base_addr, __m128i vindex, const
int scale)
__m256i _mm256_mask_i32gather_epi64 (__m256i src, __int64 const* base_addr,
__m128i vindex, __m256i mask, const int scale)
__m128i _mm_i64gather_epi64 (__int64 const* base_addr, __m128i vindex, const
int scale)
__m128i _mm_mask_i64gather_epi64 (__m128i src, __int64 const* base_addr,
__m128i vindex, __m128i mask, const int scale)
__m256i _mm256_i64gather_epi64 (__int64 const* base_addr, __m256i vindex, const
int scale)
__m256i _mm256_mask_i64gather_epi64 (__m256i src, __int64 const* base_addr,
__m256i vindex, __m256i mask, const int scale)
__m128i _mm_maskload_epi64 (__int64 const* mem_addr, __m128i mask)
__m256i _mm256_maskload_epi64 (__int64 const* mem_addr, __m256i mask)
in the intrinsic guide.  And both GCC and LLVM consistently use long
long/unsigned long long pointers for all; of those.  And that type isn't
predefined by either of the compilers, so I'd just say that the hypothetical
__int64/unsigned __int64 is long long/unsigned long long on Linux, not
int64_t/uint64_t.

[Bug target/53192] Incorrect arguments to AVX2's gather intrinsics

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> The other option is to change how intrinsics work on x86 and use resolve
> overloads inside the backend like how aarch64, arm and rs6000 backends all
> handle intrinsics these days.

Ugh no, that is terrible.  Not being able to actually figure out what the
header provides as intrinsics, with what arguments etc. from anything but
documentation is bad.