diff mbox

[rs6000] Change -mcrypto to only affect Category:Vector.Crypto instructions

Message ID CAGWvny=drz+vQTkuJ3MGenkG9+tKwsfUjFzrQcrdYDOtT_rs6Q@mail.gmail.com
State New
Headers show

Commit Message

David Edelsohn April 29, 2015, 1:21 p.m. UTC
On Wed, Mar 4, 2015 at 3:14 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> I recently observed that -mno-crypto disables all instructions in
> section 5.11 of the 2.07 ISA, rather than just those flagged as
> Category:Vector.Crypto.  This patch fixes that undesirable situation.
>
> The main fix is to ensure the remaining instructions are gated by
> TARGET_P8_VECTOR rather than TARGET_CRYPTO.  This leaves us in a
> somewhat ugly state where we have builtins named __builtin_crypto_* that
> are not controlled by -mcrypto.  However, we have to keep support for
> these existing builtins.  As discussed elsewhere, the longer-term plan
> is to implement a different common naming scheme for these builtins
> across all POWER compilers, at which point the __builtin_crypto_* forms
> will be deprecated.
>
> The changes to rs6000-builtin.def aren't the prettiest in the world, but
> were the best I could think of that continues support for the existing
> builtins while changing their predicates.  Let me know if there's a
> better way.
>
> Ok for trunk once GCC 5 branches?  I would eventually like to fix this
> in 4.8, 4.9, and 5 as well.

> Index: gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c (working copy)
> @@ -0,0 +1,36 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
> +/* { dg-options "-O2 -mcpu=power8 -mno-crypto" } */

Bill,

I think this test criterion is wrong and should be


Thoughts?

- David

Comments

Bill Schmidt April 30, 2015, 9:15 p.m. UTC | #1
Hi David,

I agree, and I'll fix the test as described and backport everywhere,
assuming I don't encounter any problems.  Thanks for catching this!

Bill

On Wed, 2015-04-29 at 09:21 -0400, David Edelsohn wrote:
> On Wed, Mar 4, 2015 at 3:14 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > I recently observed that -mno-crypto disables all instructions in
> > section 5.11 of the 2.07 ISA, rather than just those flagged as
> > Category:Vector.Crypto.  This patch fixes that undesirable situation.
> >
> > The main fix is to ensure the remaining instructions are gated by
> > TARGET_P8_VECTOR rather than TARGET_CRYPTO.  This leaves us in a
> > somewhat ugly state where we have builtins named __builtin_crypto_* that
> > are not controlled by -mcrypto.  However, we have to keep support for
> > these existing builtins.  As discussed elsewhere, the longer-term plan
> > is to implement a different common naming scheme for these builtins
> > across all POWER compilers, at which point the __builtin_crypto_* forms
> > will be deprecated.
> >
> > The changes to rs6000-builtin.def aren't the prettiest in the world, but
> > were the best I could think of that continues support for the existing
> > builtins while changing their predicates.  Let me know if there's a
> > better way.
> >
> > Ok for trunk once GCC 5 branches?  I would eventually like to fix this
> > in 4.8, 4.9, and 5 as well.
> 
> > Index: gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c
> > ===================================================================
> > --- gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c (revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/crypto-builtin-2.c (working copy)
> > @@ -0,0 +1,36 @@
> > +/* { dg-do compile { target { powerpc*-*-* } } } */
> > +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> > +/* { dg-require-effective-target powerpc_vsx_ok } */
> > +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
> > +/* { dg-options "-O2 -mcpu=power8 -mno-crypto" } */
> 
> Bill,
> 
> I think this test criterion is wrong and should be
> 
> Index: crypto-builtin-2.c
> ===================================================================
> --- crypto-builtin-2.c  (revision 222534)
> +++ crypto-builtin-2.c  (working copy)
> @@ -1,6 +1,6 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
>  /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
> -/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
>  /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*"
> } { "-mcpu=power8" } } */
>  /* { dg-options "-O2 -mcpu=power8 -mno-crypto" } */
> 
> Thoughts?
> 
> - David
>
diff mbox

Patch

Index: crypto-builtin-2.c
===================================================================
--- crypto-builtin-2.c  (revision 222534)
+++ crypto-builtin-2.c  (working copy)
@@ -1,6 +1,6 @@ 
 /* { dg-do compile { target { powerpc*-*-* } } } */
 /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
-/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*"
} { "-mcpu=power8" } } */
 /* { dg-options "-O2 -mcpu=power8 -mno-crypto" } */