[RFA] Require powerpc_vsx_ok in gcc.target/powerpc/pr71763.c

Message ID 20200417234947.10209-1-brobecker@adacore.com
State New
Headers show
Series
  • [RFA] Require powerpc_vsx_ok in gcc.target/powerpc/pr71763.c
Related show

Commit Message

Joel Brobecker April 17, 2020, 11:49 p.m.
From: Douglas Rupp <rupp@adacore.com>


Hello,

(submitting this on behalf of Doug Rupp, one of my colleagues)

We're getting an error when running this test on PowerPC VxWorks 7,
due to an unexpected warning:

    | Excess errors:
    | cc1: warning: '-mvsx' and '-mno-altivec' are incompatible

The warning comes from a combination of factors:
  - The test itself uses -mvsx explicitly via the following directive:
       // { dg-options "-O1 -mvsx" }
  - Our toolchain was configured so as to make -mno-altivec
    the default;
  - These two options are mutually exclusive.

This commit adds a powerpc_vsx_ok dg-require-effective-target directive
to that test, and thus making it UNSUPPORTED instead.

Tested on PowerPC VxWorks 7. Also tested on PowerPC ELF as well,
a platform where we do not make -mno-altivec the default, to verify
that the test continues to run as usual in that case.

gcc/testsuite/

        * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.

OK for master?

Thanks!
-- 
Joel

---
 gcc/testsuite/gcc.target/powerpc/pr71763.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.17.1

Comments

Joel Brobecker May 1, 2020, 2:26 p.m. | #1
Hello,

Just a friendly ping on the following patch, hopefully sufficiently
straightforward and tested to be allowed onto branch master.

Thank you!


On Fri, Apr 17, 2020 at 04:49:47PM -0700, Joel Brobecker wrote:
> From: Douglas Rupp <rupp@adacore.com>

> 

> Hello,

> 

> (submitting this on behalf of Doug Rupp, one of my colleagues)

> 

> We're getting an error when running this test on PowerPC VxWorks 7,

> due to an unexpected warning:

> 

>     | Excess errors:

>     | cc1: warning: '-mvsx' and '-mno-altivec' are incompatible

> 

> The warning comes from a combination of factors:

>   - The test itself uses -mvsx explicitly via the following directive:

>        // { dg-options "-O1 -mvsx" }

>   - Our toolchain was configured so as to make -mno-altivec

>     the default;

>   - These two options are mutually exclusive.

> 

> This commit adds a powerpc_vsx_ok dg-require-effective-target directive

> to that test, and thus making it UNSUPPORTED instead.

> 

> Tested on PowerPC VxWorks 7. Also tested on PowerPC ELF as well,

> a platform where we do not make -mno-altivec the default, to verify

> that the test continues to run as usual in that case.

> 

> gcc/testsuite/

> 

>         * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.

> 

> OK for master?

> 

> Thanks!

> -- 

> Joel

> 

> ---

>  gcc/testsuite/gcc.target/powerpc/pr71763.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> index b36ddfa26b0..b8888394393 100644

> --- a/gcc/testsuite/gcc.target/powerpc/pr71763.c

> +++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> @@ -1,5 +1,6 @@

>  // PR target/71763

>  // { dg-do compile }

> +// { dg-require-effective-target powerpc_vsx_ok }

>  // { dg-options "-O1 -mvsx" }

>  

>  int a, b;

> -- 

> 2.17.1


-- 
Joel
Joel Brobecker May 13, 2020, 4:22 p.m. | #2
Hello,

Would someone mind reviewing this patch, please?

The test explicitly uses -mvsx in the compilation options, so it seems
reasonable to require powerpc_vsx_ok...

Thank you!

> Just a friendly ping on the following patch, hopefully sufficiently

> straightforward and tested to be allowed onto branch master.

> 

> 

> On Fri, Apr 17, 2020 at 04:49:47PM -0700, Joel Brobecker wrote:

> > From: Douglas Rupp <rupp@adacore.com>

> > 

> > Hello,

> > 

> > (submitting this on behalf of Doug Rupp, one of my colleagues)

> > 

> > We're getting an error when running this test on PowerPC VxWorks 7,

> > due to an unexpected warning:

> > 

> >     | Excess errors:

> >     | cc1: warning: '-mvsx' and '-mno-altivec' are incompatible

> > 

> > The warning comes from a combination of factors:

> >   - The test itself uses -mvsx explicitly via the following directive:

