[patch] Fix thinko in powerpc default specs for -mabi

[patch] Fix thinko in powerpc default specs for -mabi

[Olivier Hainque]

[-- Attachment #1: Type: text/plain, Size: 1486 bytes --]

Hello,

For a powerpc compiler configured with --with-abi=elfv2, an explicit
-mabi option other than elfv1 fails to override the default.

For example, after

  [...]/configure --enable-languages=c --target=powerpc-elf --with-abi=elfv2
  make all-gcc

This command:

  ./gcc/xgcc -B./gcc/ t.c -mabi=ieeelongdouble -v

issues:

  ./gcc/cc1 [...] t.c -mabi=ieeelongdouble -mabi=elfv2

elfv2 overrides the user request here, which I think
is not as intended.

This is controlled by OPTION_DEFAULT_SPECS, where we have

  {"abi", "%{!mabi=elfv*:-mabi=%(VALUE)}" },

From https://gcc.gnu.org/pipermail/gcc-patches/2013-November/375042.html
I don't see an explicit reason to trigger only on elfv* . It just looks
like an oversight with a focus on elfv1 vs elfv2 at the time.

The attached patch is a proposal to correct this, simply removing the
"elfv" prefix from the spec and adding the corresponding description
to the block comment just above.

We have been using this for about a year now in gcc-11 based toolchains.
This helps our dejagnu testsuite runs for VxWorks on powerpc and 
hasn't produced any ill side effect to date.

The patch also bootstraps and regtests fine on powerpc64-linux.

Is this OK to commit?

Thanks in advance!

With Kind Regards,

Olivier

2022-09-14  Olivier Hainque  <hainque@adacore.com>

        * config/rs6000/option-defaults.h (OPTION_DEFAULT_SPECS):
        Have any -mabi, not only -mabi=elfv*, override the --with-abi
        configuration default.



[-- Attachment #2: with-abi.diff --]
[-- Type: text/x-diff, Size: 1475 bytes --]

commit 33933796b777591007c04448860e781ac17b9070
Author:     Olivier Hainque <hainque@adacore.com>
AuthorDate: Thu Apr 21 14:44:47 2022 +0000
Commit:     Olivier Hainque <hainque@adacore.com>
CommitDate: Thu Apr 21 14:47:37 2022 +0000

    Fix thinko in --with-abi processing on powerpc
    
    Make it so any -mabi overrides what --with-abi requests
    as a default, not only -mabi=elfv*.
    
    Part of V415-021 (-mabi overrides on powerpc)
    
    Change-Id: I62763dee62bbbd7d446f2dd091017d0c7e719cab

diff --git a/gcc/config/rs6000/option-defaults.h b/gcc/config/rs6000/option-defaults.h
index 7ebd115755a..ecf246e6b2e 100644
--- a/gcc/config/rs6000/option-defaults.h
+++ b/gcc/config/rs6000/option-defaults.h
@@ -47,6 +47,7 @@
 #endif
 
 /* Support for a compile-time default CPU, et cetera.  The rules are:
+   --with-abi is ignored if -mabi is specified.
    --with-cpu is ignored if -mcpu is specified; likewise --with-cpu-32
      and --with-cpu-64.
    --with-tune is ignored if -mtune or -mcpu is specified; likewise
@@ -54,7 +55,7 @@
    --with-float is ignored if -mhard-float or -msoft-float are
      specified.  */
 #define OPTION_DEFAULT_SPECS \
-  {"abi", "%{!mabi=elfv*:-mabi=%(VALUE)}" }, \
+  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" }, \
   {"tune", "%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}" }, \
   {"tune_32", "%{" OPT_ARCH32 ":%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}}" }, \
   {"tune_64", "%{" OPT_ARCH64 ":%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}}" }, \

Re: [patch] Fix thinko in powerpc default specs for -mabi

[Segher Boessenkool]

Hi!

On Fri, Sep 23, 2022 at 11:49:24AM -0400, Olivier Hainque wrote:
> For a powerpc compiler configured with --with-abi=elfv2, an explicit
> -mabi option other than elfv1 fails to override the default.
> 
> For example, after
> 
>   [...]/configure --enable-languages=c --target=powerpc-elf --with-abi=elfv2
>   make all-gcc
> 
> This command:
> 
>   ./gcc/xgcc -B./gcc/ t.c -mabi=ieeelongdouble -v
> 
> issues:
> 
>   ./gcc/cc1 [...] t.c -mabi=ieeelongdouble -mabi=elfv2
> 
> elfv2 overrides the user request here, which I think
> is not as intended.

-mabi= does two separate things, unfortunately.

First, you can use it to set the base ABI: elfv1, elfv2.  But you can
also use it to set ABI variants, ABI options: -mabi={no-,}altivec,
-mabi={ieee,ibm}longdouble, -mabi=vec-{extabi,default}.  Things in that
latter category are completely orthogonal to anything else (except that
some only make sense together with some base ABIs).

