Message ID | db4f94be-b2de-504c-b458-133a85248902@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000] Undefine vector, bool, pixel in xmmintrin.h | expand |
Hi Bill, On Sun, Apr 01, 2018 at 08:24:32PM -0500, Bill Schmidt wrote: > * gcc.target/powerpc/undef-bool.C: New file. > * gcc.target/powerpc/undef-bool.c: New file. I'm not sure two files with the same basename will not cause problems. In either case it is confusing. Please rename one? > --- gcc/config/rs6000/xmmintrin.h (revision 258958) > +++ gcc/config/rs6000/xmmintrin.h (working copy) > @@ -58,6 +58,18 @@ > #define _XMMINTRIN_H_INCLUDED > > #include <altivec.h> > + > +/* Avoid collisions between altivec.h and strict adherence to C++ and > + C11 standards. This should eventually be done inside altivec.h itself, > + but only after testing a full distro build. */ > +#if defined(__STRICT_ANSI__) && (defined(__cplusplus) || \ > + (defined(__STDC_VERSION__) && \ > + __STDC_VERSION__ >= 201112L)) > +#undef vector > +#undef pixel > +#undef bool > +#endif This won't work if anything made a macro for bool (etc.) before including this header. I guess we can live with that for now (it will not work without your patch either). Okay for trunk (with the testcase name fix). Thanks! Segher
On Sun, Apr 01, 2018 at 08:24:32PM -0500, Bill Schmidt wrote: > I also updated the gcc.target/powerpc/powerpc.exp file to allow C++ > tests to be placed in that directory (with a *.C suffix). I think this is wrong. Historically, we've been putting target C++ tests into g++.dg/*/*.C with appropriate target guards, the gcc.target/ tests are compiled with the gcc driver rather than g++, so in your case it just happens to work because it is a compile time test only (and one that doesn't use any STL headers), e.g. dg-do run test would fail miserably. If we (for GCC9?) want to create a spot for target C++ tests, we should just add g++.target/<cpu>/ directories and add all the needed infrastructure for those. Can you please revert the powerpc.exp change and move the test to g++.dg/ext/ ? Thanks. Jakub
> On Apr 4, 2018, at 3:53 AM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Sun, Apr 01, 2018 at 08:24:32PM -0500, Bill Schmidt wrote: >> I also updated the gcc.target/powerpc/powerpc.exp file to allow C++ >> tests to be placed in that directory (with a *.C suffix). > > I think this is wrong. > Historically, we've been putting target C++ tests into g++.dg/*/*.C > with appropriate target guards, the gcc.target/ tests are compiled with the > gcc driver rather than g++, so in your case it just happens to work because > it is a compile time test only (and one that doesn't use any STL headers), > e.g. dg-do run test would fail miserably. > > If we (for GCC9?) want to create a spot for target C++ tests, we should > just add g++.target/<cpu>/ directories and add all the needed infrastructure > for those. > > Can you please revert the powerpc.exp change and move the test to > g++.dg/ext/ ? Thanks. Sure, I can -- I just want to point out that there is precedent here. I noticed that gcc.target/s390/s390.exp allows .C suffixes and there is one such (compile-only) test in that directory, so I assumed the framework was okay for this. Thanks, Bill > > Jakub >
On Wed, Apr 04, 2018 at 03:47:18PM -0500, Bill Schmidt wrote: > > If we (for GCC9?) want to create a spot for target C++ tests, we should > > just add g++.target/<cpu>/ directories and add all the needed infrastructure > > for those. > > > > Can you please revert the powerpc.exp change and move the test to > > g++.dg/ext/ ? Thanks. > > Sure, I can -- I just want to point out that there is precedent here. I noticed Thanks. > that gcc.target/s390/s390.exp allows .C suffixes and there is one such > (compile-only) test in that directory, so I assumed the framework was okay > for this. Yes, and apparently aarch64 and arm have a couple of *.C tests too. Still it doesn't feel right, running C++ tests under make check-gcc rather than check-g++ is just weird. I think we should just introduce g++.target/ in GCC9 and move those tests there, plus any g++.dg/ tests guarded for single targets only. g++.target should do the -std=c++{98,11,14,17,2a} cycling etc. Jakub
On Apr 4, 2018, at 3:51 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Apr 04, 2018 at 03:47:18PM -0500, Bill Schmidt wrote: >>> If we (for GCC9?) want to create a spot for target C++ tests, we should >>> just add g++.target/<cpu>/ directories and add all the needed infrastructure >>> for those. >>> >>> Can you please revert the powerpc.exp change and move the test to >>> g++.dg/ext/ ? Thanks. >> >> Sure, I can -- I just want to point out that there is precedent here. I noticed > > Thanks. > >> that gcc.target/s390/s390.exp allows .C suffixes and there is one such >> (compile-only) test in that directory, so I assumed the framework was okay >> for this. > > Yes, and apparently aarch64 and arm have a couple of *.C tests too. > Still it doesn't feel right, running C++ tests under make check-gcc rather > than check-g++ is just weird. > > I think we should just introduce g++.target/ in GCC9 and move those tests > there, plus any g++.dg/ tests guarded for single targets only. g++.target > should do the -std=c++{98,11,14,17,2a} cycling etc. Agreed. I'll take a note about this for GCC9. Thanks, Bill
Index: gcc/config/rs6000/emmintrin.h =================================================================== --- gcc/config/rs6000/emmintrin.h (revision 258958) +++ gcc/config/rs6000/emmintrin.h (working copy) @@ -885,7 +885,7 @@ _mm_cvtpd_epi32 (__m128d __A) #ifdef _ARCH_PWR8 temp = vec_mergeo (temp, temp); - result = (__v4si)vec_vpkudum ((vector long)temp, (vector long)vzero); + result = (__v4si)vec_vpkudum ((__vector long)temp, (__vector long)vzero); #else { const __v16qu pkperm = {0x00, 0x01, 0x02, 0x03, 0x08, 0x09, 0x0a, 0x0b, @@ -919,7 +919,7 @@ _mm_cvtpd_ps (__m128d __A) #ifdef _ARCH_PWR8 temp = vec_mergeo (temp, temp); - result = (__v4sf)vec_vpkudum ((vector long)temp, (vector long)vzero); + result = (__v4sf)vec_vpkudum ((__vector long)temp, (__vector long)vzero); #else { const __v16qu pkperm = {0x00, 0x01, 0x02, 0x03, 0x08, 0x09, 0x0a, 0x0b, @@ -947,7 +947,7 @@ _mm_cvttpd_epi32 (__m128d __A) #ifdef _ARCH_PWR8 temp = vec_mergeo (temp, temp); - result = (__v4si)vec_vpkudum ((vector long)temp, (vector long)vzero); + result = (__v4si)vec_vpkudum ((__vector long)temp, (__vector long)vzero); #else { const __v16qu pkperm = {0x00, 0x01, 0x02, 0x03, 0x08, 0x09, 0x0a, 0x0b, Index: gcc/config/rs6000/mmintrin.h =================================================================== --- gcc/config/rs6000/mmintrin.h (revision 258958) +++ gcc/config/rs6000/mmintrin.h (working copy) @@ -317,7 +317,7 @@ _mm_unpacklo_pi8 (__m64 __m1, __m64 __m2) a = (__vector unsigned char)vec_splats (__m1); b = (__vector unsigned char)vec_splats (__m2); c = vec_mergel (a, b); - return (__builtin_unpack_vector_int128 ((vector __int128_t)c, 1)); + return (__builtin_unpack_vector_int128 ((__vector __int128_t)c, 1)); #else __m64_union m1, m2, res; Index: gcc/config/rs6000/xmmintrin.h =================================================================== --- gcc/config/rs6000/xmmintrin.h (revision 258958) +++ gcc/config/rs6000/xmmintrin.h (working copy) @@ -58,6 +58,18 @@ #define _XMMINTRIN_H_INCLUDED #include <altivec.h> + +/* Avoid collisions between altivec.h and strict adherence to C++ and + C11 standards. This should eventually be done inside altivec.h itself, + but only after testing a full distro build. */ +#if defined(__STRICT_ANSI__) && (defined(__cplusplus) || \ + (defined(__STDC_VERSION__) && \ + __STDC_VERSION__ >= 201112L)) +#undef vector +#undef pixel +#undef bool +#endif + #include <assert.h> /* We need type definitions from the MMX header file. */ Index: gcc/testsuite/gcc.target/powerpc/powerpc.exp =================================================================== --- gcc/testsuite/gcc.target/powerpc/powerpc.exp (revision 258958) +++ gcc/testsuite/gcc.target/powerpc/powerpc.exp (working copy) @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CFLAGS] then { dg-init # Main loop. -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \ +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \ "" $DEFAULT_CFLAGS set SAVRES_TEST_OPTS [list -Os -O2 {-Os -mno-multiple} {-O2 -mno-multiple}] Index: gcc/testsuite/gcc.target/powerpc/undef-bool.C =================================================================== --- gcc/testsuite/gcc.target/powerpc/undef-bool.C (nonexistent) +++ gcc/testsuite/gcc.target/powerpc/undef-bool.C (working copy) @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -std=c++11 -DNO_WARN_X86_INTRINSICS" } */ + +/* Test to ensure that "bool" gets undef'd in xmmintrin.h when + we require strict ANSI. */ + +#include <xmmintrin.h> + +bool foo (int x) +{ + return x == 2; +} + Index: gcc/testsuite/gcc.target/powerpc/undef-bool.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/undef-bool.c (nonexistent) +++ gcc/testsuite/gcc.target/powerpc/undef-bool.c (working copy) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -std=c11 -DNO_WARN_X86_INTRINSICS" } */ + +/* Test to ensure that "bool" gets undef'd in xmmintrin.h when + we require strict ANSI. Subsequent use of bool needs stdbool.h. + altivec.h should eventually avoid defining bool, vector, and + pixel, following distro testing. */ + +#include <xmmintrin.h> + +bool foo (int x) /* { dg-error "unknown type name 'bool'; did you mean '_Bool'?" } */ +{ + return x == 2; +} +