rs6000: Fix up *intrin.h for C89 [PR104239]
Commit Message
Hi!
When writing testcases for the previously posted patch, I have noticed
that 3 of the headers aren't valid C89 (I didn't have any dg-options
so -ansi -pedantic-errors was implied and these errors were reported).
The following patch fixes those, ok for trunk?
Note, as can be seen even in this patch, seems older rs6000/*intrin.h
headers uglify not just argument names (__A instead of A etc.), but also
automatic variable names and other local identifiers, while e.g. emmintrin.h
or bmi2intrin.h clearly uglify only the argument names and not local
variables. I think that should be fixed but don't have time for that myself
(libstdc++ or e.g. the x86 headers uglify everything; this is so that one
can
#define result a + b
#include <x86intrin.h>
etc.).
2022-01-26 Jakub Jelinek <jakub@redhat.com>
PR target/104239
* config/rs6000/emmintrin.h (_mm_sad_epu8): Use __asm__ instead of
asm.
* config/rs6000/smmintrin.h (_mm_minpos_epu16): Declare iterator
before for loop instead of for init clause.
* config/rs6000/bmi2intrin.h (_pext_u64): Likewise.
* gcc.target/powerpc/pr104239-3.c: New test.
Jakub
Comments
Hi!
On Wed, Jan 26, 2022 at 10:03:36PM +0100, Jakub Jelinek wrote:
> When writing testcases for the previously posted patch, I have noticed
> that 3 of the headers aren't valid C89 (I didn't have any dg-options
> so -ansi -pedantic-errors was implied and these errors were reported).
Hrm. Do they even work like that?
> Note, as can be seen even in this patch, seems older rs6000/*intrin.h
> headers uglify not just argument names (__A instead of A etc.), but also
> automatic variable names and other local identifiers, while e.g. emmintrin.h
> or bmi2intrin.h clearly uglify only the argument names and not local
> variables. I think that should be fixed but don't have time for that myself
> (libstdc++ or e.g. the x86 headers uglify everything; this is so that one
> can
> #define result a + b
> #include <x86intrin.h>
> etc.).
Heh, I argued for this before, but mistakenly for arguments as well.
Not that I can remember right now how function arguments can mess up
either?
> --- gcc/config/rs6000/bmi2intrin.h.jj 2022-01-26 20:42:53.132315506 +0000
> +++ gcc/config/rs6000/bmi2intrin.h 2022-01-26 20:46:33.687983641 +0000
> @@ -115,10 +115,11 @@ _pext_u64 (unsigned long long __X, unsig
> the Power8 Bit permute instruction. */
> if (__builtin_constant_p (__M) && (__builtin_popcountl (__M) <= 8))
> {
> + long i;
> /* Also if the pext mask is constant, then the popcount is
> constant, we can evaluate the following loop at compile
> time and use a constant bit permute vector. */
> - for (long i = 0; i < __builtin_popcountl (__M); i++)
> + for (i = 0; i < __builtin_popcountl (__M); i++)
Please put the declaration immediately before it is used, not at the
start of the function?
Okay for trunk like that. Thanks! Also any and all backports you want
are fine.
Segher
@@ -2215,7 +2215,7 @@ _mm_sad_epu8 (__m128i __A, __m128i __B)
vsum = (__vector signed int) vec_sum4s (vabsdiff, zero);
#ifdef __LITTLE_ENDIAN__
/* Sum across four integers with two integer results. */
- asm ("vsum2sws %0,%1,%2" : "=v" (result) : "v" (vsum), "v" (zero));
+ __asm__ ("vsum2sws %0,%1,%2" : "=v" (result) : "v" (vsum), "v" (zero));
/* Note: vec_sum2s could be used here, but on little-endian, vector
shifts are added that are not needed for this use-case.
A vector shift to correctly position the 32-bit integer results
@@ -687,7 +687,8 @@ _mm_minpos_epu16 (__m128i __A)
union __u __u = { .__m = __A }, __r = { .__m = {0} };
unsigned short __ridx = 0;
unsigned short __rmin = __u.__uh[__ridx];
- for (unsigned long __i = 1; __i < 8; __i++)
+ unsigned long __i;
+ for (__i = 1; __i < 8; __i++)
{
if (__u.__uh[__i] < __rmin)
{
@@ -115,10 +115,11 @@ _pext_u64 (unsigned long long __X, unsig
the Power8 Bit permute instruction. */
if (__builtin_constant_p (__M) && (__builtin_popcountl (__M) <= 8))
{
+ long i;
/* Also if the pext mask is constant, then the popcount is
constant, we can evaluate the following loop at compile
time and use a constant bit permute vector. */
- for (long i = 0; i < __builtin_popcountl (__M); i++)
+ for (i = 0; i < __builtin_popcountl (__M); i++)
{
c = __builtin_clzl (m);
p = (p << 8) | c;
@@ -0,0 +1,7 @@
+/* PR target/104239 */
+/* { dg-options "-O2 -mdejagnu-cpu=power8 -DNO_WARN_X86_INTRINSICS -std=c89" } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+
+#include <x86intrin.h>
+
+int i;