> >        // { dg-options "-O1 -mvsx" }

> >   - Our toolchain was configured so as to make -mno-altivec

> >     the default;

> >   - These two options are mutually exclusive.

> > 

> > This commit adds a powerpc_vsx_ok dg-require-effective-target directive

> > to that test, and thus making it UNSUPPORTED instead.

> > 

> > Tested on PowerPC VxWorks 7. Also tested on PowerPC ELF as well,

> > a platform where we do not make -mno-altivec the default, to verify

> > that the test continues to run as usual in that case.

> > 

> > gcc/testsuite/

> > 

> >         * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.

> > 

> > OK for master?

> > 

> > Thanks!

> > -- 

> > Joel

> > 

> > ---

> >  gcc/testsuite/gcc.target/powerpc/pr71763.c | 1 +

> >  1 file changed, 1 insertion(+)

> > 

> > diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> > index b36ddfa26b0..b8888394393 100644

> > --- a/gcc/testsuite/gcc.target/powerpc/pr71763.c

> > +++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> > @@ -1,5 +1,6 @@

> >  // PR target/71763

> >  // { dg-do compile }

> > +// { dg-require-effective-target powerpc_vsx_ok }

> >  // { dg-options "-O1 -mvsx" }

> >  

> >  int a, b;

> > -- 

> > 2.17.1

> 

> -- 

> Joel


-- 
Joel
Christophe Lyon via Gcc-patches May 16, 2020, 2:46 p.m. | #3
On 5/13/20 11:22 AM, Joel Brobecker wrote:
> Hello,

> 

> Would someone mind reviewing this patch, please?

> 

> The test explicitly uses -mvsx in the compilation options, so it seems

> reasonable to require powerpc_vsx_ok...

> 

> Thank you!



You should always CC the PPC maintainers on PPC patches.
I've CC'd both Segher and David.

Peter



>> Just a friendly ping on the following patch, hopefully sufficiently

>> straightforward and tested to be allowed onto branch master.

>>

>>

>> On Fri, Apr 17, 2020 at 04:49:47PM -0700, Joel Brobecker wrote:

>>> From: Douglas Rupp <rupp@adacore.com>

>>>

>>> Hello,

>>>

>>> (submitting this on behalf of Doug Rupp, one of my colleagues)

>>>

>>> We're getting an error when running this test on PowerPC VxWorks 7,

>>> due to an unexpected warning:

>>>

>>>     | Excess errors:

>>>     | cc1: warning: '-mvsx' and '-mno-altivec' are incompatible

>>>

>>> The warning comes from a combination of factors:

>>>   - The test itself uses -mvsx explicitly via the following directive:

>>>        // { dg-options "-O1 -mvsx" }

>>>   - Our toolchain was configured so as to make -mno-altivec

>>>     the default;

>>>   - These two options are mutually exclusive.

>>>

>>> This commit adds a powerpc_vsx_ok dg-require-effective-target directive

>>> to that test, and thus making it UNSUPPORTED instead.

>>>

>>> Tested on PowerPC VxWorks 7. Also tested on PowerPC ELF as well,

>>> a platform where we do not make -mno-altivec the default, to verify

>>> that the test continues to run as usual in that case.

>>>

>>> gcc/testsuite/

>>>

>>>         * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.

>>>

>>> OK for master?

>>>

>>> Thanks!

>>> -- 

>>> Joel

>>>

>>> ---

>>>  gcc/testsuite/gcc.target/powerpc/pr71763.c | 1 +

>>>  1 file changed, 1 insertion(+)

>>>

>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c b/gcc/testsuite/gcc.target/powerpc/pr71763.c

>>> index b36ddfa26b0..b8888394393 100644

>>> --- a/gcc/testsuite/gcc.target/powerpc/pr71763.c

>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c

>>> @@ -1,5 +1,6 @@

>>>  // PR target/71763

>>>  // { dg-do compile }

>>> +// { dg-require-effective-target powerpc_vsx_ok }

>>>  // { dg-options "-O1 -mvsx" }

>>>  

>>>  int a, b;

>>> -- 

>>> 2.17.1

>>

>> -- 

>> Joel

>
Segher Boessenkool May 16, 2020, 5:24 p.m. | #4
Hi!

On Sat, May 16, 2020 at 09:46:15AM -0500, Peter Bergner wrote:
> On 5/13/20 11:22 AM, Joel Brobecker wrote:

> > Would someone mind reviewing this patch, please?