Base ABI is not selectable for most, it is implied by your target
triple.  -mabi=elfv[12] only makes sense for targets that have either of
the two by default.

> This is controlled by OPTION_DEFAULT_SPECS, where we have
> 
>   {"abi", "%{!mabi=elfv*:-mabi=%(VALUE)}" },
> 
> >From https://gcc.gnu.org/pipermail/gcc-patches/2013-November/375042.html
> I don't see an explicit reason to trigger only on elfv* . It just looks
> like an oversight with a focus on elfv1 vs elfv2 at the time.
> 
> The attached patch is a proposal to correct this, simply removing the
> "elfv" prefix from the spec and adding the corresponding description
> to the block comment just above.
> 
> We have been using this for about a year now in gcc-11 based toolchains.
> This helps our dejagnu testsuite runs for VxWorks on powerpc and 
> hasn't produced any ill side effect to date.

So what exactly is this meant to do?

> 2022-09-14  Olivier Hainque  <hainque@adacore.com>
> 
>         * config/rs6000/option-defaults.h (OPTION_DEFAULT_SPECS):
>         Have any -mabi, not only -mabi=elfv*, override the --with-abi
>         configuration default.

>  /* Support for a compile-time default CPU, et cetera.  The rules are:
> +   --with-abi is ignored if -mabi is specified.
>     --with-cpu is ignored if -mcpu is specified; likewise --with-cpu-32
>       and --with-cpu-64.
>     --with-tune is ignored if -mtune or -mcpu is specified; likewise
> @@ -54,7 +55,7 @@
>     --with-float is ignored if -mhard-float or -msoft-float are
>       specified.  */
>  #define OPTION_DEFAULT_SPECS \
> -  {"abi", "%{!mabi=elfv*:-mabi=%(VALUE)}" }, \
> +  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" }, \
>    {"tune", "%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}" }, \
>    {"tune_32", "%{" OPT_ARCH32 ":%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}}" }, \
>    {"tune_64", "%{" OPT_ARCH64 ":%{!mtune=*:%{!mcpu=*:-mtune=%(VALUE)}}}" }, \

So this patch will make a difference to people who use --with-abi= for
one of the ABI variant things.

But it does not seem correct.  -mabi=optionA should not override the
-mabi=optionB set in --with-abi=, where A and B are independent, nor
should it override the base ABI.

Please open a PR for the problem you want to solve here, so we do not
lose track of it?

Thanks,


Segher

Re: [patch] Fix thinko in powerpc default specs for -mabi

[Olivier Hainque]

Hi Segher,

> On 3 Oct 2022, at 18:13, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> -mabi= does two separate things, unfortunately.
> 
> First, you can use it to set the base ABI: elfv1, elfv2.  But you can
> also use it to set ABI variants, ABI options: -mabi={no-,}altivec,
> -mabi={ieee,ibm}longdouble, -mabi=vec-{extabi,default}.  Things in that
> latter category are completely orthogonal to anything else (except that
> some only make sense together with some base ABIs).

Ooh, I see. I understood there were abi related options internally
(this is quite visible throughout the code of course) but didn't click
on all the implications on the command -mabi line interface.

> Base ABI is not selectable for most, it is implied by your target
> triple.  -mabi=elfv[12] only makes sense for targets that have either of
> the two by default.

Yes, I can see that now.

>> We have been using this for about a year now in gcc-11 based toolchains.
>> This helps our dejagnu testsuite runs for VxWorks on powerpc and 
>> hasn't produced any ill side effect to date.
> 
> So what exactly is this meant to do?

Biased by other ports, the presence of multiple -mabi switches
just seemed wrong on its own, so the first level motivation was
simply to address that. There might have been interactions with
another change in what we observed at the time.

I understand now that multiple -mabi on the command line is not
a problem per se, so:

> But it does not seem correct.  -mabi=optionA should not override the
> -mabi=optionB set in --with-abi=, where A and B are independent, nor
> should it override the base ABI.

Agreed!

Thanks a lot for your constructive feedback, much appreciated.

With Kind Regards,

Olivier

Re: [patch] Fix thinko in powerpc default specs for -mabi

[Olivier Hainque]

Hello,

Gentle ping for 

https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602143.html

> 2022-09-14  Olivier Hainque  <hainque@adacore.com>
> 
>         * config/rs6000/option-defaults.h (OPTION_DEFAULT_SPECS):
>         Have any -mabi, not only -mabi=elfv*, override the --with-abi
>         configuration default.

As an additional data point, the change gets the rs6000
definition in line with that of arm, mips and riscv.

Thanks in advance!

Best Regards,

Olivier