[PATCH] rs6000: Fix up __m64 typedef in mmintrin.h [PR97301]

[Jakub Jelinek]

On Fri, Jan 22, 2021 at 05:45:54PM -0600, Segher Boessenkool wrote:
> On Fri, Jan 22, 2021 at 07:02:04PM +0100, Jakub Jelinek wrote:
> > The x86 __m64 type is defined as:
> > /* The Intel API is flexible enough that we must allow aliasing with other
> >    vector types, and their scalar components.  */
> > typedef int __m64 __attribute__ ((__vector_size__ (8), __may_alias__));
> > and so matches the comment above it in that reads and stores through
> > pointers to __m64 can alias anything.
> > But in the rs6000 headers that is the case only for __m128, but not __m64.
> 
> We define __m64 as an integer type, because we do not have 8-byte
> vectors.  There is __m64_union to do conversions.

It being an integer type is not the problem.

> > The following patch adds that attribute, which fixes the
> > FAIL: gcc.target/powerpc/sse-movhps-1.c execution test
> > FAIL: gcc.target/powerpc/sse-movlps-1.c execution test
> > regressions that appeared when Honza improved ipa-modref.
> 
> What is the actual error there?  It sounds like something forgot to
> pass something through __m64_union.

The problem is that the testcase uses the
_mm_loadl_pi
API, and per the Intel intrinsic rules it is ok when that intrinsic
loads from wide range of types, e.g. including pairs of integers or
4 shorts or 8 chars or pair of floats.
And that works fine on x86 because it uses may_alias attribute on __m64
(like on pretty much all the other __m* types).
It doesn't work on ppc64le, because there __m64 is just unsigned long long
without may_alias, which means per the C aliasing rules it can load only
from object with dynamic type of long long or unsigned long long, but
not from what I've cited above.
And Honza's modref IPA stuff without that attribute results in optimizing
out the initializers.

So, if you look at the GIMPLE dumps and compare r11-3430 (before Honza's
change) and r11-3434, the difference is:
   <bb 2> [local count: 1073741824]:
-  d[0] = 2.443000030517578125e+1;
-  d[1] = 6.834600067138671875e+1;
   _2 = test.constprop (&d);
because the alias oracle fed with modref details figures out that the d
stores are using a type that isn't used to load it in the test's inlined
_mm_loadl_pi.  Adding the may_alias attribute fixes it, the d initializers
are kept.
If you look at the comment right above __m64 typedef, it explains why
it needs the attribute.

> > 2021-01-22  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR testsuite/97301
> > 	* config/rs6000/mmintrin.h (__m64): Add __may_alias__ attribute.
> > 
> > --- gcc/config/rs6000/mmintrin.h.jj	2021-01-04 10:25:46.794143679 +0100
> > +++ gcc/config/rs6000/mmintrin.h	2021-01-22 13:03:28.511043929 +0100
> > @@ -58,7 +58,8 @@
> >  #include <altivec.h>
> >  /* The Intel API is flexible enough that we must allow aliasing with other
> >     vector types, and their scalar components.  */
> > -typedef __attribute__ ((__aligned__ (8))) unsigned long long __m64;
> > +typedef __attribute__ ((__aligned__ (8),
> > +			__may_alias__)) unsigned long long __m64;
> >  
> >  typedef __attribute__ ((__aligned__ (8)))
> >  union

	Jakub