> You should always CC the PPC maintainers on PPC patches.

> I've CC'd both Segher and David.


Thanks Peter.  Yes, I hadn't seen this patch yet.

> >> On Fri, Apr 17, 2020 at 04:49:47PM -0700, Joel Brobecker wrote:

> >>> From: Douglas Rupp <rupp@adacore.com>


> >>> We're getting an error when running this test on PowerPC VxWorks 7,

> >>> due to an unexpected warning:

> >>>

> >>>     | Excess errors:

> >>>     | cc1: warning: '-mvsx' and '-mno-altivec' are incompatible

> >>>

> >>> The warning comes from a combination of factors:

> >>>   - The test itself uses -mvsx explicitly via the following directive:

> >>>        // { dg-options "-O1 -mvsx" }

> >>>   - Our toolchain was configured so as to make -mno-altivec

> >>>     the default;

> >>>   - These two options are mutually exclusive.


If the CPU does not allow -mvsx, this should just fail, saying that.  If
the CPU does allow -mvsx, it should override the earlier option, so you
should end up with everything enabled.

So it sounds like we have some problems here :-/

> >>> This commit adds a powerpc_vsx_ok dg-require-effective-target directive

> >>> to that test, and thus making it UNSUPPORTED instead.


> >>>         * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.


Ah, I see.

> >>> OK for master?


Yes.  Also okay for backports (8, 9, 10).

Thanks!

(Don't forget the changelog?)


Segher


> >>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> >>> index b36ddfa26b0..b8888394393 100644

> >>> --- a/gcc/testsuite/gcc.target/powerpc/pr71763.c

> >>> +++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c

> >>> @@ -1,5 +1,6 @@

> >>>  // PR target/71763

> >>>  // { dg-do compile }

> >>> +// { dg-require-effective-target powerpc_vsx_ok }

> >>>  // { dg-options "-O1 -mvsx" }

> >>>  

> >>>  int a, b;
Joel Brobecker May 18, 2020, 6:54 p.m. | #5
Hi Peter and Segher,

> > You should always CC the PPC maintainers on PPC patches.

> > I've CC'd both Segher and David.


Thanks a lot for doing that. And well understood for future patches.

> > >>> This commit adds a powerpc_vsx_ok dg-require-effective-target directive

> > >>> to that test, and thus making it UNSUPPORTED instead.

> 

> > >>>         * gcc.target/powerpc/pr71763.c: Require powerpc_vsx_ok.

> > >>> OK for master?

> 

> Yes.  Also okay for backports (8, 9, 10).

> 

> Thanks!

> 

> (Don't forget the changelog?)


Thank you, Segher. Pushed to master, and for 8, 9 and 10.

I did include the ChangeLog indeed. I tend to avoid sending them
in the diff, because they usually don't apply out of the box. I know
there are some ways to resolve the conflicts automatically. I haven't
gotten around to setting that up, but if this is causing any confusion
here, I'll make sure to include the ChangeLog diff in the patch as well.

Thanks again!
-- 
Joel
Segher Boessenkool May 18, 2020, 9:06 p.m. | #6
Hi!

On Mon, May 18, 2020 at 11:54:31AM -0700, Joel Brobecker wrote:
> > (Don't forget the changelog?)

> 

> Thank you, Segher. Pushed to master, and for 8, 9 and 10.


Thanks!

> I did include the ChangeLog indeed. I tend to avoid sending them

> in the diff, because they usually don't apply out of the box. I know

> there are some ways to resolve the conflicts automatically. I haven't

> gotten around to setting that up, but if this is causing any confusion

> here, I'll make sure to include the ChangeLog diff in the patch as well.


Well, you *never* should have had the changelog as part of the patch...
This has been the policy since forever.  The usual place to put it was
as text between your introductory message and the patch.  With the new
changelog policies that still holds: you no longer will patch and check
in the changelog files, but you will put the changelog at the end of
your commit message, and the rest is magic.  If I understand it all
correctly :-)  We'll hear details soon enough I bet :-)


Segher

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/pr71763.c b/gcc/testsuite/gcc.target/powerpc/pr71763.c
index b36ddfa26b0..b8888394393 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr71763.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr71763.c
@@ -1,5 +1,6 @@ 
 // PR target/71763
 // { dg-do compile }
+// { dg-require-effective-target powerpc_vsx_ok }
 // { dg-options "-O1 -mvsx" }
 
 int a, b;