[Bug c/109173] New: incorrect intrinsic signature for _mm_srai_epi64

[Bug c/109173] New: incorrect intrinsic signature for _mm_srai_epi64

[malat at debian dot org]

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

            Bug ID: 109173
           Summary: incorrect intrinsic signature for _mm_srai_epi64
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: malat at debian dot org
  Target Milestone: ---

gcc-13 currently rejects the following:

% /usr/lib/gcc-snapshot/bin/gcc -c -Wsign-conversion -Werror -mavx512vl -O2
tu.c
tu.c: In function 'foo':
tu.c:7:30: error: conversion to 'int' from 'unsigned int' may change the sign
of the result [-Werror=sign-conversion]
    7 |     return _mm_srai_epi64(a, bar());
      |                              ^~~~~
cc1: all warnings being treated as errors

with:

% cat tu.c
#include <immintrin.h>

extern unsigned int bar();

__m128i foo(__m128i a)
{
    return _mm_srai_epi64(a, bar());
}

intel documentation declares it as unsigned int

*
https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/intrinsics-for-shift-operations.html

[...]
__m128i _mm_srai_epi64(__m128i a, unsigned int imm)
[...]

% /usr/lib/gcc-snapshot/bin/gcc --version
gcc (Debian 20230315-1) 13.0.1 20230315 (experimental) [master
r13-6680-ga9ae16db8cb]

[Bug c/109173] incorrect intrinsic signature for _mm_srai_epi64

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com,
                   |                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This is a bogus assumption IMHO.

[Bug c/109173] incorrect intrinsic signature for _mm_srai_epi64

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
To expand on it further, nothing guarantees that the intrinsic is actually an
inline function, it can be also macro etc., so relying on such details as exact
operand type is impossible.

[Bug c/109173] incorrect intrinsic signature for _mm_srai_epi64

[malat at debian dot org]

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

--- Comment #3 from Mathieu Malaterre <malat at debian dot org> ---
In a previous episode:

> ICC is our gold standard as far as intrinsics are concerned. So, unless there is > a real issue with some specific builtin, I think we should follow ICC.

* https://gcc.gnu.org/bugzilla//show_bug.cgi?id=87674#c4

[Bug c/109173] incorrect intrinsic signature for _mm_srai_epi64

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Except that ICC changes it over time...

[Bug target/109173] incorrect intrinsic signature for _mm_srai_epi64

[jan.wassenberg at gmail dot com]

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

--- Comment #5 from Jan Wassenberg <jan.wassenberg at gmail dot com> ---
Thanks, Mathieu, for raising this.

Note that clang has changed their intrinsic to require an unsigned arg: 
https://github.com/google/highway/commit/45b1fac0b1c404e6573c2f182b36c245af6503e0

I understand that some implementations may be using a macro, and that's fine,
but certain GCC and clang have non-macro intrinsics that are mutually
incompatible, and users including us are forced to use #if to compile without
warnings.

My understanding is that icc is being replaced with clang. Wouldn't it be
better then to follow the Intel intrinsics documentation?

[Bug target/109173] incorrect intrinsic signature for _mm_srai_epi64

[crazylht at gmail dot com]

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

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Jan Wassenberg from comment #5)
> Thanks, Mathieu, for raising this.
> 
> Note that clang has changed their intrinsic to require an unsigned arg: 
> https://github.com/google/highway/commit/
> 45b1fac0b1c404e6573c2f182b36c245af6503e0
> 
> I understand that some implementations may be using a macro, and that's
> fine, but certain GCC and clang have non-macro intrinsics that are mutually
> incompatible, and users including us are forced to use #if to compile
> without warnings.
> 
> My understanding is that icc is being replaced with clang. Wouldn't it be
> better then to follow the Intel intrinsics documentation?

Sure, we can do that.

[Bug target/109173] incorrect intrinsic signature for _mm_srai_epi64

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Haochen Jiang <jianghc@gcc.gnu.org>:

https://gcc.gnu.org/g:4a84a2db2ad7cc57c1674849f845c42ed4ad12ab

commit r14-1234-g4a84a2db2ad7cc57c1674849f845c42ed4ad12ab
Author: Hu, Lin1 <lin1.hu@intel.com>
Date:   Tue May 23 10:08:29 2023 +0800

    i386: Fix incorrect intrinsic signature for AVX512 s{lli|rai|rli}

    This patch aims to fix incorrect intrinsic signature for
    _mm{512|256|}_s{lli|rai|rli}_epi*.

    gcc/ChangeLog:

            PR target/109173
            PR target/109174
            * config/i386/avx512bwintrin.h (_mm512_srli_epi16): Change type
