[PATCH] newlib: Remove debug flag for --enable-target-optspace

[Andrew Pinski]

On Fri, Nov 29, 2019 at 2:31 AM Stephanos Ioannidis <root@stephanos.io> wrote:
>
> Hi Jon,
>
> While `-g` does not increase the actual code size since it is intended to add debug information only, `--enable-target-optspace` semantically has nothing to do with adding debug information and keeping `-g` here only introduces an unexpected and unintentional behaviour (and confusion).
>
> As you suggested, the debug information added by `--enable-target-optspace` can be stripped, but why add it in the first place if it is not necessary?
>
> Unless there is a good reason to keep `-g` and `-Os` together in `--enable-target-optspace`, I strongly believe `-g` should be removed and moved into a separate configuration option.
>
> p.s. It looks like this was passed down from gcc and other GNU toolchains.

Note I looked into the history of this file and found it originally
did = rather than += but when it changed to being +=, the -g part was
not removed.

This can be seen here:
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02062.html

Also note the upstream of this file is in the GCC sources, so it might
be best if this patch is sent there instead and then sync'ed into the
newlib directory.

Thanks,
Andrew Pinski


>
> Regards,
>
> Stephanos
>
> -----Original Message-----
> From: Jon Beniston <jon@beniston.com>
> Sent: Friday, November 29, 2019 6:56 PM
> To: Stephanos Ioannidis <root@stephanos.io>; newlib@sourceware.org
> Subject: RE: [PATCH] newlib: Remove debug flag for --enable-target-optspace
>
> Hi Stephanos,
>
> I don't think this should be required. -g generally shouldn't cause code size to be bigger when used with -Os. The resulting ELF files will be bigger, as they include debug info, but this debug info is not burnt to flash or loaded in to RAM. If you want the ELF files smaller because they need to go on the targets filesystem, then you can run the strip program on them to remove the debug info.
>
> Cheers,
> Jon
>
> -----Original Message-----
> From: newlib-owner@sourceware.org <newlib-owner@sourceware.org> On Behalf Of Stephanos Ioannidis
> Sent: 29 November 2019 03:47
> To: newlib@sourceware.org
> Cc: Stephanos Ioannidis <root@stephanos.io>
> Subject: [PATCH] newlib: Remove debug flag for --enable-target-optspace
>
> --enable-target-optspace adds -g (debug) option flag to the CFLAGS alongside -Os (optimise for size) for no good reason.
>
> Since the -g option flag is not required by the -Os option flag and it not contribute to what --enable-target-optspace tries to achieve (i.e.
> optimise for size), this flag should be removed.
>
> It is also worth noting that many newlib users (e.g. GNU ARM Embedded) are manually adding -Os to CFLAGS because the --enable-target-optspace does not achieve what it should.
>
> Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
> ---
>  config/mt-ospace | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/config/mt-ospace b/config/mt-ospace index ce29ff431..95be8259d
> 100644
> --- a/config/mt-ospace
> +++ b/config/mt-ospace
> @@ -1,3 +1,3 @@
>  # Build libraries optimizing for space, not speed.
> - CFLAGS_FOR_TARGET += -g -Os
> - CXXFLAGS_FOR_TARGET += -g -Os
> + CFLAGS_FOR_TARGET += -Os
> + CXXFLAGS_FOR_TARGET += -Os
> --
> 2.17.1
>
>