diff mbox series

testsuite: Fix sse2-andnpd-1.c and sse-andnps-1.c testscases on powerpc

Message ID 20210122190228.GS4020736@tucnak
State New
Headers show
Series testsuite: Fix sse2-andnpd-1.c and sse-andnps-1.c testscases on powerpc | expand

Commit Message

Jakub Jelinek Jan. 22, 2021, 7:02 p.m. UTC 
On Mon, Sep 21, 2020 at 10:12:20AM +0200, Richard Biener wrote:
> On Mon, 21 Sep 2020, Jan Hubicka wrote:
> > these testcases now fails because they contains an invalid type puning
> > that happens via const VALUE_TYPE *v pointer. Since the check function
> > is noinline, modref is needed to trigger the wrong code.
> > I think it is easiest to fix it by no-strict-aliasing.
> > 
> > Regtested x86_64-linux, OK?
> 
> OK.
> 
> > 	* gcc.target/i386/m128-check.h: Add no-strict aliasing to
> > 	CHECK_EXP macro.
> > 
> > diff --git a/gcc/testsuite/gcc.target/i386/m128-check.h b/gcc/testsuite/gcc.target/i386/m128-check.h
> > index 48b23328539..6f414b07be7 100644
> > --- a/gcc/testsuite/gcc.target/i386/m128-check.h
> > +++ b/gcc/testsuite/gcc.target/i386/m128-check.h
> > @@ -78,6 +78,7 @@ typedef union
> >  
> >  #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
> >  static int						\
> > +__attribute__((optimize ("no-strict-aliasing")))	\
> >  __attribute__((noinline, unused))			\
> >  check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
> >  {							\

On powerpc64le the tests suffer from the exact same issue.

Tested on powerpc64le-linux, ok for trunk?

2021-01-22  Jakub Jelinek  <jakub@redhat.com>

	* gcc.target/powerpc/m128-check.h (check_##UINON_TYPE): Add
	optimize ("no-strict-aliasing") attribute.



	Jakub

Comments

Richard Biener Jan. 22, 2021, 7:17 p.m. UTC | #1 
On January 22, 2021 8:02:28 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Mon, Sep 21, 2020 at 10:12:20AM +0200, Richard Biener wrote:
>> On Mon, 21 Sep 2020, Jan Hubicka wrote:
>> > these testcases now fails because they contains an invalid type
>puning
>> > that happens via const VALUE_TYPE *v pointer. Since the check
>function
>> > is noinline, modref is needed to trigger the wrong code.
>> > I think it is easiest to fix it by no-strict-aliasing.
>> > 
>> > Regtested x86_64-linux, OK?
>> 
>> OK.
>> 
>> > 	* gcc.target/i386/m128-check.h: Add no-strict aliasing to
>> > 	CHECK_EXP macro.
>> > 
>> > diff --git a/gcc/testsuite/gcc.target/i386/m128-check.h
>b/gcc/testsuite/gcc.target/i386/m128-check.h
>> > index 48b23328539..6f414b07be7 100644
>> > --- a/gcc/testsuite/gcc.target/i386/m128-check.h
>> > +++ b/gcc/testsuite/gcc.target/i386/m128-check.h
>> > @@ -78,6 +78,7 @@ typedef union
>> >  
>> >  #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
>> >  static int						\
>> > +__attribute__((optimize ("no-strict-aliasing")))	\
>> >  __attribute__((noinline, unused))			\
>> >  check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
>> >  {							\
>
>On powerpc64le the tests suffer from the exact same issue.
>
>Tested on powerpc64le-linux, ok for trunk?

Ok. 

Richard. 

>2021-01-22  Jakub Jelinek  <jakub@redhat.com>
>
>	* gcc.target/powerpc/m128-check.h (check_##UINON_TYPE): Add
>	optimize ("no-strict-aliasing") attribute.
>
>--- gcc/testsuite/gcc.target/powerpc/m128-check.h
>+++ gcc/testsuite/gcc.target/powerpc/m128-check.h
>@@ -85,6 +85,7 @@ typedef union
> 
> #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
> static int						\
>+__attribute__((optimize ("no-strict-aliasing")))	\
> __attribute__((noinline, unused))			\
> check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
> {							\
>
>
>	Jakub
Segher Boessenkool Jan. 23, 2021, 12:56 a.m. UTC | #2 
Hi!

On Fri, Jan 22, 2021 at 08:02:28PM +0100, Jakub Jelinek wrote:
> On Mon, Sep 21, 2020 at 10:12:20AM +0200, Richard Biener wrote:
> > On Mon, 21 Sep 2020, Jan Hubicka wrote:
> > > these testcases now fails because they contains an invalid type puning
> > > that happens via const VALUE_TYPE *v pointer. Since the check function
> > > is noinline, modref is needed to trigger the wrong code.
> > > I think it is easiest to fix it by no-strict-aliasing.

> > > diff --git a/gcc/testsuite/gcc.target/i386/m128-check.h b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > index 48b23328539..6f414b07be7 100644
> > > --- a/gcc/testsuite/gcc.target/i386/m128-check.h
> > > +++ b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > @@ -78,6 +78,7 @@ typedef union
> > >  
> > >  #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
> > >  static int						\
> > > +__attribute__((optimize ("no-strict-aliasing")))	\
> > >  __attribute__((noinline, unused))			\
> > >  check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
> > >  {							\
> 
> On powerpc64le the tests suffer from the exact same issue.
> 
> Tested on powerpc64le-linux, ok for trunk?

So what is the actual error here?  This whole union stuff is because we
*do* want proper aliasing, afaics.


Segher


> --- gcc/testsuite/gcc.target/powerpc/m128-check.h
> +++ gcc/testsuite/gcc.target/powerpc/m128-check.h
> @@ -85,6 +85,7 @@ typedef union
>  
>  #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
>  static int						\
> +__attribute__((optimize ("no-strict-aliasing")))	\
>  __attribute__((noinline, unused))			\
>  check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
>  {							\
Jakub Jelinek Jan. 23, 2021, 8:41 a.m. UTC | #3 
On Fri, Jan 22, 2021 at 06:56:37PM -0600, Segher Boessenkool wrote:
> On Fri, Jan 22, 2021 at 08:02:28PM +0100, Jakub Jelinek wrote:
> > On Mon, Sep 21, 2020 at 10:12:20AM +0200, Richard Biener wrote:
> > > On Mon, 21 Sep 2020, Jan Hubicka wrote:
> > > > these testcases now fails because they contains an invalid type puning
> > > > that happens via const VALUE_TYPE *v pointer. Since the check function
> > > > is noinline, modref is needed to trigger the wrong code.
> > > > I think it is easiest to fix it by no-strict-aliasing.
> 
> > > > diff --git a/gcc/testsuite/gcc.target/i386/m128-check.h b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > index 48b23328539..6f414b07be7 100644
> > > > --- a/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > +++ b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > @@ -78,6 +78,7 @@ typedef union
> > > >  
> > > >  #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
> > > >  static int						\
> > > > +__attribute__((optimize ("no-strict-aliasing")))	\
> > > >  __attribute__((noinline, unused))			\
> > > >  check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
> > > >  {							\
> > 
> > On powerpc64le the tests suffer from the exact same issue.
> > 
> > Tested on powerpc64le-linux, ok for trunk?
> 
> So what is the actual error here?  This whole union stuff is because we
> *do* want proper aliasing, afaics.

The reading through union is not where the problem is, the problem
was in the reading through VALUE_TYPE, because the testcase does:
  int e[4];
...
  e[0] = (~source1[0]) & source2[0];
  e[1] = (~source1[1]) & source2[1];
  e[2] = (~source1[2]) & source2[2];
  e[3] = (~source1[3]) & source2[3];

  if (check_union128 (u, (float *)e))

So check_union128 reads through VALUE_TYPE of float, but the variable
has dynamic type int[4].  Even making a e union of int[4] and float[4]
and passing address of the float field in it wouldn't be correct,
but e.g. having another variable with float[4] type and memcpying
e into it would work too.  So:
  int e[4];
  float f[4];
...
  e[0] = (~source1[0]) & source2[0];
  e[1] = (~source1[1]) & source2[1];
  e[2] = (~source1[2]) & source2[2];
  e[3] = (~source1[3]) & source2[3];
  memcpy (f, e, sizeof (e));

  if (check_union128 (u, f))

The reason I chose the "no-strict-aliasing" attribute (and already
committed based on Richi's ack) was consistency with the i386
testcase.  I can change both.

	Jakub
Segher Boessenkool Jan. 23, 2021, 9:10 p.m. UTC | #4 
Hi!

On Sat, Jan 23, 2021 at 09:41:23AM +0100, Jakub Jelinek wrote:
> On Fri, Jan 22, 2021 at 06:56:37PM -0600, Segher Boessenkool wrote:
> > So what is the actual error here?  This whole union stuff is because we
> > *do* want proper aliasing, afaics.
> 
> The reading through union is not where the problem is, the problem
> was in the reading through VALUE_TYPE, because the testcase does:
>   int e[4];
> ...
>   e[0] = (~source1[0]) & source2[0];
>   e[1] = (~source1[1]) & source2[1];
>   e[2] = (~source1[2]) & source2[2];
>   e[3] = (~source1[3]) & source2[3];
> 
>   if (check_union128 (u, (float *)e))

Right, that should use such a union as well, or similar.  Most of the
other similar mechanics in the testcase are because we *do* want proper
alising rules, so it would be a shame to throw it all away now.

> So check_union128 reads through VALUE_TYPE of float, but the variable
> has dynamic type int[4].  Even making a e union of int[4] and float[4]
> and passing address of the float field in it wouldn't be correct,
> but e.g. having another variable with float[4] type and memcpying
> e into it would work too.  So:
>   int e[4];
>   float f[4];
> ...
>   e[0] = (~source1[0]) & source2[0];
>   e[1] = (~source1[1]) & source2[1];
>   e[2] = (~source1[2]) & source2[2];
>   e[3] = (~source1[3]) & source2[3];
>   memcpy (f, e, sizeof (e));
> 
>   if (check_union128 (u, f))

That, or have the function take a pointer-to-union, or make "e" a union
itself.  All should work.

> The reason I chose the "no-strict-aliasing" attribute (and already
> committed based on Richi's ack) was consistency with the i386
> testcase.  I can change both.

Yes please.  Thanks!


Segher
diff mbox series

Patch

--- gcc/testsuite/gcc.target/powerpc/m128-check.h
+++ gcc/testsuite/gcc.target/powerpc/m128-check.h
@@ -85,6 +85,7 @@  typedef union
 
 #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT)		\
 static int						\
+__attribute__((optimize ("no-strict-aliasing")))	\
 __attribute__((noinline, unused))			\
 check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v)	\
 {							\