from
            int to const int or const int to const unsigned int.
            (_mm512_mask_srli_epi16): Ditto.
            (_mm512_slli_epi16): Ditto.
            (_mm512_mask_slli_epi16): Ditto.
            (_mm512_maskz_slli_epi16): Ditto.
            (_mm512_srai_epi16): Ditto.
            (_mm512_mask_srai_epi16): Ditto.
            (_mm512_maskz_srai_epi16): Ditto.
            * config/i386/avx512fintrin.h (_mm512_slli_epi64): Ditto.
            (_mm512_mask_slli_epi64): Ditto.
            (_mm512_maskz_slli_epi64): Ditto.
            (_mm512_srli_epi64): Ditto.
            (_mm512_mask_srli_epi64): Ditto.
            (_mm512_maskz_srli_epi64): Ditto.
            (_mm512_srai_epi64): Ditto.
            (_mm512_mask_srai_epi64): Ditto.
            (_mm512_maskz_srai_epi64): Ditto.
            (_mm512_slli_epi32): Ditto.
            (_mm512_mask_slli_epi32): Ditto.
            (_mm512_maskz_slli_epi32): Ditto.
            (_mm512_srli_epi32): Ditto.
            (_mm512_mask_srli_epi32): Ditto.
            (_mm512_maskz_srli_epi32): Ditto.
            (_mm512_srai_epi32): Ditto.
            (_mm512_mask_srai_epi32): Ditto.
            (_mm512_maskz_srai_epi32): Ditto.
            * config/i386/avx512vlbwintrin.h (_mm256_mask_srai_epi16): Ditto.
            (_mm256_maskz_srai_epi16): Ditto.
            (_mm_mask_srai_epi16): Ditto.
            (_mm_maskz_srai_epi16): Ditto.
            (_mm256_mask_slli_epi16): Ditto.
            (_mm256_maskz_slli_epi16): Ditto.
            (_mm_mask_slli_epi16): Ditto.
            (_mm_maskz_slli_epi16): Ditto.
            (_mm_maskz_srli_epi16): Ditto.
            * config/i386/avx512vlintrin.h (_mm256_mask_srli_epi32): Ditto.
            (_mm256_maskz_srli_epi32): Ditto.
            (_mm_mask_srli_epi32): Ditto.
            (_mm_maskz_srli_epi32): Ditto.
            (_mm256_mask_srli_epi64): Ditto.
            (_mm256_maskz_srli_epi64): Ditto.
            (_mm_mask_srli_epi64): Ditto.
            (_mm_maskz_srli_epi64): Ditto.
            (_mm256_mask_srai_epi32): Ditto.
            (_mm256_maskz_srai_epi32): Ditto.
            (_mm_mask_srai_epi32): Ditto.
            (_mm_maskz_srai_epi32): Ditto.
            (_mm256_srai_epi64): Ditto.
            (_mm256_mask_srai_epi64): Ditto.
            (_mm256_maskz_srai_epi64): Ditto.
            (_mm_srai_epi64): Ditto.
            (_mm_mask_srai_epi64): Ditto.
            (_mm_maskz_srai_epi64): Ditto.
            (_mm_mask_slli_epi32): Ditto.
            (_mm_maskz_slli_epi32): Ditto.
            (_mm_mask_slli_epi64): Ditto.
            (_mm_maskz_slli_epi64): Ditto.
            (_mm256_mask_slli_epi32): Ditto.
            (_mm256_maskz_slli_epi32): Ditto.
            (_mm256_mask_slli_epi64): Ditto.
            (_mm256_maskz_slli_epi64): Ditto.

    gcc/testsuite/ChangeLog:

            PR target/109173
            PR target/109174
            * gcc.target/i386/pr109173-1.c: New test.
            * gcc.target/i386/pr109174-1.c: Ditto.

[Bug target/109173] incorrect intrinsic signature for _mm_srai_epi64

[crazylht at gmail dot com]

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

--- Comment #8 from Hongtao.liu <crazylht at gmail dot com> ---
Fixed for GCC14.

[Bug target/109173] incorrect intrinsic signature for _mm_srai_epi64

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.