From: Iain Buclaw <ibuclaw@gdcproject.org>
To: Iain Sandoe <iains.gcc@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] darwin, d: Support outfile substitution for liphobos
Date: Fri, 19 Nov 2021 10:57:19 +0100 [thread overview]
Message-ID: <1637315576.ehpqbegj8r.astroid@galago.none> (raw)
In-Reply-To: <7C82489C-4C47-4501-A59C-D6D7396B545B@gmail.com>
Excerpts from Iain Sandoe's message of November 19, 2021 10:21 am:
> Hi Iain
>
>> On 19 Nov 2021, at 08:32, Iain Buclaw <ibuclaw@gdcproject.org> wrote:
>
>> This patch fixes a stage2 bootstrap failure in the D front-end on
>> darwin due to libgphobos being dynamically linked despite
>> -static-libphobos being on the command line.
>>
>> In the gdc driver, this takes the previous fix for the Darwin D
>> bootstrap, and extends it to the -static-libphobos option as well.
>> Rather than pushing the -static-libphobos option back onto the command
>> line, the setting of SKIPOPT is instead conditionally removed. The same
>> change has been repeated for -static-libstdc++ so there is now no need
>> to call generate_option to re-add it.
>>
>> In the gcc driver, -static-libphobos has been added as a common option,
>> validated, and a new outfile substition added to config/darwin.h to
>> correctly replace -lgphobos with libgphobos.a.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu and
>> x86_64-apple-darwin20.
>>
>> OK for mainline? This would also be fine for gcc-11 release branch too,
>> as well as earlier releases with D support.
>
> the Darwin parts are fine, thanks
>
> The SKIPOPT in d-spec, presumably means “skip removing this opt”?
> otherwise the #ifndef looks odd (because of the static-libgcc|static-libphobos,
> darwin.h would do the substitution for -static-libgcc as well, so it’s not a 100%
> test).
>
The inverse. SKIPOPT means "skip this option", so previously it was
being removed by the driver when constructing the new_decoded_options,
hence why your generate_option addition was necessary before.
Iain.
> Iain
>
>>
>> Regards,
>> Iain.
>>
>> ---
>> gcc/ChangeLog:
>>
>> * common.opt (static-libphobos): Add option.
>> * config/darwin.h (LINK_SPEC): Substitute -lgphobos with libgphobos.a
>> when linking statically.
>> * gcc.c (driver_handle_option): Set -static-libphobos as always valid.
>>
>> gcc/d/ChangeLog:
>>
>> * d-spec.cc (lang_specific_driver): Set SKIPOPT on -static-libstdc++
>> and -static-libphobos only when target supports LD_STATIC_DYNAMIC.
>> Remove generate_option to re-add -static-libstdc++.
>> ---
>> gcc/common.opt | 4 ++++
>> gcc/config/darwin.h | 1 +
>> gcc/d/d-spec.cc | 18 +++++++++++-------
>> gcc/gcc.c | 6 ++++--
>> 4 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index db6010e4e20..73c12d933f3 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -3527,6 +3527,10 @@ static-libgfortran
>> Driver
>> ; Documented for Fortran, but always accepted by driver.
>>
>> +static-libphobos
>> +Driver
>> +; Documented for D, but always accepted by driver.
>> +
>> static-libstdc++
>> Driver
>>
>> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
>> index 7ed01efa694..c4ddd623e8b 100644
>> --- a/gcc/config/darwin.h
>> +++ b/gcc/config/darwin.h
>> @@ -443,6 +443,7 @@ extern GTY(()) int darwin_ms_struct;
>> %:replace-outfile(-lobjc libobjc-gnu.a%s); \
>> :%:replace-outfile(-lobjc -lobjc-gnu )}}\
>> %{static|static-libgcc|static-libgfortran:%:replace-outfile(-lgfortran libgfortran.a%s)}\
>> + %{static|static-libgcc|static-libphobos:%:replace-outfile(-lgphobos libgphobos.a%s)}\
>> %{static|static-libgcc|static-libstdc++|static-libgfortran:%:replace-outfile(-lgomp libgomp.a%s)}\
>> %{static|static-libgcc|static-libstdc++:%:replace-outfile(-lstdc++ libstdc++.a%s)}\
>> %{force_cpusubtype_ALL:-arch %(darwin_arch)} \
>> diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
>> index b12d28f1047..73ecac3bbf1 100644
>> --- a/gcc/d/d-spec.cc
>> +++ b/gcc/d/d-spec.cc
>> @@ -253,13 +253,23 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>>
>> case OPT_static_libstdc__:
>> saw_static_libcxx = true;
>> +#ifndef HAVE_LD_STATIC_DYNAMIC
>> + /* Remove -static-libstdc++ from the command only if target supports
>> + LD_STATIC_DYNAMIC. When not supported, it is left in so that a
>> + back-end target can use outfile substitution. */
>> args[i] |= SKIPOPT;
>> +#endif
>> break;
>>
>> case OPT_static_libphobos:
>> if (phobos_library != PHOBOS_NOLINK)
>> phobos_library = PHOBOS_STATIC;
>> +#ifndef HAVE_LD_STATIC_DYNAMIC
>> + /* Remove -static-libphobos from the command only if target supports
>> + LD_STATIC_DYNAMIC. When not supported, it is left in so that a
>> + back-end target can use outfile substitution. */
>> args[i] |= SKIPOPT;
>> +#endif
>> break;
>>
>> case OPT_shared_libphobos:
>> @@ -460,7 +470,7 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>> #endif
>> }
>>
>> - if (saw_libcxx || need_stdcxx)
>> + if (saw_libcxx || saw_static_libcxx || need_stdcxx)
>> {
>> #ifdef HAVE_LD_STATIC_DYNAMIC
>> if (saw_static_libcxx && !static_link)
>> @@ -468,12 +478,6 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
>> generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
>> &new_decoded_options[j++]);
>> }
>> -#else
>> - /* Push the -static-libstdc++ option back onto the command so that
>> - a target without LD_STATIC_DYNAMIC can use outfile substitution. */
>> - if (saw_static_libcxx && !static_link)
>> - generate_option (OPT_static_libstdc__, NULL, 1, CL_DRIVER,
>> - &new_decoded_options[j++]);
>> #endif
>> if (saw_libcxx)
>> new_decoded_options[j++] = *saw_libcxx;
>> diff --git a/gcc/gcc.c b/gcc/gcc.c
>> index 506c2acc282..fea6d049183 100644
>> --- a/gcc/gcc.c
>> +++ b/gcc/gcc.c
>> @@ -4576,10 +4576,12 @@ driver_handle_option (struct gcc_options *opts,
>> case OPT_static_libgcc:
>> case OPT_shared_libgcc:
>> case OPT_static_libgfortran:
>> + case OPT_static_libphobos:
>> case OPT_static_libstdc__:
>> /* These are always valid, since gcc.c itself understands the
>> - first two, gfortranspec.c understands -static-libgfortran and
>> - g++spec.c understands -static-libstdc++ */
>> + first two, gfortranspec.c understands -static-libgfortran,
>> + d-spec.cc understands -static-libphobos, and g++spec.c
>> + understands -static-libstdc++ */
>> validated = true;
>> break;
>>
>> --
>> 2.30.2
>>
>
>
next prev parent reply other threads:[~2021-11-19 9:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-19 8:32 Iain Buclaw
2021-11-19 9:21 ` Iain Sandoe
2021-11-19 9:57 ` Iain Buclaw [this message]
2021-11-26 12:51 ` Iain Buclaw
2021-11-30 17:03 ` [PING, PATCH] " Iain Buclaw
2021-12-01 14:12 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1637315576.ehpqbegj8r.astroid@galago.none \
--to=ibuclaw@gdcproject.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=iains.gcc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).