On Wed, 2017-07-19 at 16:42 -0500, Segher Boessenkool wrote:
> Hi Steve,
>
> On Wed, Jul 19, 2017 at 10:14:01AM -0500, Steven Munroe wrote:
> > This it part 2/2 for contributing PPC64LE support for X86 MMX
> > instrisics. This patch adds the DG tests to verify the headers contents.
> > Oddly there are very few MMX specific included in i386 so I had to adapt
> > some the SSE tested to smaller vector size.
>
> Juat two comments...
>
> > + /* Many MMX intrinsics are simpler / faster to implement by
> > + * transferring the __m64 (long int) to vector registers for SIMD
> > + * operations. To be efficient we also need the direct register
> > + * transfer instructions from POWER8. So we can test for
> > + * arch_2_07. */
>
> We don't use leading * in block comments. Not that I care in test
> cases, but you seem to be following the coding standards otherwise :-)
>
This is Eclipse CDT GNU format-er. Seems it is acceptable, most of the
time.
I will try to convince it not to add the leading * in the future. For
now I'll fix the comment manually before I commit.
> > --- gcc/testsuite/gcc.target/powerpc/mmx-packs.c (nonexistent)
> > +++ gcc/testsuite/gcc.target/powerpc/mmx-packs.c (working copy)
> > @@ -0,0 +1,91 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-O3 -mpower8-vector" } */
> > +/* { dg-require-effective-target lp64 } */
> > +/* { dg-require-effective-target p8vector_hw { target powerpc*-*-* } }
> > */
>
> Why have the target selector here, and not on the dg-options line as
> well? Don't we need it in both places, or neither? (I think you don't
> need it, same for all other files here).
>
I was backed into this because we dont have the
/* (dg-require-effective-target p8vector_min } */
yet. And we don't want to use -mcpu=power8 if we mean power8 or power9
and later.
The { target powerpc*-*-* } bit is there to enable possible future
sharing of DG tests across platforms. So I should either remove the
target selector or add it any line that the platform specific? For
example:
/* { dg-options "-O3 -mpower8-vector" { target powerpc*-*-* } } */
If you agree with the above, I will correct and commit.
