Re: [PATCH][14 backport] c++: Fix instantiation of imported temploid friends [PR114275]

2024-05-24 Thread Iain Sandoe



> On 24 May 2024, at 14:54, Jason Merrill  wrote:
> 
> On 5/24/24 04:06, Nathaniel Shead wrote:
>> On Thu, May 23, 2024 at 06:41:06PM -0400, Jason Merrill wrote:
>>> On 5/13/24 07:56, Nathaniel Shead wrote:
>> @@ -11751,9 +11767,16 @@ tsubst_friend_class (tree friend_tmpl, tree 
>> args)
>>  if (tmpl != error_mark_node)
>>  {
>>/* The new TMPL is not an instantiation of anything, so we
>> - forget its origins.  We don't reset CLASSTYPE_TI_TEMPLATE
>> + forget its origins.  It is also not a specialization of
>> + anything.  We don't reset CLASSTYPE_TI_TEMPLATE
>>   for the new type because that is supposed to be the
>>   corresponding template decl, i.e., TMPL.  */
>> +  spec_entry elt;
>> +  elt.tmpl = friend_tmpl;
>> +  elt.args = CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl));
>> +  elt.spec = TREE_TYPE (tmpl);
>> +  type_specializations->remove_elt ();
> 
> For GCC 14.2 let's guard this with if (modules_p ()); for GCC 15 it can be
> unconditional.  OK.
 
 I'm looking to backport this patch to GCC 14 now that it's been on trunk
 some time.  Here's the patch I'm aiming to add (squashed with the
 changes from r15-220-gec2365e07537e8) after cherrypicking the
 prerequisite commit r15-58-g2faf040335f9b4; is this OK?
 
 Or should I keep it as two separate commits to make the cherrypicking
 more obvious? Not entirely sure on the etiquette around this.
>>> 
>>> It's OK to squash them, but it's typical to use -x (directly or via git
>>> gcc-backport) to mention where a branch change was cherry-picked from, and
>>> in this case it would make sense to edit in the second commit so it's clear
>>> the backport includes both.  OK that way.
>> Sorry, still a bit confused :)  Do you mean to merge the two commits
>> together such that there are two "cherry picked from commit ..."s in the
>> commit message?  Or just list second commit, and mention that it
>> includes both in the commit message?
> 
> I was thinking
> 
> (cherry picked from commit a and b)

For the record, I do not think the git hooks with allow that exactly (or, at 
least, if they
do I did not find the right syntax);  what I’ve done in similar cases is to 
keep the main
“cherry picked from” line for the main patch and then add text to the intro 
section
saying that  and  are also included.

Iain

> 
> but the exact format doesn't matter, just looking for a mention of both 
> commits.
> 
> Jason



[pshed] testsuite, C++, Darwin: Skip cxa_atexit-6, which is not applicable.

2024-05-19 Thread Iain Sandoe
As per the analysis in the PR, tested on x86_64, i686 and aarch64 Darwin
(and on x86_64 linux), pushed to trunk, thanks,
Iain

--- 8< ---

For Darwin, non-weak functions defined in a TU always bind locally
and so cxa_atexit-6.C is not applicable here.

PR testsuite/114982

gcc/testsuite/ChangeLog:

* g++.dg/tree-ssa/cxa_atexit-6.C: Skip for Darwin.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-6.C | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-6.C 
b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-6.C
index f6599a3c9f4..e22036067dd 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-6.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-6.C
@@ -2,10 +2,14 @@
 /* { dg-require-effective-target fpic } */
 /* { dg-options "-O2 -fdump-tree-cddce1-details -fdump-tree-optimized -fPIC" } 
*/
 // { dg-require-effective-target cxa_atexit }
+/* This test is not appropriate for targets where non-weak functions defined
+   in the TU always bind locally; see PR114982.  */
+/* { dg-skip-if "PR114982" { *-*-darwin* } } */
 /* PR tree-optimization/19661 */
 
 /* The call to axexit should not be removed as A::~A() cannot be figured if it
-   is a pure/const function call as the function call g does not bind locally. 
*/
+   is a pure/const function call for platforms where the function call g does
+   not bind locally. */
 
 __attribute__((noinline))
 void g() {}
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, darwin: Compile a test without unwind frames.

2024-05-19 Thread Iain Sandoe
Tested on i686, x86_64 Darwin, pushed to trunk, thanks
Iain

--- 8< ---

In the current Darwin implementation, we do not use .cfi_ insns
and emitted EH frames contain 'coalesced' section designations
which interfere with the scan asm.

gcc/testsuite/ChangeLog:

* gcc.dg/darwin-weakimport-3.c: Suppress unwind frames.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/gcc.dg/darwin-weakimport-3.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/darwin-weakimport-3.c 
b/gcc/testsuite/gcc.dg/darwin-weakimport-3.c
index a15b5b0e7cb..7e83987bad5 100644
--- a/gcc/testsuite/gcc.dg/darwin-weakimport-3.c
+++ b/gcc/testsuite/gcc.dg/darwin-weakimport-3.c
@@ -10,11 +10,12 @@
With modern linkers this is moot, since even weak symbols
are emitted into the regular sections.
 
-   To avoid the unwind tables -fno-asynchronous-unwind-tables.
+   To avoid the unwind tables -fno-asynchronous-unwind-tables
+   and fno-unwind-tables (since EH contains coalesced data).
To ensure that we emit code for an older linker -mtarget-linker
To avoid the get_pc thunk optimise at least O1.  */
 
-/* { dg-options "-fno-asynchronous-unwind-tables -O1 -mtarget-linker 85.2" } */
+/* { dg-options "-fno-asynchronous-unwind-tables -fno-unwind-tables -O1 
-mtarget-linker 85.2" } */
 /* { dg-require-weak "" } */
 
 /* { dg-final { scan-assembler-not "coalesced" } } */
-- 
2.39.2 (Apple Git-143)



Re: Ping [PATCH/RFC] target, hooks: Allow a target to trap on unreachable [PR109267].

2024-05-14 Thread Iain Sandoe



> On 14 May 2024, at 14:29, Richard Biener  wrote:
> 
> On Wed, May 8, 2024 at 9:37 PM Iain Sandoe  wrote:
>> 
>> Hi Folks,
>> 
>> I’d like to land a viable solution to this issue if possible, (it is a show-
>> stopper for the aarch64-darwin development branch).
> 
> I was looking as to how we handle __builtin_trap (whether we have an
> optab for it) - we seem to use two target hooks, have_trap () and
> gen_trap () to expand it (and fall back to a call to abort()).  So I guess
> your target hook is reasonable though I'd name it
> expand_unreachable_as_trap maybe (well, that's now bikeshedding).
> 
> Is this all still required

The underlying problem needs a solution - that is not likely to change
since it’s baked into the ABI for Darwin, at least.

> or is there a workaround you can apply at
> mdreorg or bb-reorder time to avoid expanding _all_ unreachable()s
> as traps?

I do not know what Andrew has in mind - for now I can continue to use
this patch on my development branches to give him some time to
consider/implement something more general in the ME.

(if we get late in stage #1 without an alternate solution - I will re-ping this)

Iain


> 
>>> On 9 Apr 2024, at 14:55, Iain Sandoe  wrote:
>>> 
>>> So far, tested lightly on aarch64-darwin; if this is acceptable then
>>> it will be possible to back out of the ad hoc fixes used on x86 and
>>> powerpc darwin.
>>> Comments welcome, thanks,
>> 
>> @Andrew - you were also (at one stage) talking about some ideas about
>> how to handle this is in the middle end.
>> Is that something you are likely to have time to do?
>> Would it still be reasonable to have a target hook to control the behaviour.
>> (the implementation below allows one to make the effect per TU)
>> 
>> 
>>> Iain
>>> 
>>> --- 8< ---
>>> 
>>> 
>>> In the PR cited case a target linker cannot handle enpty FDEs,
>>> arguably this is a linker bug - but in some cases we might still
>>> wish to work around it.
>>> 
>>> In the case of Darwin, the ABI does not allow two global symbols
>>> to have the same address, so that emitting empty functions has
>>> potential (almost guarantee) to break ABI.
>>> 
>>> This patch allows a target to ask that __builtin_unreachable is
>>> expanded in the same way as __builtin_trap (either to a trap
>>> instruction or to abort() if there is no such insn).
>>> 
>>> This means that the middle end's use of unreachability for
>>> optimisation should not be altered.
>>> 
>>> __builtin_unreachble is currently expanded to a barrier and
>>> __builtin_trap is expanded to a trap insn + a barrier so that it
>>> seems we should not be unduly affecting RTL optimisations.
>>> 
>>> For Darwin, we enable this by default, but allow it to be disabled
>>> per TU using -mno-unreachable-traps.
>>> 
>>>  PR middle-end/109267
>>> 
>>> gcc/ChangeLog:
>>> 
>>>  * builtins.cc (expand_builtin_unreachable): Allow for
>>>  a target to expand this as a trap.
>>>  * config/darwin-protos.h (darwin_unreachable_traps_p): New.
>>>  * config/darwin.cc (darwin_unreachable_traps_p): New.
>>>  * config/darwin.h (TARGET_UNREACHABLE_SHOULD_TRAP): New.
>>>  * config/darwin.opt (munreachable-traps): New.
>>>  * doc/invoke.texi: Document -munreachable-traps.
>>>  * doc/tm.texi: Regenerate.
>>>  * doc/tm.texi.in: Document TARGET_UNREACHABLE_SHOULD_TRAP.
>>>  * target.def (TARGET_UNREACHABLE_SHOULD_TRAP): New hook.
>>> 
>>> Signed-off-by: Iain Sandoe 
>>> ---
>>> gcc/builtins.cc|  7 +++
>>> gcc/config/darwin-protos.h |  1 +
>>> gcc/config/darwin.cc   |  7 +++
>>> gcc/config/darwin.h|  4 
>>> gcc/config/darwin.opt  |  4 
>>> gcc/doc/invoke.texi|  7 ++-
>>> gcc/doc/tm.texi|  5 +
>>> gcc/doc/tm.texi.in |  2 ++
>>> gcc/target.def | 10 ++
>>> 9 files changed, 46 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
>>> index f8d94c4b435..13f321b6be6 100644
>>> --- a/gcc/builtins.cc
>>> +++ b/gcc/builtins.cc
>>> @@ -5929,6 +5929,13 @@ expand_builtin_trap (void)
>>> static void
>>> expand_builtin_unreachable (void)
>>> {
>>> +  /* If the target wants a trap in place of the fall-through, use that.  */

Re: Fix gnu versioned namespace mode 00/03

2024-05-13 Thread Iain Sandoe



> On 13 May 2024, at 06:06, François Dumont  wrote:
> 
> 
> On 07/05/2024 18:15, Iain Sandoe wrote:
>> Hi François
>> 
>>> On 4 May 2024, at 22:11, François Dumont  wrote:
>>> 
>>> Here is the list of patches to restore gnu versioned namespace mode.
>>> 
>>> 1/3: Bump gnu version namespace
>>> 
>>> This is important to be done first so that once build of gnu versioned 
>>> namespace is fixed there is no chance to have another build of '__8' 
>>> version with a different abi than last successful '__8' build.
>>> 
>>> 2/3: Fix build using cxx11 abi for versioned namespace
>>> 
>>> 3/3: Proposal to default to "new" abi when dual abi is disabled and accept 
>>> any default-libstdcxx-abi either dual abi is enabled or not.
>>> 
>>> All testsuite run for following configs:
>>> 
>>> - dual abi
>>> 
>>> - gcc4-compatible only abi
>>> 
>>> - new only abi
>>> 
>>> - versioned namespace abi
>> At the risk of delaying this (a bit) - I think we should also consider items 
>> like once_call that have broken impls.
> Do you have any pointer to this once_call problem, sorry I'm not aware about 
> it (apart from your messages).

(although this mentions one specific target, it applies more widely).
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66146

Also, AFAICT, any nested once_call is a problem (not just exceptions).

>>  in the current library - and at least get proposed replacements available 
>> behind the versioned namespace; rather than using up a namespace version 
>> with the current broken code.
> 
> I'm not proposing to fix all library bugs on all platforms with this patch, 
> just fix the versioned namespace mode.

Sorry, I was not intending to suggest that (although perhaps my comments read 
that way).

I was trying to suggest that, in the case where we have proposed fixes that are 
blocked because they are ABI breaks, that those could be put behind the 
versioned namspace (it was not an intention to suggest that such additions 
should be part of this patch series).

> As to do so I also need to adopt cxx11 abi in versioned mode it already 
> justify a bump of version.

I see - it’s just a bit strange that we are bumping a version for a mode that 
does not currently work;  however, i guess someone might have deployed it even 
so.
> 
> The reason I'm proposing to integrate this patch this early in gcc 15 stage 
> is to have time to integrate any other library fix/optimization that could 
> make use of it. I already have 1 on my side for the hashtable implementation

Ah, then I think we are aiming for the same thing.

> . I hope your once_call fix also have time to be ready for gcc 15, no ?

Yes; if we put it behind the versioned namespace - there are (I think) several 
proposed solutions to that specific issue.

thanks
Iain

> 
> François



Ping [PATCH/RFC] target, hooks: Allow a target to trap on unreachable [PR109267].

2024-05-08 Thread Iain Sandoe
Hi Folks,

I’d like to land a viable solution to this issue if possible, (it is a show-
stopper for the aarch64-darwin development branch).

> On 9 Apr 2024, at 14:55, Iain Sandoe  wrote:
> 
> So far, tested lightly on aarch64-darwin; if this is acceptable then
> it will be possible to back out of the ad hoc fixes used on x86 and
> powerpc darwin.
> Comments welcome, thanks,

@Andrew - you were also (at one stage) talking about some ideas about
how to handle this is in the middle end.
Is that something you are likely to have time to do?
Would it still be reasonable to have a target hook to control the behaviour.
(the implementation below allows one to make the effect per TU)


> Iain
> 
> --- 8< ---
> 
> 
> In the PR cited case a target linker cannot handle enpty FDEs,
> arguably this is a linker bug - but in some cases we might still
> wish to work around it.
> 
> In the case of Darwin, the ABI does not allow two global symbols
> to have the same address, so that emitting empty functions has
> potential (almost guarantee) to break ABI.
> 
> This patch allows a target to ask that __builtin_unreachable is
> expanded in the same way as __builtin_trap (either to a trap
> instruction or to abort() if there is no such insn).
> 
> This means that the middle end's use of unreachability for
> optimisation should not be altered.
> 
> __builtin_unreachble is currently expanded to a barrier and
> __builtin_trap is expanded to a trap insn + a barrier so that it
> seems we should not be unduly affecting RTL optimisations.
> 
> For Darwin, we enable this by default, but allow it to be disabled
> per TU using -mno-unreachable-traps.
> 
>   PR middle-end/109267
> 
> gcc/ChangeLog:
> 
>   * builtins.cc (expand_builtin_unreachable): Allow for
>   a target to expand this as a trap.
>   * config/darwin-protos.h (darwin_unreachable_traps_p): New.
>   * config/darwin.cc (darwin_unreachable_traps_p): New.
>   * config/darwin.h (TARGET_UNREACHABLE_SHOULD_TRAP): New.
>   * config/darwin.opt (munreachable-traps): New.
>   * doc/invoke.texi: Document -munreachable-traps.
>   * doc/tm.texi: Regenerate.
>   * doc/tm.texi.in: Document TARGET_UNREACHABLE_SHOULD_TRAP.
>   * target.def (TARGET_UNREACHABLE_SHOULD_TRAP): New hook.
> 
> Signed-off-by: Iain Sandoe 
> ---
> gcc/builtins.cc|  7 +++
> gcc/config/darwin-protos.h |  1 +
> gcc/config/darwin.cc   |  7 +++
> gcc/config/darwin.h|  4 
> gcc/config/darwin.opt  |  4 
> gcc/doc/invoke.texi|  7 ++-
> gcc/doc/tm.texi|  5 +
> gcc/doc/tm.texi.in |  2 ++
> gcc/target.def | 10 ++
> 9 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> index f8d94c4b435..13f321b6be6 100644
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -5929,6 +5929,13 @@ expand_builtin_trap (void)
> static void
> expand_builtin_unreachable (void)
> {
> +  /* If the target wants a trap in place of the fall-through, use that.  */
> +  if (targetm.unreachable_should_trap ())
> +{
> +  expand_builtin_trap ();
> +  return;
> +}
> +
>   /* Use gimple_build_builtin_unreachable or builtin_decl_unreachable
>  to avoid this.  */
>   gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE));
> diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h
> index b67e05264e1..48a32b2ccc2 100644
> --- a/gcc/config/darwin-protos.h
> +++ b/gcc/config/darwin-protos.h
> @@ -124,6 +124,7 @@ extern void darwin_enter_string_into_cfstring_table 
> (tree);
> extern void darwin_asm_output_anchor (rtx symbol);
> extern bool darwin_use_anchors_for_symbol_p (const_rtx symbol);
> extern bool darwin_kextabi_p (void);
> +extern bool darwin_unreachable_traps_p (void);
> extern void darwin_override_options (void);
> extern void darwin_patch_builtins (void);
> extern void darwin_rename_builtins (void);
> diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
> index dcfccb4952a..018547d09c6 100644
> --- a/gcc/config/darwin.cc
> +++ b/gcc/config/darwin.cc
> @@ -3339,6 +3339,13 @@ darwin_kextabi_p (void) {
>   return flag_apple_kext;
> }
> 
> +/* True, iff we want to map __builtin_unreachable to a trap.  */
> +
> +bool
> +darwin_unreachable_traps_p (void) {
> +  return darwin_unreachable_traps;
> +}
> +
> void
> darwin_override_options (void)
> {
> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
> index d335ffe7345..17f41cf30ef 100644
> --- a/gcc/config/darwin.h
> +++ b/gcc/config/darwin.h
> @@ -1225,6 +1225,10 @@ void add_framework_path (char *);
> #define TARGET_N_F

Re: Fix gnu versioned namespace mode 00/03

2024-05-07 Thread Iain Sandoe
Hi François

> On 4 May 2024, at 22:11, François Dumont  wrote:
> 
> Here is the list of patches to restore gnu versioned namespace mode.
> 
> 1/3: Bump gnu version namespace
> 
> This is important to be done first so that once build of gnu versioned 
> namespace is fixed there is no chance to have another build of '__8' version 
> with a different abi than last successful '__8' build.
> 
> 2/3: Fix build using cxx11 abi for versioned namespace
> 
> 3/3: Proposal to default to "new" abi when dual abi is disabled and accept 
> any default-libstdcxx-abi either dual abi is enabled or not.
> 
> All testsuite run for following configs:
> 
> - dual abi
> 
> - gcc4-compatible only abi
> 
> - new only abi
> 
> - versioned namespace abi

At the risk of delaying this (a bit) - I think we should also consider items 
like once_call that have broken impls. in the current library - and at least 
get proposed replacements available behind the versioned namespace; rather than 
using up a namespace version with the current broken code.

I have a proposed once_call replacement (but I think Jonathan also has one or 
more alternatives there)

Please can we try to identify any other similar blocked fixes?

Iain



Re: Fix gnu versioned namespace mode 01/03

2024-05-07 Thread Iain Sandoe
Hi François

As you know I am keen to see this land - but having had some experience with 
applying previous patches to actual toolchain builds .. 

> On 4 May 2024, at 22:11, François Dumont  wrote:
> 
> libstdc++: Bump gnu versioned namespace to __9

I think that the namespace version should be a top-level configure choice.  
—with-libstdcxx-namespace-version= (for example) - we ought to be able to make 
that the only thing that is required to trigger the process.

The reasons are:

 1. (significant) The information is needed by both the (FE) testsuites and the 
library ( I do not think the it’s a nice maintenance job to have to go through 
all the testcases that have the namespace visible and change them for each GCC 
release); instead we should arrange to set some variable in gcc/site.exp that 
the FE tests can consume (I do not think that this is too hard to arrange - 
although it might be necessary to figure out how to make scan* tests work with 
it)

 2. (nice-to-have) For targets which have never used a versioned namespace it 
seems odd to jump straight from 6 to 9.

Iain

> 
> libstdc++-v3/ChangeLog:
> 
> * acinclude.m4 (libtool_VERSION): Bump to 9:0:0.
> * config/abi/pre/gnu-versioned-namespace.ver (GLIBCXX_8.0): 
> Replace by GLIBCXX_9.0.
> Adapt all references to __8 namespace.
> * configure: Regenerate.
> * include/bits/c++config (_GLIBCXX_BEGIN_NAMESPACE_VERSION): 
> Define as 'namespace __9{'.
> (_GLIBCXX_STD_V): Adapt.
> * include/std/format (to_chars): Update namespace version in 
> symbols alias definitions.
> (__format::_Arg_store): Update namespace version in 
> make_format_args friend
> declaration.
> * python/libstdcxx/v6/printers.py (_versioned_namespace): Assign 
> '__9::'.
> * python/libstdcxx/v6/xmethods.py: Likewise.
> * testsuite/23_containers/map/48101_neg.cc: Adapt dg-error.
> * testsuite/23_containers/multimap/48101_neg.cc: Likewise.
> * testsuite/20_util/function/cons/70692.cc: Likewise.
> * testsuite/20_util/function_objects/bind_back/111327.cc: 
> Likewise.
> * testsuite/20_util/function_objects/bind_front/111327.cc: 
> Likewise.
> * testsuite/lib/prune.exp (libstdc++-dg-prune): Bump version 
> namespace.
> 
> Ok to commit ?
> 
> François
> 



Re: Trait built-in naming convention

2024-05-02 Thread Iain Sandoe



> On 2 May 2024, at 20:30, Ken Matsui  wrote:
> 
> On Thu, May 2, 2024 at 10:54 AM Marek Polacek  wrote:
>> 
>> On Thu, May 02, 2024 at 08:37:53PM +0300, Ville Voutilainen wrote:
>>> On Thu, 2 May 2024 at 20:25, Ken Matsui  wrote:
> There was some discussion of how to name the built-ins back in
> https://gcc.gnu.org/pipermail/gcc-patches/2007-March/thread.html#212171
> but __builtin wasn't discussed.
> 
> Apparently this naming convention follows the MSVC precedent:
> http://msdn2.microsoft.com/en-us/library/ms177194.aspx
> 
> I notice some discussion of this pattern around Clang adding various
> built-ins in https://github.com/llvm/llvm-project/issues/61852
> indicating that this is a policy based on precedent.
> 
> But I don't see any actual reason for this pattern other than that it's
> what Paolo happened to do in 2007.
> 
> I'm not sure what the right way forward is.  Perhaps we're stuck with
> the questionable choices of the past.
> 
 
 Hmm, I personally prefer the __builtin prefix.  However, it seems that
 we need to reach a consensus across MSVC, Clang, and GCC.  Would this
 be realistically possible?
 
 Until then, I think it would be better to use __ for all built-in
 traits.  What do you think?
>>> 
>>> My 0.02: __builtin as a prefix doesn't serve much of a purpose.
>>> Consider __is_constructible. It doesn't add value
>>> to make that __builtin_is_constructible. It's a built-in. Of course
>>> it's a built-in. It's a compiler-implemented trait, and
>>> this is just the intrinsic that implements it.
>> 
>> FWIW, I also like __is_constructible better than __builtin_is_constructible.
> 
> So, updating all existing built-in trait names does not seem
> realistic.  I think there are two options:
> 
> 1. As Jason said, we can use the same name as Clang does and otherwise
> use __builtin.
> 2. Or we can simply use __ for all built-in traits to keep consistency
> with other built-in traits.
> 
> Then, I feel option 2 would sound better since it's consistent across
> all built-in type traits and it might confuse Clang when they
> implement the same built-in.  Also, it would be easier for me to
> implement built-in traits as I don't need to dig into the Clang code
> every time I add a new one.

I agree, being consistent with the status-quo is valuable, some decisions
might have not be the best ones - but I think it would be terribly confusing
to mix __ and __builtin (it immediately makes the reader wonder whar the
difference is).

0.02GBP only, as always
Iain

> 
>> 
>>> Most of the existing builtins for traits don't use a __builtin prefix.
>>> Not in GCC, not in other compilers. They are indeed
>>> just double-underscored versions of the traits. I think that's fine,
>>> and consistent. There's precedent for this
>>> across Embarcadero, Clang, MSVC, and GCC. See
>>> https://clang.llvm.org/docs/LanguageExtensions.html
>>> 
>>> Yes, I know it's inconsistent with other built-ins that aren't C++
>>> library traits. But the water's been flowing under
>>> the bridge on that question for a while now.
>>> 
>>> I would also prefer at least considering mimicking a trait builtin's
>>> name if some other compiler did it first. That's not a hill
>>> to die on, we don't need to be 100% compatible including the naming,
>>> but if we can, we should just use a name that was
>>> chosen by someone else already. It's just nice to have the same name
>>> if the traits do exactly the same thing. If they don't,
>>> then it's good and in fact very good to give our trait a different name.
>>> 
>> 
>> Marek



[pushed] Objective-C, NeXT, v2: Correct a regression in code-gen.

2024-05-02 Thread Iain Sandoe
My testing of the GCC-14 release branch revealed an Objective-C
regression in code-gen, the fix has been tested on x86_64, i686
and powerpc darwin, pushed to trunk.

I will shortly apply this to the open branches, since they are
affected too.  Given that this is completely local to Darwin and
Objective-C (and pretty trivial too) - would it be acceptable for
GCC-14.1?

thanks
Iain

--- 8< ---

There have been several changes in the ABI of Objective-C which
depend on the OS version targetted.  In this case Protocols and
LabelProtocols should be made weak/hidden/extern from macOS 10.7
however there was a mistake in the code causing this to occur
from macOS 10.6.  Fixed thus.

gcc/objc/ChangeLog:

* objc-next-runtime-abi-02.cc (WEAK_PROTOCOLS_AFTER): New.
(next_runtime_abi_02_protocol_decl): Use WEAK_PROTOCOLS_AFTER
to determine this ABI change.
(build_v2_protocol_list_address_table): Likewise.

Signed-off-by: Iain Sandoe 
---
 gcc/objc/objc-next-runtime-abi-02.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/objc/objc-next-runtime-abi-02.cc 
b/gcc/objc/objc-next-runtime-abi-02.cc
index cdf559b9bea..248ef641281 100644
--- a/gcc/objc/objc-next-runtime-abi-02.cc
+++ b/gcc/objc/objc-next-runtime-abi-02.cc
@@ -72,6 +72,7 @@ along with GCC; see the file COPYING3.  If not see
 #define TAG_MSGSENDSUPER_STRET "objc_msgSendSuper2_stret"
 
 #define USE_FIXUP_BEFORE   100600
+#define WEAK_PROTOCOLS_AFTER   100700
 #define TAG_FIXUP  "_fixup"
 
 
@@ -1025,7 +1026,7 @@ next_runtime_abi_02_protocol_decl (tree p)
   /* static struct _objc_protocol _OBJC_Protocol_; */
   snprintf (buf, BUFSIZE, "_OBJC_Protocol_%s",
IDENTIFIER_POINTER (PROTOCOL_NAME (p)));
-  if (flag_next_runtime >= USE_FIXUP_BEFORE)
+  if (flag_next_runtime >= WEAK_PROTOCOLS_AFTER)
 {
   decl = create_hidden_decl (objc_v2_protocol_template, buf);
   DECL_WEAK (decl) = true;
@@ -2315,7 +2316,7 @@ build_v2_protocol_list_address_table (void)
   gcc_assert (ref->id && TREE_CODE (ref->id) == PROTOCOL_INTERFACE_TYPE);
   snprintf (buf, BUFSIZE, "_OBJC_LabelProtocol_%s",
IDENTIFIER_POINTER (PROTOCOL_NAME (ref->id)));
-  if (flag_next_runtime >= USE_FIXUP_BEFORE)
+  if (flag_next_runtime >= WEAK_PROTOCOLS_AFTER)
{
  decl = create_hidden_decl (objc_protocol_type, buf, /*is def=*/true);
  DECL_WEAK (decl) = true;
-- 
2.39.2 (Apple Git-143)



Re: [PATCH v2] [testsuite] require sqrt_insn effective target where needed

2024-04-23 Thread Iain Sandoe
Hi Folks,

> On 23 Apr 2024, at 09:59, Kewen.Lin  wrote:
> 
> Hi,
> 
> on 2024/4/22 17:56, Alexandre Oliva wrote:
>> This patch takes feedback received for 3 earlier patches, and adopts a
>> simpler approach to skip the still-failing tests, that I believe to be
>> in line with ppc maintainers' expressed preferences.
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html
>> Ping?-ish :-)
>> 
>> 
>> Some tests fail on ppc and ppc64 when testing a compiler [with options
>> for] for a CPU [emulator] that doesn't support the sqrt insn.
>> 
>> The gcc.dg/cdce3.c is one in which the expected shrink-wrap
>> optimization only takes place when the target CPU supports a sqrt
>> insn.
>> 
>> The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and
>> call sqrt(), which involves the sqrt insn that the target CPU under
>> test may not support.
>> 
>> Require a sqrt_insn effective target for all the affected tests.
>> 
>> Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu.  Also testing
>> with gcc-13 on ppc64-vx7r2 and ppc-vx7r2.  Ok to install?
>> 
>> 
>> for  gcc/testsuite/ChangeLog
>> 
>>  * gcc.dg/cdce3.c: Require sqrt_insn effective target.
>>  * gcc.target/powerpc/pr46728-10.c: Likewise.
>>  * gcc.target/powerpc/pr46728-11.c: Likewise.
>>  * gcc.target/powerpc/pr46728-13.c: Likewise.
>>  * gcc.target/powerpc/pr46728-14.c: Likewise.
>> ---
>> gcc/testsuite/gcc.dg/cdce3.c  |3 ++-
>> gcc/testsuite/gcc.target/powerpc/pr46728-10.c |1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-11.c |1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-13.c |1 +
>> gcc/testsuite/gcc.target/powerpc/pr46728-14.c |1 +
>> 5 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c
>> index 601ddf055fd71..f759a95972e8b 100644
>> --- a/gcc/testsuite/gcc.dg/cdce3.c
>> +++ b/gcc/testsuite/gcc.dg/cdce3.c
>> @@ -1,7 +1,8 @@
>> /* { dg-do compile } */
>> /* { dg-require-effective-target hard_float } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details 
>> -fdump-tree-optimized" } */
>> -/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is 
>> shrink-wrapped into error conditions\." "cdce" } } */
>> +/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is 
>> shrink-wrapped into error conditions\." "cdce" } } */
>> /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" 
>> "optimized" } } */
>> /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */
>> 
> 
> This change needs an approval from global maintainer as it touches a generic 
> test case?
> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> index 3be4728d333a4..7e9bb638106c2 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm 
>> -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
> 
> This change looks sensible to me.
> 
> Nit: With the proposed change, I'd expect that we can remove the line for 
> powerpc*-*-darwin*.
> 
> CC Iain to confirm.

Indeed, the check for sqrt_insn fails and so the test is unsupported without 
needing the separate
powerpc*-*-darwin* line,

thanks,
Iain

> 
> BR,
> Kewen
> 
>> 
>> #include 
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> index 43b6728a4b812..5bfa25925675a 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm 
>> -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> #include 
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> index b9fd63973b728..b66d0209a5e54 100644
>> --- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c
>> @@ -1,6 +1,7 @@
>> /* { dg-do run } */
>> /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */
>> /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm 
>> -mpowerpc-gpopt" } */
>> +/* { dg-require-effective-target sqrt_insn } */
>> 
>> #include 
>> 
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c 
>> b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c
>> index 

[PATCH] libgfortran: Fix up the autoreconf warnings.

2024-04-18 Thread Iain Sandoe
@tschwinge since he did quite a bit of work on getting autoreconf to
work in the GCC-13 cycle.

This does not address the issues with regenerating lib code, but it
does make things somewhat smoother for cases where the updates are
only in Makefile.am, configure.ac or libtool.m4 for example.

It is based on a patch I've been using on the release branches for
Darwin (written because I wasted a day on a warning missed among the
wall of output).

You should now be able to run "autoreconf -fv" in the libgfortran
directory with only informational output (no warnings).

So far only tested very lightly on trunk - but posting early in case
it helps the way forward.

thanks
Iain

--- 8< ---

This means using sub-dirs and amending some of the recipes accordingly.

libgfortran/ChangeLog:

* Makefile.am: Use sub-dirs, amend recipies accordingly.
* Makefile.in: Regenerate.

Signed-off-by: Iain Sandoe 
---
 libgfortran/Makefile.am | 1431 +++---
 libgfortran/Makefile.in | 9848 ++-
 2 files changed, 4126 insertions(+), 7153 deletions(-)

diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am
index 9f8a4f69863..8bef1729219 100644
--- a/libgfortran/Makefile.am
+++ b/libgfortran/Makefile.am
@@ -1,5 +1,6 @@
 ## Process this file with automake to produce Makefile.in
 
+AUTOMAKE_OPTIONS = foreign subdir-objects
 
 ACLOCAL_AMFLAGS = -I .. -I ../config
 
@@ -239,629 +240,629 @@ runtime/stop.c
 endif
 
 i_all_c= \
-$(srcdir)/generated/all_l1.c \
-$(srcdir)/generated/all_l2.c \
-$(srcdir)/generated/all_l4.c \
-$(srcdir)/generated/all_l8.c \
-$(srcdir)/generated/all_l16.c
+generated/all_l1.c \
+generated/all_l2.c \
+generated/all_l4.c \
+generated/all_l8.c \
+generated/all_l16.c
 
 i_any_c= \
-$(srcdir)/generated/any_l1.c \
-$(srcdir)/generated/any_l2.c \
-$(srcdir)/generated/any_l4.c \
-$(srcdir)/generated/any_l8.c \
-$(srcdir)/generated/any_l16.c
+generated/any_l1.c \
+generated/any_l2.c \
+generated/any_l4.c \
+generated/any_l8.c \
+generated/any_l16.c
 
 i_bessel_c= \
-$(srcdir)/generated/bessel_r4.c \
-$(srcdir)/generated/bessel_r8.c \
-$(srcdir)/generated/bessel_r10.c \
-$(srcdir)/generated/bessel_r16.c \
-$(srcdir)/generated/bessel_r17.c
+generated/bessel_r4.c \
+generated/bessel_r8.c \
+generated/bessel_r10.c \
+generated/bessel_r16.c \
+generated/bessel_r17.c
 
 i_count_c= \
-$(srcdir)/generated/count_1_l.c \
-$(srcdir)/generated/count_2_l.c \
-$(srcdir)/generated/count_4_l.c \
-$(srcdir)/generated/count_8_l.c \
-$(srcdir)/generated/count_16_l.c
+generated/count_1_l.c \
+generated/count_2_l.c \
+generated/count_4_l.c \
+generated/count_8_l.c \
+generated/count_16_l.c
 
 i_iall_c= \
-$(srcdir)/generated/iall_i1.c \
-$(srcdir)/generated/iall_i2.c \
-$(srcdir)/generated/iall_i4.c \
-$(srcdir)/generated/iall_i8.c \
-$(srcdir)/generated/iall_i16.c
+generated/iall_i1.c \
+generated/iall_i2.c \
+generated/iall_i4.c \
+generated/iall_i8.c \
+generated/iall_i16.c
 
 i_iany_c= \
-$(srcdir)/generated/iany_i1.c \
-$(srcdir)/generated/iany_i2.c \
-$(srcdir)/generated/iany_i4.c \
-$(srcdir)/generated/iany_i8.c \
-$(srcdir)/generated/iany_i16.c
+generated/iany_i1.c \
+generated/iany_i2.c \
+generated/iany_i4.c \
+generated/iany_i8.c \
+generated/iany_i16.c
 
 i_iparity_c= \
-$(srcdir)/generated/iparity_i1.c \
-$(srcdir)/generated/iparity_i2.c \
-$(srcdir)/generated/iparity_i4.c \
-$(srcdir)/generated/iparity_i8.c \
-$(srcdir)/generated/iparity_i16.c
+generated/iparity_i1.c \
+generated/iparity_i2.c \
+generated/iparity_i4.c \
+generated/iparity_i8.c \
+generated/iparity_i16.c
 
 i_findloc0_c= \
-$(srcdir)/generated/findloc0_i1.c \
-$(srcdir)/generated/findloc0_i2.c \
-$(srcdir)/generated/findloc0_i4.c \
-$(srcdir)/generated/findloc0_i8.c \
-$(srcdir)/generated/findloc0_i16.c \
-$(srcdir)/generated/findloc0_r4.c \
-$(srcdir)/generated/findloc0_r8.c \
-$(srcdir)/generated/findloc0_r10.c \
-$(srcdir)/generated/findloc0_r16.c \
-$(srcdir)/generated/findloc0_r17.c \
-$(srcdir)/generated/findloc0_c4.c \
-$(srcdir)/generated/findloc0_c8.c \
-$(srcdir)/generated/findloc0_c10.c \
-$(srcdir)/generated/findloc0_c16.c \
-$(srcdir)/generated/findloc0_c17.c
+generated/findloc0_i1.c \
+generated/findloc0_i2.c \
+generated/findloc0_i4.c \
+generated/findloc0_i8.c \
+generated/findloc0_i16.c \
+generated/findloc0_r4.c \
+generated/findloc0_r8.c \
+generated/findloc0_r10.c \
+generated/findloc0_r16.c \
+generated/findloc0_r17.c \
+generated/findloc0_c4.c \
+generated/findloc0_c8.c \
+generated/findloc0_c10.c \
+generated/findloc0_c16.c \
+generated/findloc0_c17.c
 
 i_findloc0s_c= \
-$(srcdir)/generated/findloc0_s1.c \
-$(srcdir)/generated/findloc0_s4.c
+generated/findloc0_s1.c \
+generated/findloc0_s4.c
 
 i_findloc1_c= \
-$(srcdir)/generated/findloc1_i1.c \
-$(srcdir)/generated/findloc1_i2.c \
-$(srcdir)/generated/findloc1_i4.c \
-$(srcdir)/generated/findloc1_i8.c \
-$(srcdir)/generated/findloc1_i16.c \
-$(srcdir)/generated/findloc1_r4.c \
-$(srcdir)/generated/findloc1_r8.

Re: Request for testing on non-Linux targets; remove special casing of /usr/lib and /lib from the driver

2024-04-17 Thread Iain Sandoe
Hi Andrew,

> On 17 Apr 2024, at 14:59, Rainer Orth  wrote:

>>  The driver currently will remove "/lib" and "/usr/lib" from the library
>> path that gets passed to the linker because it considers them as paths that
>> the linker will already known to search. But this is not true for newer
>> linkers, mold and lld for an example don't have a default search path.
>> This patch removes the special casing to fix FreeBSD building where lld is
>> used by default and also fix riscv-linux-gnu when used in combination with
>> mold.
>> I have tested it on x86_64-linux-gnu and it works there but since the code
>> in the driver has been around since 1992, I request some folks to test it
>> on AIX, Mac OS (Darwin) and solaris where the ld is not GNU bfd ld as I
>> don't have access to those targets currently.
> 
> actually, you do: all of those are availble inside the cfarm.
> 
> I've also tested the patch on i386-pc-solaris2.11 and
> sparc-sun-solaris2.11 with the native ld: no regressions in either case.

I tested so far on x86_64 darwin, but the behaviour of collect2 is the same
for all arches.  Actually, I do not see any difference in behaviour when looking
at the -Wl,-v output (it seems that somehow those paths are not currently
pruned for Darwin; if I add -L /lib to the command line on a GCC build without
this patch it is passed through despite that it’s non-existent).  However, 
that’s
a separate bug, perhaps.

So - as far as this patch is concerned it seems OK for Darwin,
Iain



Re: [PATCH] c++/modules: optimize tree flag streaming

2024-04-13 Thread Iain Sandoe
Hi Patrick,

> On 10 Apr 2024, at 17:33, Jason Merrill  wrote:
> 
> On 4/10/24 11:26, Patrick Palka wrote:
>> On Wed, 10 Apr 2024, Patrick Palka wrote:
>>> 
>>> On Tue, 9 Apr 2024, Jason Merrill wrote:
>>> 
 On 2/16/24 10:06, Patrick Palka wrote:
> On Thu, 15 Feb 2024, Patrick Palka wrote:
> 



> Let's keep documenting the inheritance relationship here, i.e.
> 
>  bytes_in : data
> 
>> @@ -694,13 +656,132 @@ protected:
>>/* Instrumentation.  */
>>static unsigned spans[4];
>>static unsigned lengths[4];
>> -  static int is_set;
>> +  friend struct bits_out;
> 
> It might be a little more elegant for bits_in/out to be nested classes of 
> bytes_in/out, returned from member functions, rather than friends constructed 
> directly?  OK either way, with the above comment tweak.

Unfortunately, this seems to break x86_64 Darwin bootstrap with fails as below 
- I did not yet have a chance to look in any morre detail, so this is a head’s 
up - unless you have any immediate ideas?

thanks
Iain


/src-local/gcc-master/gcc/cp/module.cc: In member function 
‘{anonymous}::bytes_in::bits_in {anonymous}::bytes_in::stream_bits()’:
/src-local/gcc-master/gcc/cp/module.cc:735:24: error: use of deleted function 
‘{anonymous}::bytes_in::bits_in::bits_in(const {anonymous}::bytes_in::bits_in&)’
  735 |   return bits_in (*this);
  |^
/src-local/gcc-master/gcc/cp/module.cc:709:3: note: declared here
  709 |   bits_in(const bits_in&) = delete;
  |   ^~~
/src-local/gcc-master/gcc/cp/module.cc: In member function 
‘{anonymous}::bytes_out::bits_out {anonymous}::bytes_out::stream_bits()’:
/src-local/gcc-master/gcc/cp/module.cc:796:25: error: use of deleted function 
‘{anonymous}::bytes_out::bits_out::bits_out(const 
{anonymous}::bytes_out::bits_out&)’
  796 |   return bits_out (*this);
  | ^
/src-local/gcc-master/gcc/cp/module.cc:755:3: note: declared here
  755 |   bits_out(const bits_out&) = delete;
  |   ^~~~




Re: [PATCH] libstdc++: Compile std::allocator instantiations as C++20

2024-04-11 Thread Iain Sandoe



> On 11 Apr 2024, at 18:33, Ville Voutilainen  
> wrote:
> 
> On Thu, 11 Apr 2024 at 20:22, Jonathan Wakely  wrote:
>> 
>> I'm considering this late patch for gcc-14 to workaround an issue
>> discovered by a recent Clang change.
>> 
>> I'm not yet sure if Clang is right to require these symbols. It's not
>> really clear, because always_inline isn't part of the standard so it's
>> not clear how it should interact with explicit instantiations and
>> modules. Exporting these four extra symbols doesn't hurt, even if Clang
>> ends up reverting or revising its change that requires them.
>> 
>> Another way to fix it would be to suppress the explicit instantiation
>> declarations in  for C++20, so that the compiler
>> always instantiates them implicitly as needed. We do similar things for
>> the explicit instantiations of std::string etc. so that new member
>> functions that aren't in the .so are implicitly instantiated as needed.
>> 
>> That would look like this instead:
>> 
>> --- a/libstdc++-v3/include/bits/allocator.h
>> +++ b/libstdc++-v3/include/bits/allocator.h
>> @@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> 
>>   // Inhibit implicit instantiations for required instantiations,
>>   // which are defined via explicit instantiations elsewhere.
>> -#if _GLIBCXX_EXTERN_TEMPLATE
>> +#if _GLIBCXX_EXTERN_TEMPLATE && __cplusplus <= 201703L
>>   extern template class allocator;
>>   extern template class allocator;
>> #endif
>> 
>> But we might want to export the new functions from the library
>> eventually anyway, so doing it now (before Clang 19 is released) might
>> be the best option.

I think clang-19 will branch after gcc-14 (so the urgency is more on the GCC
side - or making a decision to back out of the clang change.

>> 
>> Thoughts?
> 
> I think the symbol export is a fine solution. Both of these solutions
> work, so I don't have a strong preference,
> I have a minor preference for not suppressing explicit instantiations
> that are otherwise already there,
> but that is indeed not a strong preference.

In the case of interaction between modules and items not identified in
the std, the only solution is to get agreement between the ‘vendors’.

If we emit the symbols, then that gets baked into the libstdc++ ABI, right?

Is there an alternate solution that we can propose? (there is a modules
implementer’s group that meets bi-weekly, including next Tuesday - 
this includes representation from GCC, clang, MSVC and usually some
of the larger players like google… it could be a topic of discussion if
there is some tidier proposal we could make.

Iain



Re: [PATCH/RFC] On the use of -funreachable-traps to deal with PR 109627

2024-04-09 Thread Iain Sandoe



> On 9 Apr 2024, at 08:53, Iain Sandoe  wrote:
> 
> 
> 
>> On 9 Apr 2024, at 08:48, Jakub Jelinek  wrote:
>> 
>> On Tue, Apr 09, 2024 at 09:44:01AM +0200, Richard Biener wrote:
>>> (why not do it at each such switch?)
>> 
>> Because the traps would then be added even to the bbs which later
>> end up in the middle of the function.
> 
> If we defer the unreachable => trap change until expand, then it would
> not affect any of the current decisions made by the middle end.
> 
> Since the default expansion of unreachable is to a barrier - would this
> actually make material difference to RTL optimizations?

Here is an implementation of this:
https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649074.html

Taking a solution to PR109267 out of the equation - it would still be good
to get an answer to the original question “is -funreachable-traps behaving
as expected”? (since it does not substitute in the TU we’ve been discussing)

thanks
Iain



[PATCH/RFC] target, hooks: Allow a target to trap on unreachable [PR109267].

2024-04-09 Thread Iain Sandoe
So far, tested lightly on aarch64-darwin; if this is acceptable then
it will be possible to back out of the ad hoc fixes used on x86 and
powerpc darwin.
Comments welcome, thanks,
Iain

--- 8< ---


In the PR cited case a target linker cannot handle enpty FDEs,
arguably this is a linker bug - but in some cases we might still
wish to work around it.

In the case of Darwin, the ABI does not allow two global symbols
to have the same address, so that emitting empty functions has
potential (almost guarantee) to break ABI.

This patch allows a target to ask that __builtin_unreachable is
expanded in the same way as __builtin_trap (either to a trap
instruction or to abort() if there is no such insn).

This means that the middle end's use of unreachability for
optimisation should not be altered.

__builtin_unreachble is currently expanded to a barrier and
__builtin_trap is expanded to a trap insn + a barrier so that it
seems we should not be unduly affecting RTL optimisations.

For Darwin, we enable this by default, but allow it to be disabled
per TU using -mno-unreachable-traps.

PR middle-end/109267

gcc/ChangeLog:

* builtins.cc (expand_builtin_unreachable): Allow for
a target to expand this as a trap.
* config/darwin-protos.h (darwin_unreachable_traps_p): New.
* config/darwin.cc (darwin_unreachable_traps_p): New.
* config/darwin.h (TARGET_UNREACHABLE_SHOULD_TRAP): New.
* config/darwin.opt (munreachable-traps): New.
* doc/invoke.texi: Document -munreachable-traps.
* doc/tm.texi: Regenerate.
* doc/tm.texi.in: Document TARGET_UNREACHABLE_SHOULD_TRAP.
* target.def (TARGET_UNREACHABLE_SHOULD_TRAP): New hook.

Signed-off-by: Iain Sandoe 
---
 gcc/builtins.cc|  7 +++
 gcc/config/darwin-protos.h |  1 +
 gcc/config/darwin.cc   |  7 +++
 gcc/config/darwin.h|  4 
 gcc/config/darwin.opt  |  4 
 gcc/doc/invoke.texi|  7 ++-
 gcc/doc/tm.texi|  5 +
 gcc/doc/tm.texi.in |  2 ++
 gcc/target.def | 10 ++
 9 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index f8d94c4b435..13f321b6be6 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -5929,6 +5929,13 @@ expand_builtin_trap (void)
 static void
 expand_builtin_unreachable (void)
 {
+  /* If the target wants a trap in place of the fall-through, use that.  */
+  if (targetm.unreachable_should_trap ())
+{
+  expand_builtin_trap ();
+  return;
+}
+
   /* Use gimple_build_builtin_unreachable or builtin_decl_unreachable
  to avoid this.  */
   gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE));
diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h
index b67e05264e1..48a32b2ccc2 100644
--- a/gcc/config/darwin-protos.h
+++ b/gcc/config/darwin-protos.h
@@ -124,6 +124,7 @@ extern void darwin_enter_string_into_cfstring_table (tree);
 extern void darwin_asm_output_anchor (rtx symbol);
 extern bool darwin_use_anchors_for_symbol_p (const_rtx symbol);
 extern bool darwin_kextabi_p (void);
+extern bool darwin_unreachable_traps_p (void);
 extern void darwin_override_options (void);
 extern void darwin_patch_builtins (void);
 extern void darwin_rename_builtins (void);
diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index dcfccb4952a..018547d09c6 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -3339,6 +3339,13 @@ darwin_kextabi_p (void) {
   return flag_apple_kext;
 }
 
+/* True, iff we want to map __builtin_unreachable to a trap.  */
+
+bool
+darwin_unreachable_traps_p (void) {
+  return darwin_unreachable_traps;
+}
+
 void
 darwin_override_options (void)
 {
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index d335ffe7345..17f41cf30ef 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -1225,6 +1225,10 @@ void add_framework_path (char *);
 #define TARGET_N_FORMAT_TYPES 1
 #define TARGET_FORMAT_TYPES darwin_additional_format_types
 
+/* We want __builtin_unreachable to be expanded as a trap instruction.  */
+#undef TARGET_UNREACHABLE_SHOULD_TRAP
+#define TARGET_UNREACHABLE_SHOULD_TRAP darwin_unreachable_traps_p
+
 #ifndef USED_FOR_TARGET
 extern void darwin_driver_init (unsigned int *,struct cl_decoded_option **);
 #define GCC_DRIVER_HOST_INITIALIZATION \
diff --git a/gcc/config/darwin.opt b/gcc/config/darwin.opt
index 119faf7b385..f96f3de8a3e 100644
--- a/gcc/config/darwin.opt
+++ b/gcc/config/darwin.opt
@@ -91,6 +91,10 @@ mtarget-linker
 Target RejectNegative Joined Separate Var(darwin_target_linker) 
Init(LD64_VERSION)
 -mtarget-linker   Specify that ld64  is the toolchain 
linker for the current invocation.
 
+munreachable-traps
+Target Var(darwin_unreachable_traps) Init(1)
+When set (the default) this makes __builtin_unreachable render as a trap.
+
 ; Driver options.
 
 all_load
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d75fc87cdda..06b8eb02508 100

Re: [PATCH] build: Check for cargo when building rust language

2024-04-09 Thread Iain Sandoe
Hi Arthur,

> On 9 Apr 2024, at 13:01, Arthur Cohen  wrote:
> 

> On 4/9/24 10:55, Iain Sandoe wrote:
>> Hi Arthur,
>>> On 9 Apr 2024, at 11:40, Arthur Cohen  wrote:
>>> On 4/9/24 09:47, John Paul Adrian Glaubitz wrote:
>>>> Hello,
>>>> On Mon, 2024-04-08 at 18:33 +0200, pierre-emmanuel.pa...@embecosm.com 
>>>> wrote:
>>>>> The rust frontend requires cargo to build some of it's components,
>>>>> it's presence was not checked during configuration.
>>>> Isn't this creating a hen-and-egg problem? How am I supposed to build a 
>>>> Rust
>>>> compiler for a target which is not supported by rustc (yet) when gccrs is
>>>> supposed to build-depend on cargo which requires rustc?
>>>> Adrian
>>> 
>>> Quick reminder in case you haven't seen our Request for Comments on the 
>>> main ML that this is only a temporary solution. Once gccrs can compile its 
>>> dependencies, we'll go through a more "classical" bootstrapping chain.
>> I don’t suppose there’s some way to make a “download prerequisites” action 
>> for this?
> 
> Do you mean downloading cargo/Rust as a prerequisite? I don't believe this is 
> being done for GNAT/GDC, but I might be wrong.

No, you are quite correct, but the critical difference is that Ada and D both 
make use of earlier versions of GCC - so that (if one wished to be particular) 
it is possible to start with an earlier version of GCC and work forwards (in 
fact that’s what I’ve [I guess all of us] have done for D … and did a looong 
time ago for Ada).

The difference here is that we need to install an executable from somewhere 
else - and making that as simple and trustworthy as possible seems like a good 
move to encourage folks to build & test rust.

> If you mean the dependencies for our Rust components, those are currently 
> being vendored so that we're able to build them offline. I'll push the 
> commits soon.

OK.. I’m sorry to say this - but what’s actually needed is still a little fuzzy 
to me - but I am happy to wait to read the documentation patch and comment then.

thanks
Iain

> 
>> (I realise that the prerequisite might not be available for a given platform 
>> - but then the configure will then just fail to detect them and carry on).
>> At the least the build documentation requested should (ideally) try to lower 
>> the barrier to finding the deps and give reliable sources for them.
>>> rustc_codegen_gcc can probably already be used for building these 
>>> dependencies however, if you'd like to have a look at that.
>> Detailing the verious options would also be a helpful part of the build doc.
>> thanks
>> Iain
> 
> Best,
> 
> Arthur



Re: [PATCH] build: Check for cargo when building rust language

2024-04-09 Thread Iain Sandoe
Hi Arthur,

> On 9 Apr 2024, at 11:40, Arthur Cohen  wrote:

> On 4/9/24 09:47, John Paul Adrian Glaubitz wrote:
>> Hello,
>> On Mon, 2024-04-08 at 18:33 +0200, pierre-emmanuel.pa...@embecosm.com wrote:
>>> The rust frontend requires cargo to build some of it's components,
>>> it's presence was not checked during configuration.
>> Isn't this creating a hen-and-egg problem? How am I supposed to build a Rust
>> compiler for a target which is not supported by rustc (yet) when gccrs is
>> supposed to build-depend on cargo which requires rustc?
>> Adrian
> 
> Quick reminder in case you haven't seen our Request for Comments on the main 
> ML that this is only a temporary solution. Once gccrs can compile its 
> dependencies, we'll go through a more "classical" bootstrapping chain.


I don’t suppose there’s some way to make a “download prerequisites” action for 
this?

(I realise that the prerequisite might not be available for a given platform - 
but then the configure will then just fail to detect them and carry on).

At the least the build documentation requested should (ideally) try to lower 
the barrier to finding the deps and give reliable sources for them.

> rustc_codegen_gcc can probably already be used for building these 
> dependencies however, if you'd like to have a look at that.

Detailing the verious options would also be a helpful part of the build doc.

thanks
Iain



Re: [PATCH/RFC] On the use of -funreachable-traps to deal with PR 109627

2024-04-09 Thread Iain Sandoe



> On 9 Apr 2024, at 08:48, Jakub Jelinek  wrote:
> 
> On Tue, Apr 09, 2024 at 09:44:01AM +0200, Richard Biener wrote:
>> (why not do it at each such switch?)
> 
> Because the traps would then be added even to the bbs which later
> end up in the middle of the function.

If we defer the unreachable => trap change until expand, then it would
not affect any of the current decisions made by the middle end.

Since the default expansion of unreachable is to a barrier - would this
actually make material difference to RTL optimizations?

Iain

> 
>   Jakub
> 



[PATCH/RFC] On the use of -funreachable-traps to deal with PR 109627

2024-04-08 Thread Iain Sandoe
Hi

PR 109627 is about functions that have had their bodies completely elided, but 
still have the wrappers for EH frames (either .cfi_xxx or LFSxx/LFExx).

These are causing issues for some linkers because such functions result in FDEs 
with a 0 code extent.

The simplest representation of this is (from PR109527)

void foo () { __builtin_unreachable (); }

The solution (so far) is to detect this case during final lowering and replace 
the unreachable (which is expanded to nothing, at least for the targets I’ve 
dealt with) by a trap; this results in two positive improvements (1) the FDE is 
now finite-sized so the linker consumes it and (2) actually the trap is 
considerably more user-friendly UB than falling through to some other arbitrary 
place.

I was looking into using -funreachable-traps to do this for aarch64 Darwin - 
because the ad-hoc solutions that were applied to X86 and PPC are not easily 
usable for aarch64.

-funreachabe-traps was added for similar reasons (helping make missing returns 
less unexpected) in r13-1204-gd68d3664253696 by Jason (and then there have been 
further improvements resulting in the use of __builtin_unreachable trap () from 
Jakub)

As I read the commit message for r13-1204, I would expect -funreachable-traps 
to work for the simple case above, but it does not.  I think that is because 
the incremental patch below is needed.  however, I am not sure if there was 
some reason this was not done at the time?

PR 109627 is currently a show-stopper for the aarch64-darwin branch since 
libgomp and libgm2 fail to bootstrap - and other workarounds (e.g. 
-D__builtin_unreachable=__builtin_trap) do not work got m2 (since it does not 
use the C preprocessor by default).

Setting -funreachable-traps either per affected file, or globally for a target 
resolves the issue in a neater manner.

Any guidance / comments would be most welcome - if the direction seems sane, I 
can repost this patch formally.

(I have tested quite widely on Darwin and on a small number of Linux cases too)

thanks
Iain

* I will note that applying this does result in some regressions in several 
contracts test cases - but they also regress for -fsanitize=undefined 
-fsanitise-traps (not yet clear if that’s expected or we’ve uncovered a bug in 
the contracts impl.).

--


diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index f8d94c4b435..e2d26e45744 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -5931,7 +5931,8 @@ expand_builtin_unreachable (void)
 {
   /* Use gimple_build_builtin_unreachable or builtin_decl_unreachable
  to avoid this.  */
-  gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE));
+  gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE)
+  && !flag_unreachable_traps);
   emit_barrier ();
 }
 
@@ -10442,7 +10443,7 @@ fold_builtin_0 (location_t loc, tree fndecl)
 
 case BUILT_IN_UNREACHABLE:
   /* Rewrite any explicit calls to __builtin_unreachable.  */
-  if (sanitize_flags_p (SANITIZE_UNREACHABLE))
+  if (sanitize_flags_p (SANITIZE_UNREACHABLE) || flag_unreachable_traps)
return build_builtin_unreachable (loc);
   break;
 


[pushed] Darwin: Sync coverage specs with gcc/gcc.cc.

2024-04-08 Thread Iain Sandoe
Tested on x86_64 and aarch64 Darwin, pushed to trunk, thanks,
Iain

--- 8< ---

The specs for coverage ere out of date leading to test fails for
fcondition-coverage cases. Fixed by updating to match the specs
in gcc/gcc.cc.

gcc/ChangeLog:

* config/darwin.h (LINK_COMMAND_SPEC_A): Update coverage
specs.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 31019a0c49d..c09b9e9dc94 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -406,7 +406,7 @@ extern GTY(()) int darwin_ms_struct;
 %{!r:%{!nostdlib:%{!nodefaultlibs: " DARWIN_WEAK_CRTS "}}} \
 %o \
 %{!r:%{!nostdlib:%{!nodefaultlibs:\
-  %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} \
+  %{fprofile-arcs|fcondition-coverage|fprofile-generate*|coverage:-lgcov} \
   %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1): \
%{static|static-libgcc|static-libstdc++|static-libgfortran: \
  libgomp.a%s; : -lgomp }} \
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Darwin: Account for block labels in function body scans.

2024-04-05 Thread Iain Sandoe
tested on aarch64-apple-darwin21, pushed to trunk, thanks.
Iain

--- 8< ---

When we have '-O3 -g' we emit a bunch of LB{B,E} local labels which
were not currently being discarded, leading to some test fails.

Fixed by adding this case to the ignored labels.

gcc/testsuite/ChangeLog:

* lib/scanasm.exp: Add 'LB*' to the local labels that are
ignored for Darwin.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/lib/scanasm.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
index 741a5a048b8..6cf9997240d 100644
--- a/gcc/testsuite/lib/scanasm.exp
+++ b/gcc/testsuite/lib/scanasm.exp
@@ -895,7 +895,7 @@ proc configure_check-function-bodies { config } {
# example).
set up_config(fluff) {^\s*(?://)}
 } elseif { [istarget *-*-darwin*] } {
-   set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ACESV]}
+   set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ABCESV]}
 } else {
# Skip lines beginning with labels ('.L[...]:') or other directives
# ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
-- 
2.39.2 (Apple Git-143)



Re: [PATCH] libstdc++: Fix infinite loop in std::istream::ignore(n, delim) [PR93672]

2024-04-04 Thread Iain Sandoe



> On 4 Apr 2024, at 16:29, Jonathan Wakely  wrote:
> 
> I would appreciate more eyes on this to confirm my conclusions about
> negative int_type values, and the proposed fix, make sense.
> 
> Tested x86_64-linux.
> 
> -- >8 --
> 
> A negative value for the delim value passed to std::istream::ignore can
> never match any character in the stream, because the comparison is done
> using traits_type::eq_int_type(sb->sgetc(), delim) and sgetc() never
> returns negative values (except at EOF). The optimized version of
> ignore for the std::istream specialization uses traits_type::find to
> locate the delim character in the streambuf, which _can_ match a
> negative delim on platforms where char is signed, but then we do another
> comparison using eq_int_type which fails. The code then keeps looping
> forever, with traits_type::find saying the character is present and
> eq_int_type saying it's not.
> 
> A possible fix would be to check with eq_int_type after a successful
> find, to see whether we really have a match. However, that would be
> suboptimal since we know that a negative delimiter will never match
> using eq_int_type. So a better fix is to adjust the check at the top of
> the function that handles delim==eof(), so that we treat all negative
> delim values as equivalent to EOF. That way we don't bother using find
> to search for something that will never match with eq_int_type.

Is the corollary to this that a platform with signed chars can never use a
negative value as a delimiter - since that we always be treated as EOF?

- I am not sure it there’s an actual use-case where that matters, but,
Iain

> 
> The version of ignore in the primary template doesn't need a change,
> because it doesn't use traits_type::find, instead characters are
> extracted one-by-one and always matched using eq_int_type. That avoids
> the inconsistency between find and eq_int_type.
> 
> libstdc++-v3/ChangeLog:
> 
>   PR libstdc++/93672
>   * src/c++98/istream.cc (istream::ignore(streamsize, int_type)):
>   Treat all negative delimiter values as eof().
>   * testsuite/27_io/basic_istream/ignore/char/93672.cc: New test.
> ---
> libstdc++-v3/src/c++98/istream.cc |  5 -
> .../27_io/basic_istream/ignore/char/93672.cc  | 15 +++
> 2 files changed, 19 insertions(+), 1 deletion(-)
> create mode 100644 
> libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/93672.cc
> 
> diff --git a/libstdc++-v3/src/c++98/istream.cc 
> b/libstdc++-v3/src/c++98/istream.cc
> index 07ac739c26a..aa1069dea07 100644
> --- a/libstdc++-v3/src/c++98/istream.cc
> +++ b/libstdc++-v3/src/c++98/istream.cc
> @@ -112,7 +112,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> basic_istream::
> ignore(streamsize __n, int_type __delim)
> {
> -  if (traits_type::eq_int_type(__delim, traits_type::eof()))
> +  // sgetc() returns either (int_type)(unsigned char)c or -1 for EOF.
> +  // If __delim is negative, then eq_int_type(sgetc(), __delim) can only
> +  // be true for EOF, so just treat all negative values as eof().
> +  if (__delim < 0)
>   return ignore(__n);
> 
>   _M_gcount = 0;
> diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/93672.cc 
> b/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/93672.cc
> new file mode 100644
> index 000..6d11f5622c8
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/93672.cc
> @@ -0,0 +1,15 @@
> +// { dg-do run }
> +
> +#include 
> +#include 
> +
> +int main()
> +{
> +  std::istringstream in("x\xfdxxx\xfex");
> +  in.ignore(10, std::char_traits::to_int_type('\xfd'));
> +  VERIFY( in.gcount() == 2 );
> +  VERIFY( ! in.eof() );
> +  in.ignore(10, '\xfe');
> +  VERIFY( in.gcount() == 5 );
> +  VERIFY( in.eof() );
> +}
> -- 
> 2.44.0
> 



Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-04-03 Thread Iain Sandoe
Hi Richard,

> On 7 Mar 2024, at 13:40, FX Coudert  wrote:
> 
>> I think it's an obvious change ...
> 
> Thanks, pushed.
> 
> Dimitry, I suggest you post the second patch for review.

Given that the two patches here (for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632) were considered obvious - 
and are needed on release branches.

OK for backporting?

(Gerald has volunteered to do the earlier ones, I have already made/tested the 
gcc-13 case)

thanks
Iain



[PATCH] libphobos, Darwin: Enable libphobos for most Darwin.

2024-04-02 Thread Iain Sandoe
I have been building and testing D/libphobos for some time and over
some GCC and OS releases.  As discussed on IRC a while ago, I think
we're ready to enable this (it also avoids an annoying build fail at
stage 2 if one forgets to add the enable to the command line).

Also tested on x86_64 and powerpc64 linux gnu.

OK for trunk?
OK for backports?
thanks,
Iain

--- 8< ---

Earlier Darwin systems can be made to work too - but they need non-
standard 'binutils', so for now these must be enabled specifically.

libphobos/ChangeLog:

* configure.tgt: Enable libphobos for Darwin >= 12.

Signed-off-by: Iain Sandoe 
---
 libphobos/configure.tgt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libphobos/configure.tgt b/libphobos/configure.tgt
index 13879380416..7159688 100644
--- a/libphobos/configure.tgt
+++ b/libphobos/configure.tgt
@@ -27,6 +27,9 @@ case "${target}" in
   *-*-dragonfly*)
LIBPHOBOS_SUPPORTED=yes
;;
+  aarch64-*-darwin2*)
+   LIBPHOBOS_SUPPORTED=yes
+   ;;
   aarch64*-*-linux*)
LIBPHOBOS_SUPPORTED=yes
;;
@@ -58,6 +61,12 @@ case "${target}" in
   sparc*-*-solaris2.11*)
LIBPHOBOS_SUPPORTED=yes
;;
+  *-*-darwin9* | *-*-darwin1[01]*)
+   LIBDRUNTIME_ONLY=yes
+   ;;
+  x86_64-*-darwin1[2-9]* | x86_64-*-darwin2* | i?86-*-darwin1[2-7])
+   LIBPHOBOS_SUPPORTED=yes
+   ;;
   x86_64-*-freebsd* | i?86-*-freebsd*)
LIBPHOBOS_SUPPORTED=yes
;;
-- 
2.39.2 (Apple Git-143)



Ping: [PATCH] jit: Ensure ssize_t is defined.

2024-04-02 Thread Iain Sandoe


> On 29 Jan 2024, at 11:26, Iain Sandoe  wrote:

> I guess the solution here depends on the scope over which we expect
> the header to be used.
> 
>> On 28 Jan 2024, at 23:13, Iain Sandoe  wrote:
>>> On 28 Jan 2024, at 21:25, Eric Gallager  wrote:
>>> On Sun, Jan 28, 2024 at 6:45 AM Iain Sandoe  wrote:
>>>> 
>>>> Tested on i686, x86_64 Darwin, x86_64 Linux,
>>>> OK for trunk?
>>>> 
>>>> --- 8< ---
>>>> 
>>>> On some targets it seems that ssize_t is not defined by any of the
>>>> headers transitively included by .  This leads to a bootstrap
>>>> fail when jit is enabled.
>>>> 
>>>> The fix proposed here is to include sys/types.h when it is available
>>>> since that is where Posix specifies that ssize_t is defined.
>>>> 
>>>> gcc/jit/ChangeLog:
>>>> 
>>>>  * libgccjit.h: Conditionally include  where it is
>>>>  available to ensure declaration of ssize_t.
>>>> 
>>>> Signed-off-by: Iain Sandoe 
>>>> ---
>>>> gcc/jit/libgccjit.h | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>> 
>>>> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
>>>> index 235cab053e0..db4f27a48bf 100644
>>>> --- a/gcc/jit/libgccjit.h
>>>> +++ b/gcc/jit/libgccjit.h
>>>> @@ -21,6 +21,9 @@ along with GCC; see the file COPYING3.  If not see
>>>> #define LIBGCCJIT_H
>>>> 
>>>> #include 
>>>> +#if __has_include()
>>> 
>>> Is __has_include() something that we can use unconditionally?
>> 
>> Hmm.. maybe we cannot, it seems it was introduced in gcc-4.9 and we only ask
>> for 4.8, IIRC.
>> 
>> I guess HAVE_SYS_TYPES_H might be an alternative (I’ll have to retest)
> 
> Answering my own question; no that is not going to work  either since the 
> header is
> installed and config.h is not.
> 
> I guess the question is “is this header ever [meaningfully] consumed by a 
> compiler
> other than the current GCC that it supports”?
> 
> e.g. if we expected we could build libgccjit with clang in a 
> “—disable-bootstrap”
> configuration and expect that to work?
> 

this … (as attached)

> The fallback is
> #ifdef __APPLE__
> # include   /* For ssize_t.  */
> #endif


> 
> (which I will test on a number of platform versions).
> 
> since this breaks bootstrap at stage 2 on affected platform versions, so we 
> need some
> fix.



0001-jit-Ensure-ssize_t-is-defined.patch
Description: Binary data




Re: [PATCH] jit, Darwin: Implement library exports list.

2024-04-02 Thread Iain Sandoe
Hi David,

> On 25 Jan 2024, at 10:16, Iain Sandoe  wrote:
> 

>> On 24 Jan 2024, at 18:31, David Malcolm  wrote:
>> 
>> On Tue, 2024-01-16 at 11:10 +, Iain Sandoe wrote:
>>> Tested on x86_64, i686 Darwin and x86_64 Linux,
>>> OK for trunk? when ?
>>> thanks,
>>> Iain
>> 
>> Hi Iain, thanks for the patch.
>> 
>> I'll have to defer to your Darwin expertise here; given that you've
>> tested it on the above configurations I'll assume it's correct, but...
>> 
>>> 
>>> --- 8< ---
>>> 
>>> Currently, we have no exports list for libgccjit, which means that
>>> all symbols are exported, including those from libstdc++ which is
>>> linked statically into the lib.  This causes failures when the
>>> shared libstdc++ is used but some c++ symbols are satisfied from
>>> libgccjit.
>>> 
>>> This implements an export file for Darwin (which is currently
>>> manually created by cross-checking libgccjit.map).
>> 
>> ...I'm a little nervous about this; Antoyo has a number of out-of-tree
>> patches we're working towards merging, and almost all of these touch
>> libgccjit.map.
>> 
>> 
>>>  Ideally we'd
>>> script this, at some point.  
>> 
>> Yes.  How about a Python 3 script (inside "contrib", or in "gcc/jit")
>> that would do that.  
> 
> I’m not sure we want to make a build dependency on Python 3.. 
> the reason I say ‘build’ is ...
> 
>> Then whenever a patch touches libgccjit.map we'd
>> run that script to regenerate libgccjit.exp in the source tree.  I can
>> have a go at writing it, if you think that's the best way to go.
> 
> … there are two other places in the current sources where ld map files
> are converted to Darwin (and Solaris) symbol export files [libgcc, libstdc++].
> 
> In these cases, the export file is created on-the-fly at build time by scripts
> (IIRC a mixture of awk, sh, perl).
> 
> This requires more surgery to the Make stuff and that we have a suitable
> script - but it does mean that we do not need to commit the Darwin (and
> potentially Solaris) versions to the source tree.
> 
> I’m actually happy with either solution - since we do not expect this to
> be a daily occurance, we could have a maintainter’s python3 script to
> update a committed export file or we could try the mechanism used in
> the other two places (but then the script would need to use awk/sh/perl
> to avoid new build deps).
> 
>> I take it .exp is the standard extension for these exports file in the
>> Darwin world.  If so, it's a shame (but unavoidable) that it clashes
>> with the existing uses of .exp in our source tree for our
>> expect/Tcl/DejaGnu sources.
> 
> I suspect the linker will accept other extensions, although ‘exp’ is a
> convention used elsewhere, it is unfortunate that it clashes indeed.
> - let me try an alternate (e.g. .export) and report back.
> 
>> I think the patch as-is is OK for trunk now, assuming that you've
>> tested it as above.
> 
> I’m going to hold off on this for now (but do want some solution before
> 14 branches, because there are quite a few new fails from it).

It seems that we are not going to get time to implement something better
for GCC-14; this is what I applied (I renamed the extension to .exports 
which the linker is fine with) so that it is not confused with .exp files.

I guess I’ll need to do a final pass of checking I’ve copied all the syms
before 14 branches.

thanks
Iain



0001-jit-Darwin-Implement-library-exports-list.patch
Description: Binary data




[pushed] testsuite: Remove duplicate -lgcov [PR114034]

2024-04-02 Thread Iain Sandoe
Tested on x86_64, i686 Darwin and x86_64, powerpc64 linux, pushed to
trunk as obvious, thanks
Iain

--- 8< ---

Duplicate library entries now cause linker warnings with newer linker
versions on Darwin which leads to these tests regressing.  The library
is already added by the test flags so there is no need to put an extra
one in the options.

PR testsuite/114034

gcc/testsuite/ChangeLog:

* g++.dg/gcov/gcov-dump-1.C: Remove extra -lgcov.
* g++.dg/gcov/gcov-dump-2.C: Likewise.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/g++.dg/gcov/gcov-dump-1.C | 2 +-
 gcc/testsuite/g++.dg/gcov/gcov-dump-2.C | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/gcov/gcov-dump-1.C 
b/gcc/testsuite/g++.dg/gcov/gcov-dump-1.C
index f0e81e9b042..774a7269ff2 100644
--- a/gcc/testsuite/g++.dg/gcov/gcov-dump-1.C
+++ b/gcc/testsuite/g++.dg/gcov/gcov-dump-1.C
@@ -1,4 +1,4 @@
-/* { dg-options "-fprofile-generate -ftest-coverage -lgcov" } */
+/* { dg-options "-fprofile-generate -ftest-coverage " } */
 /* { dg-do run { target native } } */
 
 int value;
diff --git a/gcc/testsuite/g++.dg/gcov/gcov-dump-2.C 
b/gcc/testsuite/g++.dg/gcov/gcov-dump-2.C
index 6234a81a586..e748989d2c0 100644
--- a/gcc/testsuite/g++.dg/gcov/gcov-dump-2.C
+++ b/gcc/testsuite/g++.dg/gcov/gcov-dump-2.C
@@ -1,4 +1,4 @@
-/* { dg-options "-fprofile-generate -ftest-coverage -lgcov" } */
+/* { dg-options "-fprofile-generate -ftest-coverage " } */
 /* { dg-do run { target native } } */
 
 int value;
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Darwin: Allow for an undefined symbol [PR114036].

2024-04-02 Thread Iain Sandoe
Tested on x86_64-darwin17,21,23 and on x86_64 and powerpc64 linux gnu,
pushed to trunk, thanks
Iain

--- 8< ---

Darwin's linker defaults to requiring all symbols to be defined at
static link time (unless specifically noted or dynamic lookuo is
enabled).

For this test, we just need to note that the symbol is expected to
be undefined.

PR testsuite/114036

gcc/testsuite/ChangeLog:

* gcc.misc-tests/gcov-14.c: Allow for 'Foo' to be undefined
on Darwin link lines.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/gcc.misc-tests/gcov-14.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.misc-tests/gcov-14.c 
b/gcc/testsuite/gcc.misc-tests/gcov-14.c
index 2bebf7e4a93..61a9191c068 100644
--- a/gcc/testsuite/gcc.misc-tests/gcov-14.c
+++ b/gcc/testsuite/gcc.misc-tests/gcov-14.c
@@ -3,7 +3,7 @@
 /* { dg-do run { target native } } */
 /* { dg-options "-O2 -fprofile-arcs -ftest-coverage -fgnu89-inline" } */
 /* The following line arranges that Darwin has behavior like elf weak import.  
*/
-/* { dg-additional-options "-flat_namespace -undefined suppress" { target 
*-*-darwin* }  } */
+/* { dg-additional-options "-Wl,-U,_Foo" { target *-*-darwin* }  } */
 /* { dg-require-weak "" } */
 /* { dg-skip-if "undefined weak not supported" { { hppa*-*-hpux* } && { ! lp64 
} } } */
 /* { dg-skip-if "undefined weak not supported" { powerpc-ibm-aix* } } */
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Correct a version check.

2024-04-02 Thread Iain Sandoe
Tested on x86_64-darwin17,21,23, pushed to trunk, thanks,
Iain

--- 8< ---

When the version for dsymutil comes from a clang build, it is
of the form NNmm.pp.qq where NN and mm are the major and minor
LLVM version components.  We need to check for a major version
greater than or equal to 7 - so use 700 in the check.

gcc/ChangeLog:

* config/darwin.cc (darwin_override_options): Update the
clang major version value in the dsymutil check.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index c37a1a4756f..63b8c509405 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -3420,7 +3420,7 @@ darwin_override_options (void)
   /* External toolchains based on LLVM or clang 7+ have support for
 dwarf-4.  */
   if ((dsymutil_version.kind == LLVM && dsymutil_version.major >= 7)
- || (dsymutil_version.kind == CLANG && dsymutil_version.major >= 7))
+ || (dsymutil_version.kind == CLANG && dsymutil_version.major >= 700))
dwarf_version = 4;
   else if (dsymutil_version.kind == DWARFUTILS
   && dsymutil_version.major >= 121)
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Do not emit .macinfo when dsymutil cannot consume it.

2024-04-02 Thread Iain Sandoe
This causes quite a number of testsuite fails on systems using Xcode 15.
More significantly, it is a serious debug regression (since the entire
debug is ignored when macinfo is seen).

tested on x86_64-darwin17,21,23 with / without Xcode-15, pushed to trunk,
thanks
Iain

--- 8< ---

Some verions of dsymutil do not ignore .macinfo sections, but instead
ignore the entire debug in the file.

To avoid this total loss of debug, when we detect that the debug level
is g3 and the dsymutil version cannot support it, we reduce the level
to g2 and issue a note.

This behaviour can be overidden by -gstrict-dwarf (although the objects
will contain macinfo; dsymutil will not produce a .dSYM with it).

gcc/ChangeLog:

* config/darwin.cc (darwin_override_options): Reduce the debug
level to 2 if dsymutil cannot handle .macinfo sections.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.cc | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index 9e5d64e6f32..c37a1a4756f 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -3415,11 +3415,6 @@ darwin_override_options (void)
  global_options.x_flag_objc_abi);
 }
 
-  /* Limit DWARF to the chosen version, the linker and debug linker might not
- be able to consume newer structures.  */
-  if (!OPTION_SET_P (dwarf_strict))
-dwarf_strict = 1;
-
   if (!OPTION_SET_P (dwarf_version))
 {
   /* External toolchains based on LLVM or clang 7+ have support for
@@ -3442,6 +3437,24 @@ darwin_override_options (void)
   OPTION_SET_P (dwarf_split_debug_info) = 0;
 }
 
+  /* Cases where dsymutil will exclude files with .macinfo sections; we are
+ better off forcing the debug level to 2 than completely excluding the
+ files.  If strict dwarf is set, then emit the macinfo anyway.  */
+  if (debug_info_level == DINFO_LEVEL_VERBOSE
+  && (!OPTION_SET_P (dwarf_strict) || dwarf_strict == 0)
+  && ((dsymutil_version.kind == CLANG && dsymutil_version.major >= 1500)
+ || (dsymutil_version.kind == LLVM && dsymutil_version.major >= 15)))
+{
+  inform (input_location,
+ "%<-g3%> is not supported by the debug linker in use (set to 2)");
+  debug_info_level = DINFO_LEVEL_NORMAL;
+}
+
+  /* Limit DWARF to the chosen version, the linker and debug linker might not
+ be able to consume newer structures.  */
+  if (!OPTION_SET_P (dwarf_strict))
+dwarf_strict = 1;
+
   /* Do not allow unwind tables to be generated by default for m32.
  fnon-call-exceptions will override this, regardless of what we do.  */
   if (generating_for_darwin_version < 10
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Darwin: Update bad-mapper-1 after libiberty changes.

2024-04-02 Thread Iain Sandoe
Tested on i686-darwin9, x86_64-darwin17, 21, 23, and on x86_64 and powerpc64
linux gnu, pushed to trunk, thanks,
Iain

--- 8< ---

A recent change to libiberty has improved the process spawning on
older Darwin platforms.  This patch updates the expected test output
after the changes.

gcc/testsuite/ChangeLog:

* g++.dg/modules/bad-mapper-1.C: Update expected test output
for earlier Darwin.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/g++.dg/modules/bad-mapper-1.C | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/modules/bad-mapper-1.C 
b/gcc/testsuite/g++.dg/modules/bad-mapper-1.C
index b0b0b86c9cd..3dfb5a6073e 100644
--- a/gcc/testsuite/g++.dg/modules/bad-mapper-1.C
+++ b/gcc/testsuite/g++.dg/modules/bad-mapper-1.C
@@ -1,9 +1,9 @@
 //  { dg-additional-options "-fmodules-ts -fmodule-mapper=|this-will-not-work" 
}
 import unique1.bob;
-// { dg-error "-:failed (exec|CreateProcess|posix_spawn).*mapper.* 
.*this-will-not-work" "" { target { ! { *-*-darwin[89]* *-*-darwin10* 
hppa*-*-hpux* } } } 0 }
+// { dg-error "-:failed (exec|CreateProcess|posix_spawn).*mapper.* 
.*this-will-not-work" "" { target { ! { hppa*-*-hpux* } } } 0 }
 // { dg-prune-output "fatal error:" }
 // { dg-prune-output "failed to read" }
 // { dg-prune-output "compilation terminated" }
-// { dg-error "-:failed mapper handshake communication" "" { target { 
*-*-darwin[89]* *-*-darwin10* hppa*-*-hpux* } } 0 }
+// { dg-error "-:failed mapper handshake communication" "" { target { 
hppa*-*-hpux* } } 0 }
 // { dg-prune-output "trying to exec .this-will-not-work."  }
 // { dg-prune-output "unknown Compiled Module Interface"  }
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Darwin: Use the IOKit framework in framework-1.c [PR114049].

2024-03-19 Thread Iain Sandoe
Tested on x86_64-darwin21, a cross to powerpc-darwin9 and on
x86_64-linux-gnu, pushed to trunk (will backport to branches soon).
thanks,
Iain

--- 8< ---

The intent of the test is to show that we find a framework that
is installed in /System/Library/Frameworks when the user has added
a '-F' option.  The trick is to choose some header that is present
for all the Darwin versions we support and that does not contain any
content we cannot parse.  We had been using the Kernel framework for
this, but recent SDK versions have revealed that this is not suitable.

Replacing with a use of IOKit.

PR target/114049

gcc/testsuite/ChangeLog:

* gcc.dg/framework-1.c: Use an IOKit header instead of a
Kernel one.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/gcc.dg/framework-1.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/framework-1.c 
b/gcc/testsuite/gcc.dg/framework-1.c
index de4adc39868..fdec129a8fb 100644
--- a/gcc/testsuite/gcc.dg/framework-1.c
+++ b/gcc/testsuite/gcc.dg/framework-1.c
@@ -1,4 +1,10 @@
 /* { dg-do compile { target *-*-darwin* } } */
 /* { dg-options "-F." } */
 
-#include 
+/* The intent of the test is to show that we find a framework that
+   is installed in /System/Library/Frameworks when the user has added
+   a '-F' option.  The trick is to choose some header that is present
+   for all the Darwin versions we support and that does not contain any
+   content we cannot parse.  */
+
+#include 
-- 
2.39.2 (Apple Git-143)



[pushed] libstdc++: Sync the atomic_link_flags implementation with GCC.

2024-03-19 Thread Iain Sandoe
This was pre-approved on irc by Jonathan, tested on x86_64-linux,Darwin,
pushed to trunk, thanks,
Iain

--- 8< ---

For Darwin, in order to allow uninstalled testing, we need to provide
a '-B' option pointing to each path containing an uninstalled library
that we are using (these get appended to the embedded runpaths).

This updates the version of the atomic_link_flags proc in the libstdc++
testsuite to do the same as the one in the GCC testsuite.

libstdc++-v3/ChangeLog:

* testsuite/lib/dg-options.exp (atomic_link_flags): Emit a -B
option for the path to the uninstalled libatomic.

Signed-off-by: Iain Sandoe 
---
 libstdc++-v3/testsuite/lib/dg-options.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp 
b/libstdc++-v3/testsuite/lib/dg-options.exp
index bc387d17ed7..00ca678a53a 100644
--- a/libstdc++-v3/testsuite/lib/dg-options.exp
+++ b/libstdc++-v3/testsuite/lib/dg-options.exp
@@ -314,7 +314,7 @@ proc atomic_link_flags { paths } {
   if { [file exists "${gccpath}/libatomic/.libs/libatomic.a"]
|| [file exists 
"${gccpath}/libatomic/.libs/libatomic.${shlib_ext}"] } {
   append flags " -B${gccpath}/libatomic/ "
-  append flags " -L${gccpath}/libatomic/.libs"
+  append flags " -B${gccpath}/libatomic/.libs"
   append ld_library_path ":${gccpath}/libatomic/.libs"
   }
 } else {
-- 
2.39.2 (Apple Git-143)



[PATCH] libstdc++, Darwin: Do not use dev/null as the file for executables.

2024-03-19 Thread Iain Sandoe
Note that Windows-based platforms do something quite similar, but always
use the same (x.exe) filename.  I wonder, in passing, if that makes a
vulnerability in parallel testing (I chose to avoid the possibility for
Darwin).

Tested on x86_64-darwin21, x86_64-linux-gnu,
OK for trunk?
back-ports where applicable (I did not yet check)?
thanks
Iain

--- 8< ---

Darwin has a separate debug linker, which is invoked when the command
line contains source files and debug is enabled.

Using /dev/null as the executable name does not, therefore, work when
debug is enabled, since the debug linker does not accept /dev/null as
a valid executable name.

The leads to incorrectly UNSUPPORTED testcases because of the unintended
error result from the test compilation.

The solution here is to use a temporary file that is deleted at the
end of the test (which is the mechanism used elsewhere)

libstdc++-v3/ChangeLog:

* testsuite/lib/libstdc++.exp (v3_target_compile): Instead of
/dev/null, use a temporary file for test executables on Darwin.

Signed-off-by: Iain Sandoe 
---
 libstdc++-v3/testsuite/lib/libstdc++.exp | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp 
b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 58804ecab26..4ae4cecb169 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -615,11 +615,15 @@ proc v3_target_compile { source dest type options } {
}
 }
 
+# For Windows and Darwin we might want to create a temporary file.
+# note that it needs deleteting.
+set file_to_delete ""
 # Small adjustment for Windows hosts.
 if { $dest == "/dev/null"
  && [info exists ::env(OS)] && [string match "Windows*" $::env(OS)] } {
if { $type == "executable" } {
set dest "x.exe"
+   set file_to_delete ${dest}
} else {
# Windows uses special file named "nul" as a substitute for
# /dev/null
@@ -627,6 +631,15 @@ proc v3_target_compile { source dest type options } {
}
 }
 
+# Using /dev/null as the executable name does not work on Darwin when
+# debug is enabled, since the debug linker does not accept /dev/null as
+# a valid executable name.
+if { $dest == "/dev/null" && [istarget *-*-darwin*]
+ && $type == "executable" } {
+   set dest dev-null-[pid].exe
+   set file_to_delete ${dest}
+}
+
 lappend options "compiler=$cxx_final"
 lappend options "timeout=[timeout_value]"
 
@@ -637,7 +650,12 @@ proc v3_target_compile { source dest type options } {
 }
 
 set comp_output [target_compile $source $dest $type $options]
-
+if { $type == "executable" && $file_to_delete != "" } {
+   file delete $file_to_delete
+   if { [istarget *-*-darwin*] && [file exists $file_to_delete.dSYM] } {
+   file delete -force $file_to_delete.dSYM
+   }
+}
 return $comp_output
 }
 
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Fix bootstrap break on X86.

2024-03-16 Thread Iain Sandoe
Tested on x86_64-darwin21, pushed to the branch, thanks
Iain

--- 8< ---

The changes in r12-9536-gefcca6481eab18 mangled the whitespace in
the ENDFILE_SPEC i386/darwin.h leading to a bootstrap fail.

gcc/ChangeLog:

* config/i386/darwin.h (ENDFILE_SPEC): Fix whitespace.

Signed-off-by: Iain Sandoe 
---
 gcc/config/i386/darwin.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index 2f773924d6e..12cdc34a19e 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -109,8 +109,8 @@ along with GCC; see the file COPYING3.  If not see
 "%{!force_cpusubtype_ALL:-force_cpusubtype_ALL} "
 
 #undef ENDFILE_SPEC
-#define ENDFILE_SPEC
-\  
"%{mdaz-ftz:crtfastmath.o%s;Ofast|ffast-math|funsafe-math-optimizations:%{!mno-daz-ftz:crtfastmath.o%s}}
 \
+#define ENDFILE_SPEC \
+  
"%{mdaz-ftz:crtfastmath.o%s;Ofast|ffast-math|funsafe-math-optimizations:%{!mno-daz-ftz:crtfastmath.o%s}}
 \
%{mpc32:crtprec32.o%s} \
%{mpc64:crtprec64.o%s} \
%{mpc80:crtprec80.o%s}" TM_DESTRUCTOR
-- 
2.39.2 (Apple Git-143)



Re: [PATCH] Fix libcc1plugin and libc1plugin to avoid poisoned identifiers

2024-03-13 Thread Iain Sandoe
Hi Dimitry,

> On 7 Mar 2024, at 16:48, Dimitry Andric  wrote:
> 
> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
> 
> Use INCLUDE_VECTOR before including system.h, instead of directly
> including , to avoid running into poisoned identifiers.

I would say that the patch itself is obvious, but you have not mentioned how
it was tested?

thanks
Iain

> 
> Signed-off-by: Dimitry Andric 
> ---
> libcc1/libcc1plugin.cc | 3 +--
> libcc1/libcp1plugin.cc | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
> index 72d17c3b81c..e64847466f4 100644
> --- a/libcc1/libcc1plugin.cc
> +++ b/libcc1/libcc1plugin.cc
> @@ -32,6 +32,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -69,8 +70,6 @@
> #include "gcc-c-interface.h"
> #include "context.hh"
> 
> -#include 
> -
> using namespace cc1_plugin;
> 
> 
> diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
> index 0eff7c68d29..da68c5d0ac1 100644
> --- a/libcc1/libcp1plugin.cc
> +++ b/libcc1/libcp1plugin.cc
> @@ -33,6 +33,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -71,8 +72,6 @@
> #include "rpc.hh"
> #include "context.hh"
> 
> -#include 
> -
> using namespace cc1_plugin;
> 
> 
> -- 
> 2.43.2
> 



Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-03-06 Thread Iain Sandoe
Hi Dimitry, folks,

> On 6 Mar 2024, at 23:02, Dimitry Andric  wrote:
> 
> On 6 Mar 2024, at 15:57, FX Coudert  wrote:
>> 
>>> Hmm I recall trying it and finding a problem - was there some different fix 
>>> applied
>>> in the end?
>> 
>> The bug is still open, I don’t think a patch was applied, and I don’t find 
>> any email to the list stating what the problem could be.
> 
> The original patch (https://gcc.gnu.org/bugzilla/attachment.cgi?id=56010) 
> still applies to the master branch.

I have retested this on various Darwin versions and confirm that it fixes the 
bootstrap fail on x86_64-darwin23 and works OK on other versions (including 32b 
hosts). 

+1 for applying this soon.



the second patch really needs to be posted separately to make review easier (if 
we were not in stage 4, I’d say it’s ‘obvious’ anyway).

thanks
Iain


> It turned out there is also a related problem in libcc1plugin.cc and 
> libcp1plugin.cc , which is fixed by 
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57639 :
> 
> commit 49222b98ac8e30a4a042ada0ece3d7df93f049d2
> Author: Dimitry Andric 
> Date:   2024-03-06T23:46:27+01:00
> 
>Fix libcc1plugin and libc1plugin to use INCLUDE_VECTOR before including
>system.h, instead of directly including , to avoid running into
>poisoned identifiers.
> 
> diff --git a/libcc1/libcc1plugin.cc b/libcc1/libcc1plugin.cc
> index 72d17c3b81c..e64847466f4 100644
> --- a/libcc1/libcc1plugin.cc
> +++ b/libcc1/libcc1plugin.cc
> @@ -32,6 +32,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -69,8 +70,6 @@
> #include "gcc-c-interface.h"
> #include "context.hh"
> 
> -#include 
> -
> using namespace cc1_plugin;
> 
> 
> diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
> index 0eff7c68d29..da68c5d0ac1 100644
> --- a/libcc1/libcp1plugin.cc
> +++ b/libcc1/libcp1plugin.cc
> @@ -33,6 +33,7 @@
> #undef PACKAGE_VERSION
> 
> #define INCLUDE_MEMORY
> +#define INCLUDE_VECTOR
> #include "gcc-plugin.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -71,8 +72,6 @@
> #include "rpc.hh"
> #include "context.hh"
> 
> -#include 
> -
> using namespace cc1_plugin;
> 
> 
> 



Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

2024-03-06 Thread Iain Sandoe



> On 6 Mar 2024, at 13:54, Sam James  wrote:
> 
> FX Coudert  writes:
> 
>> I would like to patch this patch from September 2023:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631611.html
>> 
>> This bug is now hitting macOS in the latest version of Xcode (it was 
>> originally seen on freebsd).
>> I confirm that the patch is restoring bootstrap on x86_64-apple-darwin23
> 
> Iain hit an issue with it and I never got a chance to look into how to
> fix it.

Hmm I recall trying it and finding a problem - was there some different fix 
applied
in the end?

Iain

> 
>> 
>> OK to push?
>> FX
>> 
>> 
>> 
>> 
>> 
>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>> 
>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>> the ctype macros due to including safe-ctype.h before including C++
>>> standard headers such as , , etc, causes many compilation
>>> errors, similar to:
>>> 
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>> only applies to structs, variables, functions, and namespaces
>>> 546 | _LIBCPP_INLINE_VISIBILITY
>>> | ^
>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>> '_LIBCPP_INLINE_VISIBILITY'
>>> 813 | # define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>> | ^
>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>> '_LIBCPP_HIDE_FROM_ABI'
>>> 792 |
>>> __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>> _LIBCPP_VERSIONED_IDENTIFIER
>>> | ^
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>> declaration list
>>> 547 | char_type toupper(char_type __c) const
>>> | ^
>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>> provided to function-like macro invocation
>>> 553 | const char_type* toupper(char_type* __low, const
>>> char_type* __high) const
>>> | ^
>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>> macro 'toupper' defined here
>>> 146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>> | ^
>>> 
>>> This is because libc++ uses different transitive includes than
>>> libstdc++, and some of those transitive includes pull in various ctype
>>> declarations (typically via ).
>>> 
>>> There was already a special case for including  before
>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>> the same location, to fix the problem.
>>> 



Re: [C++ coroutines] Initial implementation pushed to master.

2024-03-06 Thread Iain Sandoe



> On 5 Mar 2024, at 17:31, H.J. Lu  wrote:
> 
> On Sat, Jan 18, 2020 at 4:54 AM Iain Sandoe  wrote:
>> 

>> 2020-01-18  Iain Sandoe  
>> 
>>* Makefile.in: Add coroutine-passes.o.
>>* builtin-types.def (BT_CONST_SIZE): New.
>>(BT_FN_BOOL_PTR): New.
>>(BT_FN_PTR_PTR_CONST_SIZE_BOOL): New.
>>* builtins.def (DEF_COROUTINE_BUILTIN): New.
>>* coroutine-builtins.def: New file.
>>* coroutine-passes.cc: New file.
> 
> There are
> 
>  tree res_tgt = TREE_OPERAND (gimple_call_arg (stmt, 2), 0);
>  tree _dest = destinations.get_or_insert (idx, );
>  if (existed && dump_file)
>Why does this behavior depend on dump_file?

This was checking for a potential wrong-code error during development;
there is no point in making it into a diagnostic (since the user could not fix
the problem if it happened).  I guess changing to a gcc_checking_assert()
would be reasonable but I’d prefer to do that once GCC-15 opens.

Have you found any instance where this results in a reported bug?
(I do not recall anything on my coroutines bug list that would seem to indicate 
this).

thanks for noting it.
Iain


>{
>  fprintf (
>dump_file,
>"duplicate YIELD RESUME point (" HOST_WIDE_INT_PRINT_DEC
>") ?\n",
>idx);
>  print_gimple_stmt (dump_file, stmt, 0, TDF_VOPS|TDF_MEMSYMS);
>}
>  else
>res_dest = res_tgt;
> 
> H.J.



Re: [PATCH] lto, Darwin: Fix offload section names.

2024-02-29 Thread Iain Sandoe
Hi Thomas,

> On 29 Feb 2024, at 14:37, Thomas Schwinge  wrote:

> On 2024-01-16T15:00:16+, Iain Sandoe  wrote:
>> Currently, these section names have wrong syntax for Mach-O.
>> Although they were added some time ago; recently added tests are
>> now emitting them leading to new fails on Darwin.
>> 
>> This adds a Mach-O variant for each.
> 
>> gcc/lto-section-names.h | 10 ++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/gcc/lto-section-names.h b/gcc/lto-section-names.h
>> index a743deb4efb..1cdadf36ec0 100644
>> --- a/gcc/lto-section-names.h
>> +++ b/gcc/lto-section-names.h
>> @@ -25,7 +25,11 @@ along with GCC; see the file COPYING3.  If not see
>>name for the functions and static_initializers.  For other types of
>>sections a '.' and the section type are appended.  */
>> #define LTO_SECTION_NAME_PREFIX ".gnu.lto_"
>> +#if OBJECT_FORMAT_MACHO
>> +#define OFFLOAD_SECTION_NAME_PREFIX "__GNU_OFFLD_LTO,"
>> +#else
>> #define OFFLOAD_SECTION_NAME_PREFIX ".gnu.offload_lto_"
>> +#endif
>> 
>> /* Can be either OFFLOAD_SECTION_NAME_PREFIX when we stream IR for offload
>>compiler, or LTO_SECTION_NAME_PREFIX for LTO case.  */
>> @@ -35,8 +39,14 @@ extern const char *section_name_prefix;
>> 
>> #define LTO_SEGMENT_NAME "__GNU_LTO"
>> 
>> +#if OBJECT_FORMAT_MACHO
>> +#define OFFLOAD_VAR_TABLE_SECTION_NAME "__GNU_OFFLOAD,__vars"
>> +#define OFFLOAD_FUNC_TABLE_SECTION_NAME "__GNU_OFFLOAD,__funcs"
>> +#define OFFLOAD_IND_FUNC_TABLE_SECTION_NAME "__GNU_OFFLOAD,__ind_fns"
>> +#else
>> #define OFFLOAD_VAR_TABLE_SECTION_NAME ".gnu.offload_vars"
>> #define OFFLOAD_FUNC_TABLE_SECTION_NAME ".gnu.offload_funcs"
>> #define OFFLOAD_IND_FUNC_TABLE_SECTION_NAME ".gnu.offload_ind_funcs"
>> +#endif
>> 
>> #endif /* GCC_LTO_SECTION_NAMES_H */
> 
> Just to note that, per my understanding, this will require corresponding
> changes elsewhere, once you attempt to actually enable offloading
> compilation for Darwin (which -- ;-) I suspect -- is not on your agenda
> right now):

It is disappointing, but adding offloading to Darwin seems to be out of reach 
at the moment.

AFAIK, we have no support for NVidia after macOS 10.13 and the AMD units fitted 
to new(ish)
boxes are high-end graphics cards (when last I discussed with Andrew, we could 
not conclude
whether they would be handled usefully).

Adding arbitrary extension cards is (technically) feasible to some of the 
2019-era server-style
machines - but that would still need approved and signed kernel drivers.  I 
have not looked into
whether the “studio” Arm64 machine might support such additions (the 
constraints on kernel-
side addtions would surely be even more strict on the newer OS versions).

So, indeed (for now at least) sadly, this is not even on the distant horizon :-(

Iain

> 
>$ git grep --cached -F .gnu.offload_
>gcc/config/gcn/mkoffload.cc:  if (sscanf (buf, " .section 
> .gnu.offload_vars%c", ) > 0)
>gcc/config/gcn/mkoffload.cc:  else if (sscanf (buf, " .section 
> .gnu.offload_funcs%c", ) > 0)
>gcc/config/gcn/mkoffload.cc:  /* Likewise for .gnu.offload_vars; used 
> for reverse offload. */
>gcc/config/gcn/mkoffload.cc:  else if (sscanf (buf, " .section 
> .gnu.offload_ind_funcs%c", ) > 0)
>['gcc/lto-section-names.h' adjusted per above.]
>libgcc/offloadstuff.c:#define OFFLOAD_FUNC_TABLE_SECTION_NAME 
> ".gnu.offload_funcs"
>libgcc/offloadstuff.c:#define OFFLOAD_IND_FUNC_TABLE_SECTION_NAME 
> ".gnu.offload_ind_funcs"
>libgcc/offloadstuff.c:#define OFFLOAD_VAR_TABLE_SECTION_NAME 
> ".gnu.offload_vars"
>lto-plugin/lto-plugin.c:  if (startswith (name, ".gnu.offload_lto_.opts"))
> 
> 
> Grüße
> Thomas



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-23 Thread Iain Sandoe



> On 21 Feb 2024, at 23:36, Iain Sandoe  wrote:
> 
>> On 21 Feb 2024, at 23:06, Jason Merrill  wrote:
>> 
>> On 2/20/24 00:45, Alexandre Oliva wrote:
>>> On Feb 16, 2024, Jason Merrill  wrote:
>>>> So, for stage2+, let's add just prev- libgcc.
>>> I'm pretty sure this will break bootstrap-lean where libgcc_s isn't a
>>> system library, and we're building post-bootstrap host tools :-(
>>> We need the current stage lib after the prev stage is removed.
>> 
>> That's a good point, we should make sure it doesn't break.  It looks to me 
>> like stage3-bubble removes stage1 after we're done building stage3, which 
>> should be fine, but compare removes the stage2 libgcc that we might still 
>> need to run stage3.  So indeed I guess we still want both prev and current 
>> libgcc directories in RPATH to handle the case where we've removed the 
>> previous stage, as below.
> 
> I’ll try that on darwin and aarch64 linux (I quite often need to use 
> bootstrap-lean on the latter becuase of low disk space)

I tested this addition with bootstrap-lean on i686-darwin9 (needs libgcc_s to 
boostrap), x86_64-darwin21/23 (does not use libstdc++ in the system) .. 
aarch64-linux-gnu
.. and it worked OK.

Note that the testsuite does still have some glitches with bootstrap-lean (but 
those are independent of this patch).
(we really need to adapt the idea of “host compiler/plugin compiler” to be the 
stage3 one when we bootstrap-lean).

Iain


> 
>>> I also doubt that TARGET_LIB_PATH was defined and used for no reason.
>>> My hunch is that bootstrap options and/or targets that don't have these
>>> libraries as system libraries will break in some obscure way without it.
>>> But I don't have the bandwidth to track down the history behind their
>>> inclusion.
>> 
>> That has not seemed to be the case in Iain's testing on a system without 
>> these libraries as system libraries.
> 
> Unless we change to (or add) a bootstrap where we use shared libstdc++ in the 
> compiler, I think that is the case.
> 
> As I mentioned in an earlier post, unfortunately we do not yet have a way to 
> distinguish module builds for host from module builds for target (when a 
> library is used for both - which is the case for libstdc++, libbacktrace and 
> libgrust at least),  This means that either the target library has to be 
> built without a shared version (libbacktrace does this), or the host versions 
> get built with a shared library which is not used (libstdc++) .. AFAICT the 
> only reason we build libgomp and libatomic in bootstrap phase 1 and 2 is 
> because they are dependents of the unused shared libstdc++.
> 
> Ideally, we’d fix Makefile.{tpl,def} to allow the same module to have 
> different recipies for host and target builds, but that’s also not a 5 minute 
> hack….
> 
>> I can't think of why we would need to depend on the current stage target 
>> libraries, and we already weren't depending on the previous stage target 
>> libraries.  I believe the only target code we run is tests, and if the tests 
>> need the target libraries in RPATH that should happen in the testsuite.
> 
> Which could also be improved (we do not in Dejagnu really distinguish 
> runpaths needed by the compiler from those needed by the built executables)
> 
>> It's arguable that we should pass TARGET_LIB_PATH down to make it easier for 
>> the testsuites to find them, in case they are currently relying on them 
>> being part of RPATH.  
> 
>> My impression from Iain's testing is that this isn't actually needed.
> 
> there’s actually a fair amount of specific code to locate dependent libs in 
> places (some of which I just cleaned up a bit since it was now causing fails 
> with Darwin’s new linker complaining about duplicated libs and so on).  So we 
> are not currently expecting this information to be passed down.
> 
>> I wouldn't mind keeping TARGET_LIB_PATH unused, but I'm not sure why that 
>> would be better than bringing it back if we turn out to need it.
> 
> +1
>> 
>> 
>>> I insist that the entire approach of choosing the same set of target
>>> library directories regardless of the freshness relationship between
>>> e.g. a system libstdc++ and the one we're building can't possibly be an
>>> overall improvement, it's only trading problems in some scenarios (where
>>> we're building an older libstdc++) for problems in other scenarios
>>> (where we're building a newer libstdc++).  The latter is unfortunately
>>> far more likely, which is reason enough for the current arrangement, but
>>> libstdc++ problems will likely only hit if the gap bet

Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-22 Thread Iain Sandoe
Hi Gaius,

> On 22 Feb 2024, at 18:06, Gaius Mulley  wrote:
> 
> Iain Sandoe  writes:
> 
>> Right now, AFAIK the only target runtimes used by host tools are
>> libstdc++, libgcc and libgnat.  I agree that might change with rust -
>> since the rust folks are talking about using one of the runtimes in
>> the FE, I am not aware of other language FEs requiring their targte
>> runtimes to be available to the host tools (adding Gaius in case I
>> missed something with m2 - which is quite complex inthe
>> bootstrapping).

> the m2 infrastructure translates and builds gcc/m2/gm2-libs along with
> gcc/m2/gm2-compiler and uses these objects for cc1gm2, pge, mc etc -
> rather than the library archives generated from /libgm2

If I understand this (and my builds of the m2 stuff) correctly, this is done
locally to the builds of the host-side components; in particular not controlled
by the top level Makefile.{tpl,def}?

(so that we do not see builds of libgm2 in stage1/2- but only in the
stage3-target builds?

in which case, this should be outside the scope of the patch here.

Iain



Re: [PATCH v1 03/13] aarch64: Mark x18 register as a fixed register for MS ABI

2024-02-22 Thread Iain Sandoe



> On 22 Feb 2024, at 17:45, Andrew Pinski  wrote:
> 
> On Thu, Feb 22, 2024 at 3:56 AM Richard Earnshaw (lists)
>  wrote:
>> 
>> On 21/02/2024 18:30, Evgeny Karpov wrote:
>>> 
>> +/* X18 reserved for the TEB on Windows.  */
>> +#ifdef TARGET_ARM64_MS_ABI
>> +# define FIXED_X18 1
>> +# define CALL_USED_X18 0
>> +#else
>> +# define FIXED_X18 0
>> +# define CALL_USED_X18 1
>> +#endif
>> 
>> I'm not overly keen on ifdefs like this (and the one below), it can get 
>> quite confusing if we have to support more than a couple of ABIs.  Perhaps 
>> we could create a couple of new headers, one for the EABI (which all 
>> existing targets would then need to include) and one for the MS ABI.  Then 
>> the mingw port would use that instead of the EABI header.
>> 
>> An alternative is to make all this dynamic, based on the setting of the 
>> aarch64_calling_abi enum and to make the adjustments in 
>> aarch64_conditional_register_usage.
> 
> Dynamically might be needed also if we want to support ms_abi
> attribute and/or -mabi=ms to support the wine folks.

X18 is also reserved on Darwin - in my current branch I have it non-dynamic too.
Iain

> 
> Thanks,
> Andrew Pinski
> 
>> 
>> +# define CALL_USED_X18 0
>> 
>> Is that really correct?  If the register is really reserved, but some code 
>> modifies it anyway, this will cause the compiler to restore the old value at 
>> the end of a function; generally, for a reserved register, code that knows 
>> what it's doing would want to make permanent changes to this value.
>> 
>> +#ifdef TARGET_ARM64_MS_ABI
>> +# define STATIC_CHAIN_REGNUM   R17_REGNUM
>> +#else
>> +# define STATIC_CHAIN_REGNUM   R18_REGNUM
>> +#endif
>> 
>> If we went the enum way, we'd want something like
>> 
>> #define STATIC_CHAIN_REGNUM (calling_abi == AARCH64_CALLING_ABI_MS ? 
>> R17_REGNUM : R18_REGNUM)
>> 
>> R.



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-21 Thread Iain Sandoe



> On 21 Feb 2024, at 23:06, Jason Merrill  wrote:
> 
> On 2/20/24 00:45, Alexandre Oliva wrote:
>> On Feb 16, 2024, Jason Merrill  wrote:
>>> So, for stage2+, let's add just prev- libgcc.
>> I'm pretty sure this will break bootstrap-lean where libgcc_s isn't a
>> system library, and we're building post-bootstrap host tools :-(
>> We need the current stage lib after the prev stage is removed.
> 
> That's a good point, we should make sure it doesn't break.  It looks to me 
> like stage3-bubble removes stage1 after we're done building stage3, which 
> should be fine, but compare removes the stage2 libgcc that we might still 
> need to run stage3.  So indeed I guess we still want both prev and current 
> libgcc directories in RPATH to handle the case where we've removed the 
> previous stage, as below.

I’ll try that on darwin and aarch64 linux (I quite often need to use 
bootstrap-lean on the latter becuase of low disk space).

>> I also doubt that TARGET_LIB_PATH was defined and used for no reason.
>> My hunch is that bootstrap options and/or targets that don't have these
>> libraries as system libraries will break in some obscure way without it.
>> But I don't have the bandwidth to track down the history behind their
>> inclusion.
> 
> That has not seemed to be the case in Iain's testing on a system without 
> these libraries as system libraries.

Unless we change to (or add) a bootstrap where we use shared libstdc++ in the 
compiler, I think that is the case.

As I mentioned in an earlier post, unfortunately we do not yet have a way to 
distinguish module builds for host from module builds for target (when a 
library is used for both - which is the case for libstdc++, libbacktrace and 
libgrust at least),  This means that either the target library has to be built 
without a shared version (libbacktrace does this), or the host versions get 
built with a shared library which is not used (libstdc++) .. AFAICT the only 
reason we build libgomp and libatomic in bootstrap phase 1 and 2 is because 
they are dependents of the unused shared libstdc++.

Ideally, we’d fix Makefile.{tpl,def} to allow the same module to have different 
recipies for host and target builds, but that’s also not a 5 minute hack….

> I can't think of why we would need to depend on the current stage target 
> libraries, and we already weren't depending on the previous stage target 
> libraries.  I believe the only target code we run is tests, and if the tests 
> need the target libraries in RPATH that should happen in the testsuite.

Which could also be improved (we do not in Dejagnu really distinguish runpaths 
needed by the compiler from those needed by the built executables)

> It's arguable that we should pass TARGET_LIB_PATH down to make it easier for 
> the testsuites to find them, in case they are currently relying on them being 
> part of RPATH.  

> My impression from Iain's testing is that this isn't actually needed.

there’s actually a fair amount of specific code to locate dependent libs in 
places (some of which I just cleaned up a bit since it was now causing fails 
with Darwin’s new linker complaining about duplicated libs and so on).  So we 
are not currently expecting this information to be passed down.

>  I wouldn't mind keeping TARGET_LIB_PATH unused, but I'm not sure why that 
> would be better than bringing it back if we turn out to need it.

+1
> 
> 
>> I insist that the entire approach of choosing the same set of target
>> library directories regardless of the freshness relationship between
>> e.g. a system libstdc++ and the one we're building can't possibly be an
>> overall improvement, it's only trading problems in some scenarios (where
>> we're building an older libstdc++) for problems in other scenarios
>> (where we're building a newer libstdc++).  The latter is unfortunately
>> far more likely, which is reason enough for the current arrangement, but
>> libstdc++ problems will likely only hit if the gap between system and
>> being-built libraries is large enough (say, new symbols in the newer
>> libstdc++ used by the compiler, but not available in the system
>> library).
> 
> If bootstrap doesn't actually need the target libraries, as seems to be the 
> case, then I think removing them from RPATH trades the former problem for no 
> problem.
> 
>> I'm really uncomfortable with this change, especially at this stage.
>> I'd much rather have a relatively obscure workaround for this relatively
>> obscure problem, while keeping the defaults that have accumulated lots
>> of testing on lots of configurations.
> 
> I'm happy to defer this change to GCC 15 stage 1.
> 
>> An idea that occurred to me is to have some configure option or just a
>> make variable that would be prepended to RPATH_ENVVAR, so that it would
>> preempt TARGET_LIB_PATH.  That would be a far more conservative change,
>> that I think we could make even at this stage.  WDYT?
> 
> I'm hoping for a fix that doesn't require individual users to know about 

Re: [PATCH] libgccjit: Add option to allow special characters in function names

2024-02-20 Thread Iain Sandoe



> On 20 Feb 2024, at 20:50, David Malcolm  wrote:
> 
> On Thu, 2024-02-15 at 17:08 -0500, Antoni Boucher wrote:
>> Hi.
>> This patch adds a new option to allow special characters like . and $
>> in function names.
>> This is useful to allow for mangling using those characters.
>> Thanks for the review.
> 
> Thanks for the patch.
> 
>> diff --git a/gcc/jit/docs/topics/contexts.rst 
>> b/gcc/jit/docs/topics/contexts.rst
>> index 10a0e50f9f6..4af75ea7418 100644
>> --- a/gcc/jit/docs/topics/contexts.rst
>> +++ b/gcc/jit/docs/topics/contexts.rst
>> @@ -453,6 +453,10 @@ Boolean options
>>  If true, the :type:`gcc_jit_context` will not clean up intermediate 
>> files
>>  written to the filesystem, and will display their location on stderr.
>> 
>> +  .. macro:: GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
>> +
>> + If true, allow special characters like . and $ in function names.
> 
> The documentation and the comment in libgccjit.h say:
>  "allow special characters like . and $ in function names."
> and on reading the implementation, the special characters are exactly
> '.' and '$'.
> 
> The API seems rather arbitrary and inflexible to me; why the choice of
> those characters?  Presumably those are the ones that Rust's mangling
> scheme uses, but do other mangling schemes require other chars?
> 
> How about an API for setting the valid chars, something like:
> 
> extern void
> gcc_jit_context_set_valid_symbol_chars (gcc_jit_context *ctxt,
>const char *chars);
> 
> to specify the chars that are valid in addition to underscore and
> alphanumeric.
> 
> In your case you'd call:
> 
>  gcc_jit_context_set_valid_symbol_chars (ctxt, ".$");
> 
> Or is that overkill?

If we ever wanted to support objective-c (NeXT runtime) then we’d need to
be able to support +,-,[,] space and : at least.  The interesting thing there is
that most assemblers do not support that either (and the symbols then need
to be quoted into the assembler) .

So, it’s not (IMO) overkill considering at least one potential extension.

Iain

> 
> Dave
> 



[PATCH] libgcc, aarch64: Allow for BE platforms in heap trampolines.

2024-02-20 Thread Iain Sandoe
Andrew Pinski pointed out on irc, that the current implementation of the
heap trampoline code fragment would make the instruction byte order follow
memory byte order for BE AArch64, which is not what is required.

This patch revises the initializers so that instruction byte order is
independent of memory byte order.

I have tested this on aarch64-linux-gnu, aarch64-darwin and on a cross to
aarch64_be-linux-gnu (including compile tests on the latter, but I have no
way, at present, to carry out execute tests).

(Note that this patch is applied on top of the one for PR113971).

OK for trunk, or what would be a way forward?
thanks
Iain 

--- 8< ---

This arranges that the byte order of the instruction sequences is
independent of the byte order of memory.

libgcc/ChangeLog:

* config/aarch64/heap-trampoline.c
(aarch64_trampoline_insns): Arrange to encode instructions as a
byte array so that the order is independent of memory byte order.
(struct aarch64_trampoline): Likewise.

Signed-off-by: Iain Sandoe 
---
 libgcc/config/aarch64/heap-trampoline.c | 30 -
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/libgcc/config/aarch64/heap-trampoline.c 
b/libgcc/config/aarch64/heap-trampoline.c
index 1e3460b1601..885df629da7 100644
--- a/libgcc/config/aarch64/heap-trampoline.c
+++ b/libgcc/config/aarch64/heap-trampoline.c
@@ -30,23 +30,23 @@ void __gcc_nested_func_ptr_created (void *chain, void 
*func, void *dst);
 void __gcc_nested_func_ptr_deleted (void);
 
 #if defined(__linux__)
-static const uint32_t aarch64_trampoline_insns[] = {
-  0xd503245f, /* hint34 */
-  0x58b1, /* ldr x17, .+20 */
-  0x58d2, /* ldr x18, .+24 */
-  0xd61f0220, /* br  x17 */
-  0xd5033f9f, /* dsb sy */
-  0xd5033fdf /* isb */
+static const unsigned char aarch64_trampoline_insns[6][4] = {
+  {0x5f, 0x24, 0x03, 0xd5}, /* hint34 */
+  {0xb1, 0x00, 0x00, 0x58}, /* ldr x17, .+20 */
+  {0xd2, 0x00, 0x00, 0x58}, /* ldr x18, .+24 */
+  {0x20, 0x02, 0x1f, 0xd6}, /* br  x17 */
+  {0x9f, 0x3f, 0x03, 0xd5}, /* dsb sy */
+  {0xdf, 0x3f, 0x03, 0xd5} /* isb */
 };
 
 #elif __APPLE__
-static const uint32_t aarch64_trampoline_insns[] = {
-  0xd503245f, /* hint34 */
-  0x58b1, /* ldr x17, .+20 */
-  0x58d0, /* ldr x16, .+24 */
-  0xd61f0220, /* br  x17 */
-  0xd5033f9f, /* dsb sy */
-  0xd5033fdf /* isb */
+static const unsigned char aarch64_trampoline_insns[6][4] = {
+  {0x5f, 0x24, 0x03, 0xd5}, /* hint34 */
+  {0xb1, 0x00, 0x00, 0x58}, /* ldr x17, .+20 */
+  {0xd0, 0x00, 0x00, 0x58}, /* ldr x16, .+24 */
+  {0x20, 0x02, 0x1f, 0xd6}, /* br  x17 */
+  {0x9f, 0x3f, 0x03, 0xd5}, /* dsb sy */
+  {0xdf, 0x3f, 0x03, 0xd5} /* isb */
 };
 
 #else
@@ -54,7 +54,7 @@ static const uint32_t aarch64_trampoline_insns[] = {
 #endif
 
 struct aarch64_trampoline {
-  uint32_t insns[6];
+  unsigned char insns[6][4];
   void *func_ptr;
   void *chain_ptr;
 };
-- 
2.39.2 (Apple Git-143)



[PATCH] aarch64: Allow aarch64-linux-muscl for heap trampolines [PR113971].

2024-02-20 Thread Iain Sandoe
Tested on aarch64-linux-gnu, aarch64-darwin by me and on aarch64-linux-musl
by Sam James (thanks!).  OK for trunk?
thanks
Iain

--- 8< ---


This allows the same trampoline pattern to be used on all linux variants
rather than restricting it to linux gnu.

PR target/113971.

libgcc/ChangeLog:

* config/aarch64/heap-trampoline.c: Allow all linux variants.

Signed-off-by: Iain Sandoe 
---
 libgcc/config/aarch64/heap-trampoline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgcc/config/aarch64/heap-trampoline.c 
b/libgcc/config/aarch64/heap-trampoline.c
index 9d5b19983b1..1e3460b1601 100644
--- a/libgcc/config/aarch64/heap-trampoline.c
+++ b/libgcc/config/aarch64/heap-trampoline.c
@@ -29,7 +29,7 @@ void *allocate_trampoline_page (void);
 void __gcc_nested_func_ptr_created (void *chain, void *func, void *dst);
 void __gcc_nested_func_ptr_deleted (void);
 
-#if defined(__gnu_linux__)
+#if defined(__linux__)
 static const uint32_t aarch64_trampoline_insns[] = {
   0xd503245f, /* hint34 */
   0x58b1, /* ldr x17, .+20 */
@@ -82,7 +82,7 @@ allocate_trampoline_page (void)
 {
   void *page;
 
-#if defined(__gnu_linux__)
+#if defined(__linux__)
   page = mmap (0, getpagesize (), PROT_WRITE | PROT_EXEC,
   MAP_ANON | MAP_PRIVATE, 0, 0);
 #elif __APPLE__
-- 
2.39.2 (Apple Git-143)



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-19 Thread Iain Sandoe



> On 16 Feb 2024, at 21:05, Jason Merrill  wrote:
> 
> On 2/14/24 18:33, Iain Sandoe wrote:
>>> On 14 Feb 2024, at 22:59, Iain Sandoe  wrote:
>>>> On 12 Feb 2024, at 19:59, Jason Merrill  wrote:
>>>> 
>>>> On 2/10/24 07:30, Iain Sandoe wrote:
>>>>>> On 10 Feb 2024, at 12:07, Jason Merrill  wrote:
>>>>>> 
>>>>>> On 2/10/24 05:46, Iain Sandoe wrote:
>>>>>>>> On 9 Feb 2024, at 23:21, Iain Sandoe  wrote:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>>>>>>>>>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>>>>>>>>>> 
>>>>>>>>>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>>>>>>>>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we 
>>>>>>>>>>>>> want the just-built gnat1 to find the system libgcc.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> In stage 2, when we build with the stage 1 gcc, we want the 
>>>>>>>>>>>>> just-built gnat1 to find the stage 1 libgcc.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> In neither case do we want it to find the libgcc from the current 
>>>>>>>>>>>>> stage.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> So it seems to me that what we want is for stage2+ 
>>>>>>>>>>>>> LD_LIBRARY_PATH to include the TARGET_LIB_PATH from the previous 
>>>>>>>>>>>>> stage.  Something like the below, on top of the earlier patch.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Does this make sense?  Does it work on Darwin?
>>>>>>>>>>>> 
>>>>>>>>>>>> Oops, that was broken, please consider this one instead:
>>>>>>>>>>> Yes, this one makes sense (and the current code would not work 
>>>>>>>>>>> since it lacks the prev- prefix on TARGET_LIB_PATH).
>>>>>>>>>> 
>>>>>>>>>> Indeed, that seems like evidence that the only element of 
>>>>>>>>>> TARGET_LIB_PATH that has been useful in HOST_EXPORTS is the prev- 
>>>>>>>>>> part of HOST_LIB_PATH_gcc.
>>>>>>>>>> 
>>>>>>>>>> So, here's another patch that just includes that for post-stage1:
>>>>>>>>>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
>>>>>>>>> 
>>>>>>>>> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
>>>>>>>>> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH 
>>>>>>>>> for it to succeed so,
>>>>>>>>> presumably, the post stage1 exports are not being forwarded to that 
>>>>>>>>> build.  I’ll try to analyze what
>>>>>>>>> exactly is failing.
>>>>>>>> 
>>>>>>>> The fail is occurring in the target libada build; so, I suppose, one 
>>>>>>>> might say it’s reasonable that it
>>>>>>>> requires this host path to be added to the target exports since it’s a 
>>>>>>>> host library used during target
>>>>>>>> builds (or do folks expect the host exports to be made for target lib 
>>>>>>>> builds as well?)
>>>>>>>> 
>>>>>>>> Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this
>>>>>>> Hmm this is still not right, in this case, I think it should actually 
>>>>>>> be the “just built” directory;
>>>>>>> - if we have a tool that depends on host libraries (that happen to be 
>>>>>>> also target ones),
>>>>>>>  then those libraries have to be built before the tool so that they can 
>>>>>>> be linked 

[PATCH] libstdc++, Darwin: Handle a linker warning [PR112397].

2024-02-18 Thread Iain Sandoe
Tested on i686-darwin9, x86_64-darwin14,17,19,21,23, x86_64-linux,
aarch64-linux-gnu,

OK for trunk?
eventual back-ports?
thanks
Iain

--- 8< ---

Darwin's linker warns when we make a direct branch to code that is
in a weak definition (citing that if a different implementation of
the weak function is chosen by the dynamic linker this would be an
error).

As the analysis in the PR shows, this can happen when we have hot/
cold partitioning and there is an error path that is primarily cold
but makes use of epilogue code in the hot section.  In this simple
case, we can easily deduce that the code is in fact safe; however
that is not something we can realistically implement in the linker.

Since the user-replaceable allocators are implemented using weak
definitions, this is a warning that is frequently flagged up in both
the testsuite and end-user code.

The chosen solution here is to suppress the hot/cold partitioning for
these cases (it is unlikely to impact performance much c.f. the
actual allocation).

PR target/112397

libstdc++-v3/ChangeLog:

* configure: Regenerate.
* configure.ac: Detect if we are building for Darwin.
* libsupc++/Makefile.am: If we are building for Darwin, then
suppress hot/cold partitioning for the array allocators.
* libsupc++/Makefile.in: Regenerated.

Signed-off-by: Iain Sandoe 
Co-authored-by: Jonathan Wakely 
---
 libstdc++-v3/configure | 35 +++---
 libstdc++-v3/configure.ac  |  6 +
 libstdc++-v3/libsupc++/Makefile.am |  8 +++
 libstdc++-v3/libsupc++/Makefile.in |  6 +
 4 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index c68cac4f345..37396bd6ebb 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -109,6 +109,12 @@ ACX_LT_HOST_FLAGS
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)
 AM_CONDITIONAL([ENABLE_DARWIN_AT_RPATH], [test x$enable_darwin_at_rpath = 
xyes])
+os_is_darwin=no
+case ${host_os} in
+  darwin*) os_is_darwin=yes ;;
+  *) ;;
+esac
+AM_CONDITIONAL([OS_IS_DARWIN], [test x${os_is_darwin} = xyes])
 
 if test "$enable_vtable_verify" = yes; then
   predep_objects_CXX="${predep_objects_CXX} 
${glibcxx_builddir}/../libgcc/vtv_start.o"
diff --git a/libstdc++-v3/libsupc++/Makefile.am 
b/libstdc++-v3/libsupc++/Makefile.am
index d0e1618507e..e151ce7a1fe 100644
--- a/libstdc++-v3/libsupc++/Makefile.am
+++ b/libstdc++-v3/libsupc++/Makefile.am
@@ -132,6 +132,14 @@ atomicity_file = 
${glibcxx_srcdir}/$(ATOMICITY_SRCDIR)/atomicity.h
 atomicity.cc: ${atomicity_file}
$(LN_S) ${atomicity_file} ./atomicity.cc || true
 
+if OS_IS_DARWIN
+# See PR 112397
+new_opvnt.lo: new_opvnt.cc
+   $(LTCXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
+new_opvnt.o: new_opvnt.cc
+   $(CXXCOMPILE) -fno-reorder-blocks-and-partition -I. -c $<
+endif
+
 # AM_CXXFLAGS needs to be in each subdirectory so that it can be
 # modified in a per-library or per-sub-library way.  Need to manually
 # set this option because CONFIG_CXXFLAGS has to be after
-- 
2.39.2 (Apple Git-143)



[PATCH] libiberty: Fix error return value in pex_unix_exec_child [PR113957].

2024-02-16 Thread Iain Sandoe
tested on x86_64-darwin, so far. OK for trunk if regression test is
successful on Linux too?
thanks
Iain

--- 8< ---

r14-5310-g879cf9ff45d940 introduced some new handling for spawning sub
processes.  The return value from the generic exec_child is examined
and needs to be < 0 to signal an error. However, the unix flavour of
this routine is returning the PID value set from the posix_spawn{p}.

This latter value is undefined per the manual pages for both Darwin
and Linux, and it seems Darwin, at least, sets the value to some
usually positive number (presumably the PID that would have been used
if the fork had succeeded).

The fix proposed here is to set the pid = -1 in the relevant error
paths.

PR other/113957

libiberty/ChangeLog:

* pex-unix.c (pex_unix_exec_child): Set pid = -1 in the error
paths, since that is used to signal an erroneous outcome for
the routine.

Signed-off-by: Iain Sandoe 
---
 libiberty/pex-unix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index af98062a94c..f3a1cc95ada 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -695,6 +695,7 @@ pex_unix_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED,
{
  *err = ret;
  *errmsg = "posix_spawnp";
+ pid = -1; /* The value of pid is unspecified on failure.  */
  goto exit;
}
 }
@@ -705,6 +706,7 @@ pex_unix_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED,
{
  *err = ret;
  *errmsg = "posix_spawn";
+ pid = -1;
  goto exit;
}
 }
-- 
2.39.2 (Apple Git-143)



Re: [PATCH] aarch64, acle header: Cast uint64_t pointers to DIMode.

2024-02-16 Thread Iain Sandoe



> On 15 Feb 2024, at 18:05, Richard Sandiford  wrote:
> 
> Iain Sandoe  writes:
>>> On 5 Feb 2024, at 14:56, Iain Sandoe  wrote:
>>> 
>>> Tested on aarch64-linux,darwin and a cross from aarch64-darwin to linux,
>>> OK for trunk, or some alternative is needed?
>> 
>> Hmm.. apparently, this fails the linaro pre-commit CI for g++ with:
>> error: invalid conversion from 'long int*' to 'long unsigned int*' 
>> [-fpermissive]
>> 
>> So, I guess some alternative is needed, advice welcome,
> 
> The builtins are registered with:
> 
> static void
> aarch64_init_rng_builtins (void)
> {
>  tree unsigned_ptr_type = build_pointer_type (unsigned_intDI_type_node);
>  ...
> 
> Does it work if you change unsigned_intDI_type_node to
> get_typenode_from_name (UINT64_TYPE)?

Yes, that works fine; tested on aarch64-linux and aarch64-darwin.

revised, as below,
OK for trunk?
Iain


Subject: [PATCH] aarch64: Register rng builtins with uint64_t pointers.

Currently, these are registered as unsigned_intDI_type_node which is not
necessarily the same type definition as uint64_t.  On platforms where these
differ that causes fails in consuming the arm_acle.h header.

gcc/ChangeLog:

* config/aarch64/aarch64-builtins.cc (aarch64_init_rng_builtins):
Register these builtins with a pointer to uint64_t rather than unsigned
DI mode.
---
 gcc/config/aarch64/aarch64-builtins.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/config/aarch64/aarch64-builtins.cc 
b/gcc/config/aarch64/aarch64-builtins.cc
index e211a7271ba..1330558f109 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -1759,7 +1759,8 @@ aarch64_init_tme_builtins (void)
 static void
 aarch64_init_rng_builtins (void)
 {
-  tree unsigned_ptr_type = build_pointer_type (unsigned_intDI_type_node);
+  tree unsigned_ptr_type
+= build_pointer_type (get_typenode_from_name (UINT64_TYPE));
   tree ftype
 = build_function_type_list (integer_type_node, unsigned_ptr_type, NULL);
   aarch64_builtin_decls[AARCH64_BUILTIN_RNG_RNDR]
-- 
2.39.2 (Apple Git-143)




Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-14 Thread Iain Sandoe


> On 14 Feb 2024, at 22:59, Iain Sandoe  wrote:

>> On 12 Feb 2024, at 19:59, Jason Merrill  wrote:
>> 
>> On 2/10/24 07:30, Iain Sandoe wrote:
>>>> On 10 Feb 2024, at 12:07, Jason Merrill  wrote:
>>>> 
>>>> On 2/10/24 05:46, Iain Sandoe wrote:
>>>>>> On 9 Feb 2024, at 23:21, Iain Sandoe  wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>>>>>>>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>>>>>>>> 
>>>>>>>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>>>>>>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we 
>>>>>>>>>>> want the just-built gnat1 to find the system libgcc.
>>>>>>>>>>> 
>>>>>>>>>>> In stage 2, when we build with the stage 1 gcc, we want the 
>>>>>>>>>>> just-built gnat1 to find the stage 1 libgcc.
>>>>>>>>>>> 
>>>>>>>>>>> In neither case do we want it to find the libgcc from the current 
>>>>>>>>>>> stage.
>>>>>>>>>>> 
>>>>>>>>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH 
>>>>>>>>>>> to include the TARGET_LIB_PATH from the previous stage.  Something 
>>>>>>>>>>> like the below, on top of the earlier patch.
>>>>>>>>>>> 
>>>>>>>>>>> Does this make sense?  Does it work on Darwin?
>>>>>>>>>> 
>>>>>>>>>> Oops, that was broken, please consider this one instead:
>>>>>>>>> Yes, this one makes sense (and the current code would not work since 
>>>>>>>>> it lacks the prev- prefix on TARGET_LIB_PATH).
>>>>>>>> 
>>>>>>>> Indeed, that seems like evidence that the only element of 
>>>>>>>> TARGET_LIB_PATH that has been useful in HOST_EXPORTS is the prev- part 
>>>>>>>> of HOST_LIB_PATH_gcc.
>>>>>>>> 
>>>>>>>> So, here's another patch that just includes that for post-stage1:
>>>>>>>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
>>>>>>> 
>>>>>>> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
>>>>>>> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for 
>>>>>>> it to succeed so,
>>>>>>> presumably, the post stage1 exports are not being forwarded to that 
>>>>>>> build.  I’ll try to analyze what
>>>>>>> exactly is failing.
>>>>>> 
>>>>>> The fail is occurring in the target libada build; so, I suppose, one 
>>>>>> might say it’s reasonable that it
>>>>>> requires this host path to be added to the target exports since it’s a 
>>>>>> host library used during target
>>>>>> builds (or do folks expect the host exports to be made for target lib 
>>>>>> builds as well?)
>>>>>> 
>>>>>> Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this
>>>>> Hmm this is still not right, in this case, I think it should actually be 
>>>>> the “just built” directory;
>>>>> - if we have a tool that depends on host libraries (that happen to be 
>>>>> also target ones),
>>>>>  then those libraries have to be built before the tool so that they can 
>>>>> be linked to it.
>>>>>  (we specially copy libgcc* and the CRTs to gcc/ to allow for this case)
>>>>> - there is no prev-gcc in cross and —disable-bootstrap builds, but the 
>>>>> tool will still be
>>>>>   linked to the just-built host libraries (which will also be installed).
>>>>> So, I think we have to add HOST_LIB_PATH_gcc to HOST_LIB_PATH
>>>>> and HOST_PREV_LIB_PATH_gcc to POSTSTAGE1_HOST_EXPORTS (as per this patch).
>>>> 
>>>> I don't follow.  In a cross build, host libraries are a different 
>>>

Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-14 Thread Iain Sandoe



> On 12 Feb 2024, at 19:59, Jason Merrill  wrote:
> 
> On 2/10/24 07:30, Iain Sandoe wrote:
>>> On 10 Feb 2024, at 12:07, Jason Merrill  wrote:
>>> 
>>> On 2/10/24 05:46, Iain Sandoe wrote:
>>>>> On 9 Feb 2024, at 23:21, Iain Sandoe  wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>>>>>>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>>>>>>> 
>>>>>>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>>>>>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we 
>>>>>>>>>> want the just-built gnat1 to find the system libgcc.
>>>>>>>>>> 
>>>>>>>>>> In stage 2, when we build with the stage 1 gcc, we want the 
>>>>>>>>>> just-built gnat1 to find the stage 1 libgcc.
>>>>>>>>>> 
>>>>>>>>>> In neither case do we want it to find the libgcc from the current 
>>>>>>>>>> stage.
>>>>>>>>>> 
>>>>>>>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH 
>>>>>>>>>> to include the TARGET_LIB_PATH from the previous stage.  Something 
>>>>>>>>>> like the below, on top of the earlier patch.
>>>>>>>>>> 
>>>>>>>>>> Does this make sense?  Does it work on Darwin?
>>>>>>>>> 
>>>>>>>>> Oops, that was broken, please consider this one instead:
>>>>>>>> Yes, this one makes sense (and the current code would not work since 
>>>>>>>> it lacks the prev- prefix on TARGET_LIB_PATH).
>>>>>>> 
>>>>>>> Indeed, that seems like evidence that the only element of 
>>>>>>> TARGET_LIB_PATH that has been useful in HOST_EXPORTS is the prev- part 
>>>>>>> of HOST_LIB_PATH_gcc.
>>>>>>> 
>>>>>>> So, here's another patch that just includes that for post-stage1:
>>>>>>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
>>>>>> 
>>>>>> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
>>>>>> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for 
>>>>>> it to succeed so,
>>>>>> presumably, the post stage1 exports are not being forwarded to that 
>>>>>> build.  I’ll try to analyze what
>>>>>> exactly is failing.
>>>>> 
>>>>> The fail is occurring in the target libada build; so, I suppose, one 
>>>>> might say it’s reasonable that it
>>>>> requires this host path to be added to the target exports since it’s a 
>>>>> host library used during target
>>>>> builds (or do folks expect the host exports to be made for target lib 
>>>>> builds as well?)
>>>>> 
>>>>> Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this
>>>> Hmm this is still not right, in this case, I think it should actually be 
>>>> the “just built” directory;
>>>>  - if we have a tool that depends on host libraries (that happen to be 
>>>> also target ones),
>>>>   then those libraries have to be built before the tool so that they can 
>>>> be linked to it.
>>>>   (we specially copy libgcc* and the CRTs to gcc/ to allow for this case)
>>>>  - there is no prev-gcc in cross and —disable-bootstrap builds, but the 
>>>> tool will still be
>>>>linked to the just-built host libraries (which will also be installed).
>>>> So, I think we have to add HOST_LIB_PATH_gcc to HOST_LIB_PATH
>>>> and HOST_PREV_LIB_PATH_gcc to POSTSTAGE1_HOST_EXPORTS (as per this patch).
>>> 
>>> I don't follow.  In a cross build, host libraries are a different 
>>> architecture from target libraries, and certainly can't be linked into host 
>>> binaries.
>>> 
>>> In a disable-bootstrap build, even before my change TARGET_LIB_PATH isn't 
>>> added to RPATH_ENVVAR, since that has been guarded with @if gcc-bootstrap.
>>> 
>>> So in a bootstrap build, it shouldn't be needed for stage1 either.  And for 
>>> stage2, the one we need is from stage1, that matches the compiler we're 
>>> building host tools with.
>>> 
>>> What am I missing?
>> nothing, I was off on a tangent about the cross/non-bootstrap, sorry about 
>> that.
>> However, when doing target builds (the previous point) it seems we do have 
>> to make provision for gnat1 to find libgcc_s, and, at present, it seems that 
>> only the target exports are active.
> 
> Ah, I see: When building target libraries in stage2, we run the stage2 
> compiler that needs the stage1 libgcc_s, but we don't have the HOST_EXPORTS 
> because we're building target code, so we also need to get the libgcc path 
> into TARGET_EXPORTS.
> 
> Since TARGET_LIB_PATH is only added when gcc-bootstrap, I guess the previous 
> libgcc is the only piece needed in TARGET_EXPORTS as well.  So, how about 
> this version of the patch?

I tested this one on an affected platform version with and without 
—enable-host-shared and for all languages (less go which is not yet supported). 
 It works for me, thanks,
Iain



> 
> Jason<0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>



Re: [PATCH v2] x86: Support x32 and IBT in heap trampoline

2024-02-14 Thread Iain Sandoe



> On 14 Feb 2024, at 18:12, H.J. Lu  wrote:
> 
> On Tue, Feb 13, 2024 at 8:46 AM Jakub Jelinek  wrote:
>> 
>> On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote:
>>> Add x32 and IBT support to x86 heap trampoline implementation with a
>>> testcase.
>>> 
>>> 2024-02-13  Jakub Jelinek  
>>>  H.J. Lu  
>>> 
>>> libgcc/
>>> 
>>>  PR target/113855
>>>  * config/i386/heap-trampoline.c (trampoline_insns): Add IBT
>>>  support and pad to the multiple of 4 bytes.  Use movabsq
>>>  instead of movabs in comments.  Add -mx32 variant.
>>> 
>>> gcc/testsuite/
>>> 
>>>  PR target/113855
>>>  * gcc.dg/heap-trampoline-1.c: New test.
>>>  * lib/target-supports.exp (check_effective_target_heap_trampoline):
>>>  New.
>> 
>> LGTM, but please give Iain a day or two to chime in.
>> 
>>Jakub
>> 
> 
> I am checking it in today.

I have just one question;

 from your patch the use of endbr* seems to be unconditionally based on the
 flags used to build libgcc.

 However, I was expecting that the use of extended trampolines like this would
 depend on command line flags used to compile the end-user’s code.

 As per the discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113855#c4
 I was expecting that we would need to extend this implementation to cover more
 cases (i.e. the GCC-14 implementation is “base”).  

 any comments?
Iain

 
> 
> -- 
> H.J.



Re: [PATCH] x86, libgcc: Implement ia32 basic heap trampoline [PR113855].

2024-02-10 Thread Iain Sandoe



> On 10 Feb 2024, at 17:46, Jakub Jelinek  wrote:
> 
> On Sat, Feb 10, 2024 at 05:14:44PM +0000, Iain Sandoe wrote:
>>  PR target/113855
>> 
>> gcc/ChangeLog:
>> 
>>  * config/i386/darwin.h (DARWIN_HEAP_T_LIB): Moved to be
>>  available to all sub-targets.
>>  * config/i386/darwin32-biarch.h (DARWIN_HEAP_T_LIB): Delete.
>>  * config/i386/darwin64-biarch.h (DARWIN_HEAP_T_LIB): Delete.
>> 
>> libgcc/ChangeLog:
>> 
>>  * config.host: Add trampoline support to x?86-linux.
>>  * config/i386/heap-trampoline.c (trampoline_insns): Provide
>>  a variant for IA32.
>>  (union ix86_trampoline): Likewise.
>>  (__gcc_nested_func_ptr_created): Implement a basic trampoline
>>  for IA32.
> 
> LGTM.
> 
> I bet it probably doesn't work properly for -mx32 (which defines
> __x86_64__), CCing H.J. on that, but that is a preexisting issue
> (and I don't have any experience with it; I guess one would either
> need to add 4 bytes of padding after the func_ptr so that those
> bits remain zeros as sizeof (void *) is 4, but presumably it would be
> better to just use movl (but into %r10) and maybe the jmpl instead
> of movabsq.

It seems that ix86_trampoline_init flips codegen on TARGET_64BIT
so perhaps if we make it __x86_64__ && __LP64__?

I had also considered maybe forcing the alignment of the trampoline
allocations to a cache line (or maybe at least half a cache line).

These considerations are also something to add to the improvements
we can make to generalize the handling.

For now, assuming remaining testing does not throw up any new
issues, I’ll apply this to make progress on the PR.

thanks
Iain

> 
>   Jakub
> 



[PATCH] x86, libgcc: Implement ia32 basic heap trampoline [PR113855].

2024-02-10 Thread Iain Sandoe
I have so far tested this on i686-darwin17 and on x86_64-linux (with 32b
multilib )with the following permutations:
C (dg.exp=*nest*), Ada : 
\{-m64,-m32\}\{,-ftrampoline-impl=heap\}\{,-shared-libgcc\}
Fortran \{-m64,-m32\}\{,-ftrampoline-impl=heap\}

The only fails [gnat] are expected (scanning for GNU-stack when the impl. is 
heap).

Testing to follow on i686-darwin9 (32b kernel) and x86_64-darwin17 with the
corresponding multilibs.

I do not have ready access to an i686-linux host.

How does this look to resolve the PR?
Iain

--- 8< ---

The initial heap trampoline implementation was targeting 64b
platforms.  As the PR demonstrates this creates an issue where it
is expected that the same symbols are exported for 32 and 64b.

Rather than conditionalize the exports and code-gen on x86_64,
this patch provides a basic implementation of the IA32 trampoline.

This also avoids potential user confusion, when a 32b target has
64b multilibs, and vice versa; which is the case for Darwin.

PR target/113855

gcc/ChangeLog:

* config/i386/darwin.h (DARWIN_HEAP_T_LIB): Moved to be
available to all sub-targets.
* config/i386/darwin32-biarch.h (DARWIN_HEAP_T_LIB): Delete.
* config/i386/darwin64-biarch.h (DARWIN_HEAP_T_LIB): Delete.

libgcc/ChangeLog:

* config.host: Add trampoline support to x?86-linux.
* config/i386/heap-trampoline.c (trampoline_insns): Provide
a variant for IA32.
(union ix86_trampoline): Likewise.
(__gcc_nested_func_ptr_created): Implement a basic trampoline
for IA32.
---
 gcc/config/i386/darwin.h |  3 ++-
 gcc/config/i386/darwin32-biarch.h|  3 ---
 gcc/config/i386/darwin64-biarch.h|  3 ---
 libgcc/config.host   |  1 +
 libgcc/config/i386/heap-trampoline.c | 40 +---
 5 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/gcc/config/i386/darwin.h b/gcc/config/i386/darwin.h
index 8e64b4e9b5f..bf9c45d70bb 100644
--- a/gcc/config/i386/darwin.h
+++ b/gcc/config/i386/darwin.h
@@ -119,9 +119,10 @@ along with GCC; see the file COPYING3.  If not see
 /* We default to x86_64 for single-arch builds, bi-arch overrides.  */
 #define DARWIN_ARCH_SPEC "x86_64"
 #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC
+#endif
+
 #undef DARWIN_HEAP_T_LIB
 #define DARWIN_HEAP_T_LIB " -lheapt_w "
-#endif
 
 #undef SUBTARGET_EXTRA_SPECS
 #define SUBTARGET_EXTRA_SPECS   \
diff --git a/gcc/config/i386/darwin32-biarch.h 
b/gcc/config/i386/darwin32-biarch.h
index 2180f5a5352..051ad12b425 100644
--- a/gcc/config/i386/darwin32-biarch.h
+++ b/gcc/config/i386/darwin32-biarch.h
@@ -27,9 +27,6 @@ along with GCC; see the file COPYING3.  If not see
 #undef  DARWIN_SUBARCH_SPEC
 #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC
 
-#undef DARWIN_HEAP_T_LIB
-#define DARWIN_HEAP_T_LIB " %{m64:-lheapt_w}"
-
 #undef SUBTARGET_EXTRA_SPECS
 #define SUBTARGET_EXTRA_SPECS   \
   DARWIN_EXTRA_SPECS\
diff --git a/gcc/config/i386/darwin64-biarch.h 
b/gcc/config/i386/darwin64-biarch.h
index 620800749a8..85436791a6c 100644
--- a/gcc/config/i386/darwin64-biarch.h
+++ b/gcc/config/i386/darwin64-biarch.h
@@ -28,9 +28,6 @@ along with GCC; see the file COPYING3.  If not see
 #undef  DARWIN_SUBARCH_SPEC
 #define DARWIN_SUBARCH_SPEC DARWIN_ARCH_SPEC
 
-#undef DARWIN_HEAP_T_LIB
-#define DARWIN_HEAP_T_LIB "%{!m32:-lheapt_w}"
-
 #undef SUBTARGET_EXTRA_SPECS
 #define SUBTARGET_EXTRA_SPECS   \
   DARWIN_EXTRA_SPECS\
diff --git a/libgcc/config.host b/libgcc/config.host
index 3e7d00f67aa..59a42d3a01f 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -774,6 +774,7 @@ i[34567]86-*-linux*)
tmake_file="${tmake_file} i386/t-crtpc t-crtfm i386/t-crtstuff 
t-dfprules"
tm_file="${tm_file} i386/elf-lib.h"
md_unwind_header=i386/linux-unwind.h
+   tmake_file="${tmake_file} i386/t-heap-trampoline"
;;
 i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-kopensolaris*-gnu)
extra_parts="$extra_parts crtprec32.o crtprec64.o crtprec80.o 
crtfastmath.o"
diff --git a/libgcc/config/i386/heap-trampoline.c 
b/libgcc/config/i386/heap-trampoline.c
index 657b344c10c..1df0aa06108 100644
--- a/libgcc/config/i386/heap-trampoline.c
+++ b/libgcc/config/i386/heap-trampoline.c
@@ -29,12 +29,13 @@ void *allocate_trampoline_page (void);
 void __gcc_nested_func_ptr_created (void *chain, void *func, void *dst);
 void __gcc_nested_func_ptr_deleted (void);
 
+#if __x86_64__
 static const uint8_t trampoline_insns[] = {
-  /* movabs $,%r11  */
+  /* movabs $,%r11  */
   0x49, 0xbb,
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 
-  /* movabs $,%r10  */
+  /* movabs $,%r10  */
   0x49, 0xba,
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 
@@ -54,6 +55,33 @@ union ix86_trampoline {
   } fields;
 };
 
+#elif __i386__
+
+static 

Re: [PATCH] Darwin, testsuite: -multiply_defined is obsolete

2024-02-10 Thread Iain Sandoe



> On 10 Feb 2024, at 14:54, FX Coudert  wrote:
> 
> With Xcode 15, gcc.dg/ssp-2.c fails due to a warning: -multiply_defined is 
> obsolete
> 
> The patches ignores the warning when present.
> OK to push?

yes, thanks.
Iain

… although part of me is curious about whether we still have any supported
OS version for which the  “-multiply_defined,suppress” is needed, we should try
to put that evaluation on the GCC-15  TODO.

> 
> FX
> 
> <0001-Darwin-testsuite-multiply_defined-is-obsolete.patch>



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-10 Thread Iain Sandoe



> On 10 Feb 2024, at 12:07, Jason Merrill  wrote:
> 
> On 2/10/24 05:46, Iain Sandoe wrote:
>>> On 9 Feb 2024, at 23:21, Iain Sandoe  wrote:
>>> 
>>> 
>>> 
>>>> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>>>>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>>>>> 
>>>>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>>>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we want 
>>>>>>>> the just-built gnat1 to find the system libgcc.
>>>>>>>> 
>>>>>>>> In stage 2, when we build with the stage 1 gcc, we want the just-built 
>>>>>>>> gnat1 to find the stage 1 libgcc.
>>>>>>>> 
>>>>>>>> In neither case do we want it to find the libgcc from the current 
>>>>>>>> stage.
>>>>>>>> 
>>>>>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
>>>>>>>> include the TARGET_LIB_PATH from the previous stage.  Something like 
>>>>>>>> the below, on top of the earlier patch.
>>>>>>>> 
>>>>>>>> Does this make sense?  Does it work on Darwin?
>>>>>>> 
>>>>>>> Oops, that was broken, please consider this one instead:
>>>>>> Yes, this one makes sense (and the current code would not work since it 
>>>>>> lacks the prev- prefix on TARGET_LIB_PATH).
>>>>> 
>>>>> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
>>>>> that has been useful in HOST_EXPORTS is the prev- part of 
>>>>> HOST_LIB_PATH_gcc.
>>>>> 
>>>>> So, here's another patch that just includes that for post-stage1:
>>>>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
>>>> 
>>>> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
>>>> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it 
>>>> to succeed so,
>>>> presumably, the post stage1 exports are not being forwarded to that build. 
>>>>  I’ll try to analyze what
>>>> exactly is failing.
>>> 
>>> The fail is occurring in the target libada build; so, I suppose, one might 
>>> say it’s reasonable that it
>>> requires this host path to be added to the target exports since it’s a host 
>>> library used during target
>>> builds (or do folks expect the host exports to be made for target lib 
>>> builds as well?)
>>> 
>>> Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this
>> Hmm this is still not right, in this case, I think it should actually be the 
>> “just built” directory;
>>  - if we have a tool that depends on host libraries (that happen to be also 
>> target ones),
>>   then those libraries have to be built before the tool so that they can be 
>> linked to it.
>>   (we specially copy libgcc* and the CRTs to gcc/ to allow for this case)
>>  - there is no prev-gcc in cross and —disable-bootstrap builds, but the tool 
>> will still be
>>linked to the just-built host libraries (which will also be installed).
>> So, I think we have to add HOST_LIB_PATH_gcc to HOST_LIB_PATH
>> and HOST_PREV_LIB_PATH_gcc to POSTSTAGE1_HOST_EXPORTS (as per this patch).
> 
> I don't follow.  In a cross build, host libraries are a different 
> architecture from target libraries, and certainly can't be linked into host 
> binaries.
> 
> In a disable-bootstrap build, even before my change TARGET_LIB_PATH isn't 
> added to RPATH_ENVVAR, since that has been guarded with @if gcc-bootstrap.
> 
> So in a bootstrap build, it shouldn't be needed for stage1 either.  And for 
> stage2, the one we need is from stage1, that matches the compiler we're 
> building host tools with.
> 
> What am I missing?

nothing, I was off on a tangent about the cross/non-bootstrap, sorry about that.

However, when doing target builds (the previous point) it seems we do have to 
make provision for gnat1 to find libgcc_s, and, at present, it seems that only 
the target exports are active.

Iain


> Jason



Re: [PATCH] Darwin, testsuite: -bind_at_load is deprecated

2024-02-10 Thread Iain Sandoe
Hi FX,

> On 10 Feb 2024, at 11:58, FX Coudert  wrote:
> 
> With Xcode 15, gcc.dg/darwin-ld-2.c fails due to a warning:
> ld: warning: -bind_at_load is deprecated on macOS
> 
> The patches ignores the warning when present.
> OK to push?

Yes, thanks.

Iain

I guess for GCC-15 we might need to see which options should now be
stripped for new(er) darwin.

> 
> FX
> 
> 
> <0001-Darwin-testsuite-bind_at_load-is-deprecated.patch>



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-10 Thread Iain Sandoe



> On 9 Feb 2024, at 23:21, Iain Sandoe  wrote:
> 
> 
> 
>> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>>> 
>>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>>> 
>>>>>> 
>>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we want 
>>>>>> the just-built gnat1 to find the system libgcc.
>>>>>> 
>>>>>> In stage 2, when we build with the stage 1 gcc, we want the just-built 
>>>>>> gnat1 to find the stage 1 libgcc.
>>>>>> 
>>>>>> In neither case do we want it to find the libgcc from the current stage.
>>>>>> 
>>>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
>>>>>> include the TARGET_LIB_PATH from the previous stage.  Something like the 
>>>>>> below, on top of the earlier patch.
>>>>>> 
>>>>>> Does this make sense?  Does it work on Darwin?
>>>>> 
>>>>> Oops, that was broken, please consider this one instead:
>>>> Yes, this one makes sense (and the current code would not work since it 
>>>> lacks the prev- prefix on TARGET_LIB_PATH).
>>> 
>>> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
>>> that has been useful in HOST_EXPORTS is the prev- part of HOST_LIB_PATH_gcc.
>>> 
>>> So, here's another patch that just includes that for post-stage1:
>>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
>> 
>> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
>> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it to 
>> succeed so,
>> presumably, the post stage1 exports are not being forwarded to that build.  
>> I’ll try to analyze what
>> exactly is failing.
> 
> The fail is occurring in the target libada build; so, I suppose, one might 
> say it’s reasonable that it
> requires this host path to be added to the target exports since it’s a host 
> library used during target
> builds (or do folks expect the host exports to be made for target lib builds 
> as well?)
> 
> Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this

Hmm this is still not right, in this case, I think it should actually be the 
“just built” directory;

 - if we have a tool that depends on host libraries (that happen to be also 
target ones),
  then those libraries have to be built before the tool so that they can be 
linked to it.
  (we specially copy libgcc* and the CRTs to gcc/ to allow for this case)

 - there is no prev-gcc in cross and —disable-bootstrap builds, but the tool 
will still be
   linked to the just-built host libraries (which will also be installed).

So, I think we have to add HOST_LIB_PATH_gcc to HOST_LIB_PATH
and HOST_PREV_LIB_PATH_gcc to POSTSTAGE1_HOST_EXPORTS (as per this patch).

It would be nice to declare the libraries that are used for both host and 
target as
separate modules in the top level template (since we might want to build them 
with different
configure options; e.g. there’s no real point in bootstrapping the host 
libstdc++ as
a shared lib - pulling in its dependent libs -  if we are only going to link 
the .a).

Iain
(will check a cross-compiler build with the changes above).

> because HOST_LIB_PATH is
> appended to the TARGET_EXPORTS (we do not seem to make those depend on the 
> stage).
> 
> Iain



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-09 Thread Iain Sandoe



> On 9 Feb 2024, at 10:56, Iain Sandoe  wrote:
>> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
>> 
>> On 2/8/24 12:55, Paolo Bonzini wrote:
>>> On 2/8/24 18:16, Jason Merrill wrote:
>>>>>> 
>>>>> 
>>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we want 
>>>>> the just-built gnat1 to find the system libgcc.
>>>>> 
>>>>> In stage 2, when we build with the stage 1 gcc, we want the just-built 
>>>>> gnat1 to find the stage 1 libgcc.
>>>>> 
>>>>> In neither case do we want it to find the libgcc from the current stage.
>>>>> 
>>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
>>>>> include the TARGET_LIB_PATH from the previous stage.  Something like the 
>>>>> below, on top of the earlier patch.
>>>>> 
>>>>> Does this make sense?  Does it work on Darwin?
>>>> 
>>>> Oops, that was broken, please consider this one instead:
>>> Yes, this one makes sense (and the current code would not work since it 
>>> lacks the prev- prefix on TARGET_LIB_PATH).
>> 
>> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
>> that has been useful in HOST_EXPORTS is the prev- part of HOST_LIB_PATH_gcc.
>> 
>> So, here's another patch that just includes that for post-stage1:
>> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>
> 
> Hmm this still fails for me with gnat1 being unable to find libgcc_s.
> It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it to 
> succeed so,
> presumably, the post stage1 exports are not being forwarded to that build.  
> I’ll try to analyze what
> exactly is failing.

The fail is occurring in the target libada build; so, I suppose, one might say 
it’s reasonable that it
requires this host path to be added to the target exports since it’s a host 
library used during target
builds (or do folks expect the host exports to be made for target lib builds as 
well?)

Appending the prev-gcc dirctory to the HOST_LIB_PATH fixes this because 
HOST_LIB_PATH is
appended to the TARGET_EXPORTS (we do not seem to make those depend on the 
stage).

Iain




Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-09 Thread Iain Sandoe



> On 8 Feb 2024, at 21:44, Jason Merrill  wrote:
> 
> On 2/8/24 12:55, Paolo Bonzini wrote:
>> On 2/8/24 18:16, Jason Merrill wrote:
> 
 
 Hmm.  In stage 1, when we build with the system gcc, I'd think we want the 
 just-built gnat1 to find the system libgcc.
 
 In stage 2, when we build with the stage 1 gcc, we want the just-built 
 gnat1 to find the stage 1 libgcc.
 
 In neither case do we want it to find the libgcc from the current stage.
 
 So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
 include the TARGET_LIB_PATH from the previous stage.  Something like the 
 below, on top of the earlier patch.
 
 Does this make sense?  Does it work on Darwin?
>>> 
>>> Oops, that was broken, please consider this one instead:
>> Yes, this one makes sense (and the current code would not work since it 
>> lacks the prev- prefix on TARGET_LIB_PATH).
> 
> Indeed, that seems like evidence that the only element of TARGET_LIB_PATH 
> that has been useful in HOST_EXPORTS is the prev- part of HOST_LIB_PATH_gcc.
> 
> So, here's another patch that just includes that for post-stage1:
> <0001-build-drop-target-libs-from-LD_LIBRARY_PATH-PR105688.patch>

Hmm this still fails for me with gnat1 being unable to find libgcc_s.
It seems I have to add the PREV_HOST_LIB_PATH_gcc to HOST_LIB_PATH for it to 
succeed so,
presumably, the post stage1 exports are not being forwarded to that build.  
I’ll try to analyze what
exactly is failing.
Iain



[pushed] libgcc, Darwin: Update symbol exports to include bitint and bf.

2024-02-09 Thread Iain Sandoe
Tested on i686-darwin8, x86_64-darwin{14,17,19,21,23} pushed to trunk.
thanks,
Iain

--- 8< ---

Some exports were missed from the GCC-13 cycle, these are added here
along with the bitint-related ones added in GCC-14.

libgcc/ChangeLog:

* config/i386/libgcc-darwin.ver: Export bf and bitint-related
synbols.
---
 libgcc/config/i386/libgcc-darwin.ver | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/libgcc/config/i386/libgcc-darwin.ver 
b/libgcc/config/i386/libgcc-darwin.ver
index c97dae73855..06560d6b47f 100644
--- a/libgcc/config/i386/libgcc-darwin.ver
+++ b/libgcc/config/i386/libgcc-darwin.ver
@@ -1,9 +1,12 @@
+
+# NOTE: Darwin does not version individual symbols, the grouping is
+# preserved here to document at which GCC revision they were introduced.
+
 GCC_4.8.0 {
   __cpu_model
   __cpu_indicator_init
 }
 
-%inherit GCC_12.0.0 GCC_7.0.0
 GCC_12.0.0 {
   __divhc3
   __mulhc3
@@ -22,3 +25,22 @@ GCC_12.0.0 {
   __trunctfhf2
   __truncxfhf2
 }
+
+GCC_14.0.0 {
+  # Added to GCC_13.0.0 in i386/libgcc-glibc.ver.
+  __extendbfsf2
+  __floattibf
+  __floatuntibf
+  __truncdfbf2
+  __truncsfbf2
+  __trunctfbf2
+  __truncxfbf2
+  __trunchfbf2
+  # Added to GCC_14.0.0 in i386/libgcc-glibc.ver.
+  __fixxfbitint
+  __fixtfbitint
+  __floatbitintbf
+  __floatbitinthf
+  __floatbitintxf
+  __floatbitinttf
+}
-- 
2.39.2 (Apple Git-143)



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-08 Thread Iain Sandoe



> On 8 Feb 2024, at 19:25, Jason Merrill  wrote:
> 
> On 2/8/24 12:51, Iain Sandoe wrote:
>>> On 8 Feb 2024, at 17:16, Jason Merrill  wrote:
>>> 
>>> On 2/8/24 12:12, Jason Merrill wrote:
>>>> On 2/8/24 10:04, Iain Sandoe wrote:
>>>>> Hi Jason,
>>>>> 
>>>>> I have tested this on modern Darwin (with libc++ as the system library) 
>>>>> and on
>>>>> older Darwin, where we do see the issue - because the system linker is 
>>>>> written
>>>>> in C++ and links with libstdc++ (so sometimes we get a crash, or worse 
>>>>> unpredictable
>>>>> beahviour).
>>>> Thank you!
>>>>> -
>>>>> 
>>>>> For modern Darwin [ >  macOS 10.11] , there’s no issue seen so far, but 
>>>>> for older Darwin….
>>>>> 
>>>>>> On 8 Feb 2024, at 05:36, Alexandre Oliva  wrote:
>>>>>> 
>>>>>> On Feb  6, 2024, Jason Merrill  wrote:
>>>>>> 
>>>>>>> Reverting that hunk of the change fixed my problem with bubblestrapping 
>>>>>>> GCC
>>>>>>> 12 with ccache on a host with a newer system libstdc++.
>>>>>> 
>>>>>> Did you have libcc1, gnattools and gotools enabled in your testing?
>>>>> 
>>>>> … I have done all but “go” since that’s not supported on Darwin.
>>>>> 
>>>>> The patch breaks bootstrap on older Darwin because:
>>>>> 
>>>>>Ada uses exceptions.
>>>>>gnat1 pulls in system libraries that link with the system unwinder
>>>>>- so we have to link gnat1 “-shared-libgcc”
>>>>>- which means we need to be able to find the just-built one when 
>>>>> building the target libs.
>>>> Hmm.  In stage 1, when we build with the system gcc, I'd think we want the 
>>>> just-built gnat1 to find the system libgcc.
>>>> In stage 2, when we build with the stage 1 gcc, we want the just-built 
>>>> gnat1 to find the stage 1 libgcc.
>>>> In neither case do we want it to find the libgcc from the current stage.
>>>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
>>>> include the TARGET_LIB_PATH from the previous stage.  Something like the 
>>>> below, on top of the earlier patch.
>>>> Does this make sense?
>> Yes in terms of me mixing up “just built” and “prev-*“
> 
> I've been unclear about that myself.
> 
>> However, IIUC this still means we’re adding LD_LIBRARY_PATHs for each target 
>> library, including libstdc++ (so I’m not sure if it solves the problem that 
>> I’d seen before).
> 
> Yes, after stage 1.
> 
> You're right that we won't need all of the stage1 target libraries to run the 
> stage2 compiler.  But if your gnat1 is linked with shared libgcc, isn't also 
> linked with shared libstdc++, so it would need to find the stage1 libstdc++?

No, we continue to link with -static-libstdc++ (the only reason we use shared 
libgcc is to avoid two unwinders in the same process, which never ends well - 
usually gnat1 hangs)

> In general, though, it would make sense to decide whether to include both 
> libgcc and libstdc++ based on with_static_shared_libraries.  And likewise the 
> other language runtimes (libada, libgfortran, ...)?

Right now, AFAIK the only target runtimes used by host tools are libstdc++, 
libgcc and libgnat.  I agree that might change with rust - since the rust folks 
are talking about using one of the runtimes in the FE,  I am not aware of other 
language FEs requiring their targte runtimes to be available to the host tools 
(adding Gaius in case I missed something with m2 - which is quite complex inthe 
bootstrapping).

linking these shared would seem to me to be placing additional constraints on 
the build that are likely to take some shaking down - personally, I much prefer 
a toolchain with no shared deps - since then one knows what was tested is not 
going to change in reponse to some DSO update.

> We probably don't need libgomp/libatomic/libitm?

unless/until we have multiple threads in the compiler(s), it seems unlikely, 
indeed

> libsanitizer/vtv/ssp should only be needed for a bootstrap using those 
> hardening options.

Ah, now in that case we probably _do_ need to force shared libstdc++ (since the 
sanitizers link against c++ lib)
- an alternative is to force all of libstdc++.a to be included in the host 
tools, and use that to provide the symbols,

> If your stage2 gnat1 does need the stage1 libstdc++ and you're also using the 
> system ld that needs the system libstdc++, that seems to call for another 
> approach discussed on the PR: have exec-tool.in splice out the build 
> directory from LD_LIBRARY_PATH when calling a tool from outside the build 
> directory.

I’ve done all sorts of dances with this - e.g. linking the ld with a 
specially-named libstdc++ (or forcing the linker to be statically linked - 
although that creates a secondary problem since the Darwin libLTO (for clang) 
is also linked against c++)

Iain

> 
> Jason



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-08 Thread Iain Sandoe



> On 8 Feb 2024, at 17:16, Jason Merrill  wrote:
> 
> On 2/8/24 12:12, Jason Merrill wrote:
>> On 2/8/24 10:04, Iain Sandoe wrote:
>>> Hi Jason,
>>> 
>>> I have tested this on modern Darwin (with libc++ as the system library) and 
>>> on
>>> older Darwin, where we do see the issue - because the system linker is 
>>> written
>>> in C++ and links with libstdc++ (so sometimes we get a crash, or worse 
>>> unpredictable
>>> beahviour).
>> Thank you!
>>> -
>>> 
>>> For modern Darwin [ >  macOS 10.11] , there’s no issue seen so far, but for 
>>> older Darwin….
>>> 
>>>> On 8 Feb 2024, at 05:36, Alexandre Oliva  wrote:
>>>> 
>>>> On Feb  6, 2024, Jason Merrill  wrote:
>>>> 
>>>>> Reverting that hunk of the change fixed my problem with bubblestrapping 
>>>>> GCC
>>>>> 12 with ccache on a host with a newer system libstdc++.
>>>> 
>>>> Did you have libcc1, gnattools and gotools enabled in your testing?
>>> 
>>> … I have done all but “go” since that’s not supported on Darwin.
>>> 
>>> The patch breaks bootstrap on older Darwin because:
>>> 
>>>Ada uses exceptions.
>>>gnat1 pulls in system libraries that link with the system unwinder
>>>- so we have to link gnat1 “-shared-libgcc”
>>>- which means we need to be able to find the just-built one when 
>>> building the target libs.
>> Hmm.  In stage 1, when we build with the system gcc, I'd think we want the 
>> just-built gnat1 to find the system libgcc.
>> In stage 2, when we build with the stage 1 gcc, we want the just-built gnat1 
>> to find the stage 1 libgcc.
>> In neither case do we want it to find the libgcc from the current stage.
>> So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
>> include the TARGET_LIB_PATH from the previous stage.  Something like the 
>> below, on top of the earlier patch.
>> Does this make sense?  

Yes in terms of me mixing up “just built” and “prev-*“

However, IIUC this still means we’re adding LD_LIBRARY_PATHs for each target 
library, including libstdc++ (so I’m not sure if it solves the problem that I’d 
seen before).

I can see we need to add any paths to shared libraries used by the host tools 
(so that, perhaps, we’ll need to add libgrust if the rust FE uses that) but, 
otherwise AFAIK, the only [shared] target library used by the host tools is 
libgcc_s (when gnat1 needs it) - otherwise we default to static linking of 
libstdc++ and libgcc (and the gnat* tools link libgnat statically too) ? 

>> Does it work on Darwin?

I’ll kick off a run shortly - know later this evening.

Iain

> 
> Oops, that was broken, please consider this one instead:
> 
> <0001-build-add-prev-target-libs-to-rpath.patch>



Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

2024-02-08 Thread Iain Sandoe
Hi Jason,

I have tested this on modern Darwin (with libc++ as the system library) and on
older Darwin, where we do see the issue - because the system linker is written
in C++ and links with libstdc++ (so sometimes we get a crash, or worse 
unpredictable
beahviour).

-

For modern Darwin [ >  macOS 10.11] , there’s no issue seen so far, but for 
older Darwin….

> On 8 Feb 2024, at 05:36, Alexandre Oliva  wrote:
> 
> On Feb  6, 2024, Jason Merrill  wrote:
> 
>> Reverting that hunk of the change fixed my problem with bubblestrapping GCC
>> 12 with ccache on a host with a newer system libstdc++.
> 
> Did you have libcc1, gnattools and gotools enabled in your testing?

… I have done all but “go” since that’s not supported on Darwin.

The patch breaks bootstrap on older Darwin because:

  Ada uses exceptions.
  gnat1 pulls in system libraries that link with the system unwinder
  - so we have to link gnat1 “-shared-libgcc”
  - which means we need to be able to find the just-built one when building the 
target libs.

- that means we need to find it as a host library, which we were getting away 
with
because the host list was appended to the target list.

To recover bootstrap I think the  following addition is needed:

diff --git a/Makefile.tpl b/Makefile.tpl
index cb39fbd0434..65621bf6882 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -663,7 +663,7 @@ TARGET_LIB_PATH_[+module+] = 
$$r/$(TARGET_SUBDIR)/[+module+]/[+lib_path+]:
 # so that programs built for the host machine work.
 HOST_LIB_PATH = [+ FOR host_modules +][+
   IF lib_path +]$(HOST_LIB_PATH_[+module+])[+ ENDIF lib_path +][+
-  ENDFOR host_modules +]
+  ENDFOR host_modules +]$(HOST_LIB_PATH_gcc)
 
 # Define HOST_LIB_PATH_gcc here, for the sake of TARGET_LIB_PATH, ouch
 @if gcc

It’s not clear to me why we consider prev-gcc as relevant to host libgcc (we 
must have built libgcc
in order to have linked it to any host deps .. but perhaps I’m missing a case).

I agree with the sentiment below that this is a delicate area….

> Those non-bootstrapped tools are most likely to be problem spots.
> 
> I have a vague recollection that, in native scenarios, when
> bootstrapping but not when not boostrapping, they are (or used to be?)
> built using the just-built (last-stage) compiler and target libraries,
> rather than system tools or previous stage (which I believe would fail
> bootstrap-lean).
> 
> Just by looking at the current code I can't bring back to mind all the
> involved moving parts :-/ but my guess is that the need for
> TARGET_LIB_PATH comes about for this combination of circumstances, in
> which we would need to use the just-built native-target libstdc++ to
> link or to run these host tools using the just-built tools.
> 
> Having a newer system libstdc++ would, I suspect, unfortunately mask the
> problem that those who had an older or no system libstdc++ would likely
> run into.
> 
> 
> OTOH, if the system linker requires the newer system library, and ld.so
> and ld overload LD_LIBRARY_PATH for both the libraries loaded for ld to
> run, the libraries ld searches to link executables, and the libraries
> loaded for so-linked programs to run, we seem to need some means to
> distinguish between these three circumstances so as to be able to set
> LD_LIBRARY_PATH correctly for each one, whether the libstdc++ we're
> building is newer or older than the system one.  And there's an added
> complication that any single ld invocation involves two different and
> potentially conflicting uses of LD_LIBRARY_PATH.
> 
> /me runs screaming :-)
> 
> 
> Perhaps the best we can do out of these conflicting requirements is to
> somehow detect a system libstdc++ newer than the one we're about to
> build, and fail early if we find that some of the tools depend on the
> newer libstdc++, or disable this env var setting for this case only and
> hope the newer system library is compatible enough.
> 
> Holy nightmare, Batman! :-)  :-(
> 
> 
> Perhaps the way to go is an explicit configure setting, recommended by
> the fail early, to disable the env var setting?
> 
> -- 
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>   Free Software Activist   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive



Re: [PATCH] aarch64, acle header: Cast uint64_t pointers to DIMode.

2024-02-05 Thread Iain Sandoe



> On 5 Feb 2024, at 14:56, Iain Sandoe  wrote:
> 
> Tested on aarch64-linux,darwin and a cross from aarch64-darwin to linux,
> OK for trunk, or some alternative is needed?

Hmm.. apparently, this fails the linaro pre-commit CI for g++ with:
error: invalid conversion from 'long int*' to 'long unsigned int*' 
[-fpermissive]

So, I guess some alternative is needed, advice welcome,
Iain

> thanks
> Iain
> 
> --- 8< ---
> 
> Currently, most of the acle tests fail on the Darwin port because
> DI mode is "long" and uint64 is "long long".  The fix for this used
> in other headers is to cast the pointers using __builtin_aarch64_simd_di
> and that is what this patch does.
> 
> gcc/ChangeLog:
> 
>   * config/aarch64/arm_acle.h (__rndr): Cast uint64 pointer to DI
>   mode to avoid typedef mismatches.
>   (__rndrrs): Likewise.
> ---
> gcc/config/aarch64/arm_acle.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
> index 2aa681090fa..823f87187b1 100644
> --- a/gcc/config/aarch64/arm_acle.h
> +++ b/gcc/config/aarch64/arm_acle.h
> @@ -309,14 +309,14 @@ __extension__ extern __inline int
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> __rndr (uint64_t *__res)
> {
> -  return __builtin_aarch64_rndr (__res);
> +  return __builtin_aarch64_rndr ((__builtin_aarch64_simd_di *) __res);
> }
> 
> __extension__ extern __inline int
> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> __rndrrs (uint64_t *__res)
> {
> -  return __builtin_aarch64_rndrrs (__res);
> +  return __builtin_aarch64_rndrrs ((__builtin_aarch64_simd_di *) __res);
> }
> 
> #pragma GCC pop_options
> -- 
> 2.39.2 (Apple Git-143)
> 



[PATCH] aarch64, acle header: Cast uint64_t pointers to DIMode.

2024-02-05 Thread Iain Sandoe
Tested on aarch64-linux,darwin and a cross from aarch64-darwin to linux,
OK for trunk, or some alternative is needed?
thanks
Iain

--- 8< ---

Currently, most of the acle tests fail on the Darwin port because
DI mode is "long" and uint64 is "long long".  The fix for this used
in other headers is to cast the pointers using __builtin_aarch64_simd_di
and that is what this patch does.

gcc/ChangeLog:

* config/aarch64/arm_acle.h (__rndr): Cast uint64 pointer to DI
mode to avoid typedef mismatches.
(__rndrrs): Likewise.
---
 gcc/config/aarch64/arm_acle.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
index 2aa681090fa..823f87187b1 100644
--- a/gcc/config/aarch64/arm_acle.h
+++ b/gcc/config/aarch64/arm_acle.h
@@ -309,14 +309,14 @@ __extension__ extern __inline int
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __rndr (uint64_t *__res)
 {
-  return __builtin_aarch64_rndr (__res);
+  return __builtin_aarch64_rndr ((__builtin_aarch64_simd_di *) __res);
 }
 
 __extension__ extern __inline int
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __rndrrs (uint64_t *__res)
 {
-  return __builtin_aarch64_rndrrs (__res);
+  return __builtin_aarch64_rndrrs ((__builtin_aarch64_simd_di *) __res);
 }
 
 #pragma GCC pop_options
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Darwin: Allow for undefined symbols in shared test.

2024-02-02 Thread Iain Sandoe
tested on i686, x86_64 Darwin, x86_64, aarch64 linux, pushed to trunk,
thanks,
Iain

--- 8< ---

Darwin's linker defaults to error on undefined (which makes it look as
if we do not support shared, leading to tests being marked incorrectly
as unsupported).

This fixes the issue by allowing the symbols used in the target
supports test to be undefined.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp (check_effective_target_shared):
Allow the external symbols referenced in the test to be undefined.
---
 gcc/testsuite/lib/target-supports.exp | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index 8aefb320d8c..f66dcaac7b8 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -1385,6 +1385,13 @@ proc check_effective_target_aarch64_tlsle32 { } {
 # emitted, 0 otherwise.
 
 proc check_effective_target_shared { } {
+# Darwin's linker defaults to error on undefined (which makes it look as
+# if we do not support shared) but we can tell it to allow the symbols used
+# here to be undefined.
+set extra_flags ""
+if { [istarget *-*-darwin\[912\]*] } {
+  set extra_flags "-Wl,-U,_foo,-U,_bar"
+}
 # Note that M68K has a multilib that supports -fpic but not
 # -fPIC, so we need to check both.  We test with a program that
 # requires GOT references, and with a libc symbol that would
@@ -1397,7 +1404,7 @@ proc check_effective_target_shared { } {
char *baz (void) {
return foo () + (char*) malloc (bar);
}
-} "-shared -fpic"]
+} "-shared -fpic $extra_flags"]
 }
 
 # Return 1 if -pie, -fpie and -fPIE are supported, 0 otherwise.
-- 
2.39.2 (Apple Git-143)



Re: [PATCH] libgcc: Avoid warnings on __gcc_nested_func_ptr_created [PR113402]

2024-02-01 Thread Iain Sandoe



> On 1 Feb 2024, at 08:51, Richard Biener  wrote:
> 
> On Thu, Feb 1, 2024 at 9:23 AM Jakub Jelinek  wrote:
>> 
>> On Wed, Jan 31, 2024 at 01:04:20PM +0100, Jakub Jelinek wrote:
>>> On Sun, Jan 28, 2024 at 11:02:33AM +, Iain Sandoe wrote:
>>>>* config/aarch64/heap-trampoline.c: Rename
>>>>__builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
>>>>__builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
>>>>* config/i386/heap-trampoline.c: Likewise.
>>>>* libgcc2.h: Likewise.
>>> 
>>> I'm seeing hundreds of
>>> In file included from ../../../libgcc/libgcc2.c:56:
>>> ../../../libgcc/libgcc2.h:32:13: warning: conflicting types for built-in 
>>> function ‘__gcc_nested_func_ptr_created’; expected ‘void(void *, void *, 
>>> void *)’ [-Wbuiltin-declaration-mismatch]
>>>   32 | extern void __gcc_nested_func_ptr_created (void *, void *, void **);
>>>  | ^
>>> warnings.
>>> 
>>> Either we need to add like in r14-6218
>>> #pragma GCC diagnostic ignored "-Wbuiltin-declaration-mismatch"
>>> (but in that case because of the libgcc2.h prototype (why is it there?)
>>> it would need to be also with #pragma GCC diagnostic push/pop around),
>>> or we could go with just following how the builtins are prototyped on the
>>> compiler side and only cast to void ** when dereferencing (which is in
>>> a single spot in each TU).
>> 
>> Bootstrapped/regtested on x86_64-linux and i686-linux successfully.
> 
> Looks obvious to me.

Thanks, also tested on x86_64 darwin; I guess having typed pointers on the 
builtins
would be a pretty tricky change.

Iain

> 
> Richard.
> 
>>> 2024-01-31  Jakub Jelinek  
>>> 
>>>  * libgcc2.h (__gcc_nested_func_ptr_created): Change type of last
>>>  argument from void ** to void *.
>>>  * config/i386/heap-trampoline.c (__gcc_nested_func_ptr_created):
>>>  Change type of dst from void ** to void * and cast dst to void **
>>>  before dereferencing it.
>>>  * config/aarch64/heap-trampoline.c (__gcc_nested_func_ptr_created):
>>>  Likewise.
>> 
>>Jakub



Re: [PATCH] libgcc: Fix up i386/t-heap-trampoline [PR113403]

2024-02-01 Thread Iain Sandoe



> On 1 Feb 2024, at 08:22, Jakub Jelinek  wrote:
> 
> On Wed, Jan 31, 2024 at 12:59:27PM +0100, Jakub Jelinek wrote:
>> On Sun, Jan 28, 2024 at 02:07:32PM +0000, Iain Sandoe wrote:
>>> --- a/libgcc/config/aarch64/t-heap-trampoline
>>> +++ b/libgcc/config/aarch64/t-heap-trampoline
>>> @@ -16,4 +16,5 @@
>>> # along with GCC; see the file COPYING3.  If not see
>>> # <http://www.gnu.org/licenses/>.
>>> 
>>> -LIB2ADD += $(srcdir)/config/aarch64/heap-trampoline.c
>>> +LIB2ADDEH += $(srcdir)/config/aarch64/heap-trampoline.c
>>> +LIB2ADDEHSHARED += $(srcdir)/config/aarch64/heap-trampoline.c
>>> --- a/libgcc/config/i386/t-heap-trampoline
>>> +++ b/libgcc/config/i386/t-heap-trampoline
>>> @@ -16,4 +16,5 @@
>>> # along with GCC; see the file COPYING3.  If not see
>>> # <http://www.gnu.org/licenses/>.
>>> 
>>> -LIB2ADD += $(srcdir)/config/i386/heap-trampoline.c
>>> +LIB2ADDEH += $(srcdir)/config/i386/heap-trampoline.c
>>> +LIB2ADDEHSHARED += $(srcdir)/config/aarch64/heap-trampoline.c
>> 
>> I'm seeing
>> ../../../libgcc/shared-object.mk:14: warning: overriding recipe for target 
>> 'heap-trampoline.o'
>> ../../../libgcc/shared-object.mk:14: warning: ignoring old recipe for target 
>> 'heap-trampoline.o'
>> ../../../libgcc/shared-object.mk:17: warning: overriding recipe for target 
>> 'heap-trampoline_s.o'
>> ../../../libgcc/shared-object.mk:17: warning: ignoring old recipe for target 
>> 'heap-trampoline_s.o'
>> 
>> Shouldn't we go with following patch?
>> I can test it on x86_64-linux and i686-linux, but can't test it e.g. on
>> Darwin easily.
>> 
>> 2024-01-31  Jakub Jelinek  
>> 
>>  * config/i386/t-heap-trampoline: Add to LIB2ADDEHSHARED
>>  i386/heap-trampoline.c rather than aarch64/heap-trampoline.c.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux successfully.

Thanks, looks silly pasto that somehow managed to survive default testing
options.

Tested x86_64-darwin (along with the warning suppression patch) and
tested for ftrampoline-impl={heap,stack} and {static,shared}-libgcc.

thanks
Iain



Re: [PATCH] libstdc++: Enable std::text_encoding for darwin and FreeBSD

2024-01-31 Thread Iain Sandoe
Hi Jonathan,

> On 30 Jan 2024, at 15:02, Jonathan Wakely  wrote:
> 
> This should fix the std/text_encoding/* FAILs that Iain sees on darwin.
> I assume it will make it work for FreeBSD too.
> 
> I won't push this until I hear it works for at least one of those.

It works on x86_64-darwin{19,21,23} and on a cross to powerpc-darwin9.
The header is present on all versions we currently support (although I did not 
yet
test more widely than noted above).

As discussed on IRC, Darwin is qualified to Posix 2003 (and does not, generally,
have all the optional parts), so we do not expect the symbols to be present in
locale.h (even with some _XOPEN_SOURCE= value).

thanks for taking care of this,
Iain
> 
> Tested x86_64-linux.
> 
> -- >8 --
> 
> The  header is needed for newlocale and locale_t on these
> targets.
> 
> libstdc++-v3/ChangeLog:
> 
>   * acinclude.m4 (GLIBCXX_CHECK_TEXT_ENCODING): Use  if
>   needed for newlocale.
>   * configure: Regenerate.
>   * src/c++26/text_encoding.cc: Use .
> ---
> libstdc++-v3/acinclude.m4   | 3 +++
> libstdc++-v3/configure  | 3 +++
> libstdc++-v3/src/c++26/text_encoding.cc | 3 +++
> 3 files changed, 9 insertions(+)
> 
> diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> index f9ba7ef744b..f72bd0f45b8 100644
> --- a/libstdc++-v3/acinclude.m4
> +++ b/libstdc++-v3/acinclude.m4
> @@ -5834,6 +5834,9 @@ AC_LANG_SAVE
>   AC_MSG_CHECKING([whether nl_langinfo_l is defined in ])
>   AC_TRY_COMPILE([
>   #include 
> +  #if __has_include()
> +  # include 
> +  #endif
>   #include 
>   ],[
> locale_t loc = newlocale(LC_ALL_MASK, "", (locale_t)0);
> diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
> index 65ce679f1bd..f4bc0486768 100755
> --- a/libstdc++-v3/configure
> +++ b/libstdc++-v3/configure
> @@ -54533,6 +54533,9 @@ $as_echo_n "checking whether nl_langinfo_l is defined 
> in ... " >&6;
> /* end confdefs.h.  */
> 
>   #include 
> +  #if __has_include()
> +  # include 
> +  #endif
>   #include 
> 
> int
> diff --git a/libstdc++-v3/src/c++26/text_encoding.cc 
> b/libstdc++-v3/src/c++26/text_encoding.cc
> index 33c6c07820c..b9a50ef1a00 100644
> --- a/libstdc++-v3/src/c++26/text_encoding.cc
> +++ b/libstdc++-v3/src/c++26/text_encoding.cc
> @@ -27,6 +27,9 @@
> 
> #ifdef _GLIBCXX_USE_NL_LANGINFO_L
> #include 
> +#if __has_include()
> +# include 
> +#endif
> #include 
> 
> #if __CHAR_BIT__ == 8
> -- 
> 2.43.0
> 



[PATCH] testsuite, ubsan: Add libstdc++ deps where required.

2024-01-30 Thread Iain Sandoe
tested on i686, x86_64 (and aarch64) Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks
Iain

--- 8< ---

We use the ubsan tests from both C, C++, D and Fortran.
the sanitizer libraries link to libstdc++.

When we are using the C/gdc/gfortran driver, and the target might
require a path to the libstdc++ (e.g. for handing -static- or
for embedded runpaths), we need to add a suitable option (or we get
fails at execution time because of the missing paths).

Conversely, we do not want to add multiple instances of these
paths (since that leads to failures on tools that report warnings
for duplicate runpaths).

This patch modifies the _init function to allow a sigle parameter
that determines whether the *asan_init should add a path for
libstdc++ (yes for C driver, no for C++ driver).
gcc/testsuite/ChangeLog:

* g++.dg/ubsan/ubsan.exp:Add a parameter to init to say that
we expect the C++ driver to provide paths for libstdc++.
* gcc.dg/ubsan/ubsan.exp: Add a parameter to init to say that
we need a path added for libstdc++.
* gdc.dg/ubsan/ubsan.exp: Likewise.
* gfortran.dg/ubsan/ubsan.exp: Likewise.
* lib/ubsan-dg.exp: Handle a single parameter to init that
requests addition of a path to libstdc++ to link flags.
---
 gcc/testsuite/g++.dg/ubsan/ubsan.exp  |  3 ++-
 gcc/testsuite/gcc.dg/ubsan/ubsan.exp  |  3 ++-
 gcc/testsuite/gdc.dg/ubsan/ubsan.exp  |  3 ++-
 gcc/testsuite/gfortran.dg/ubsan/ubsan.exp |  4 ++--
 gcc/testsuite/lib/ubsan-dg.exp| 20 +++-
 5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/gcc/testsuite/g++.dg/ubsan/ubsan.exp 
b/gcc/testsuite/g++.dg/ubsan/ubsan.exp
index d7197070a92..4bab1b83de9 100644
--- a/gcc/testsuite/g++.dg/ubsan/ubsan.exp
+++ b/gcc/testsuite/g++.dg/ubsan/ubsan.exp
@@ -22,7 +22,8 @@ load_lib ubsan-dg.exp
 
 # Initialize `dg'.
 dg-init
-ubsan_init
+# libubsan uses libstdc++ but we assume that's added by the g++ impl.
+ubsan_init 0
 
 # Main loop.
 if [check_effective_target_fsanitize_undefined] {
diff --git a/gcc/testsuite/gcc.dg/ubsan/ubsan.exp 
b/gcc/testsuite/gcc.dg/ubsan/ubsan.exp
index 84170495e28..560e5843be6 100644
--- a/gcc/testsuite/gcc.dg/ubsan/ubsan.exp
+++ b/gcc/testsuite/gcc.dg/ubsan/ubsan.exp
@@ -24,7 +24,8 @@ load_lib ubsan-dg.exp
 
 # Initialize `dg'.
 dg-init
-ubsan_init
+# libubsan uses libstdc++ so make sure we provide paths for it.
+ubsan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_undefined] {
diff --git a/gcc/testsuite/gdc.dg/ubsan/ubsan.exp 
b/gcc/testsuite/gdc.dg/ubsan/ubsan.exp
index 6ad665a1a8d..7613a3b487c 100644
--- a/gcc/testsuite/gdc.dg/ubsan/ubsan.exp
+++ b/gcc/testsuite/gdc.dg/ubsan/ubsan.exp
@@ -20,7 +20,8 @@ load_lib ubsan-dg.exp
 
 # Initialize `dg'.
 dg-init
-ubsan_init
+# libubsan uses libstdc++ so make sure we provide paths for it.
+ubsan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_undefined] {
diff --git a/gcc/testsuite/gfortran.dg/ubsan/ubsan.exp 
b/gcc/testsuite/gfortran.dg/ubsan/ubsan.exp
index 0c61153e68b..b2360785e6c 100644
--- a/gcc/testsuite/gfortran.dg/ubsan/ubsan.exp
+++ b/gcc/testsuite/gfortran.dg/ubsan/ubsan.exp
@@ -22,10 +22,10 @@
 load_lib gfortran-dg.exp
 load_lib ubsan-dg.exp
 
-
 # Initialize `dg'.
 dg-init
-ubsan_init
+# libubsan uses libstdc++ so make sure we provide paths for it.
+ubsan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_undefined] {
diff --git a/gcc/testsuite/lib/ubsan-dg.exp b/gcc/testsuite/lib/ubsan-dg.exp
index 108b9980cac..860e78f3975 100644
--- a/gcc/testsuite/lib/ubsan-dg.exp
+++ b/gcc/testsuite/lib/ubsan-dg.exp
@@ -31,7 +31,7 @@ proc check_effective_target_fsanitize_undefined {} {
 # (originally from g++.exp)
 #
 
-proc ubsan_link_flags { paths } {
+proc ubsan_link_flags { paths needs_cxx } {
 global srcdir
 global ld_library_path
 global shlib_ext
@@ -43,15 +43,24 @@ proc ubsan_link_flags { paths } {
 set shlib_ext [get_shlib_extension]
 set ubsan_saved_library_path $ld_library_path
 
+# Providing -B instead of -L means that it works for targets that use
+# spec substitution for handling -static-x, it also works for targets
+# the use the startfile paths to provide a runpath for uninstalled test.
+# Each -B option will produce a -L on the link line (for paths that exist).
 if { $gccpath != "" } {
   if { [file exists "${gccpath}/libsanitizer/ubsan/.libs/libubsan.a"]
   || [file exists 
"${gccpath}/libsanitizer/ubsan/.libs/libubsan.${shlib_ext}"] } {
  append flags " -B${gccpath}/libsanitizer/ "
  append flags " -B${gccpath}/libsanitizer/ubsan/ "
- append flags " -L${gccpath}/libsanitizer/ubsan/.libs"
+ append flags " -B${gccpath}/libsanitizer/ubsan/.libs"
  append ld_library_path ":${gccpath}/libsanitizer/ubsan/.libs"
- append ld_library_path ":${gccpath}/libstdc++-v3/src/.libs"
   }
+  # libasan links to libstdc++, so we must 

[PATCH] testsuite, asan, hwsan: Add libstdc++ deps where required.

2024-01-30 Thread Iain Sandoe
tested on i686, x86_64 (and aarch64) Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks
Iain

--- 8< ---

We use the shared asan/hwasan from both C,C++,D and Fortran.
The sanitizer libraries link to libstdc++.

When we are using the C/gdc/gfortran driver, and the target might
require a path to the libstdc++ (e.g. for handing -static- or
for embedded runpaths), we need to add a suitable option (or we get
fails at execution time because of the missing paths).

Conversely, we do not want to add multiple instances of these
paths (since that leads to failures on tools that report warnings
for duplicate runpaths).

This patch modifies the _init function to allow a single parameter
that determines whether the *asan_init should add a path for
libstdc++ (yes for C driver, no for C++ driver).

gcc/testsuite/ChangeLog:

* g++.dg/asan/asan.exp: Add a parameter to init to say that
we expect the C++ driver to provide paths for libstdc++.
* g++.dg/hwasan/hwasan.exp: Likewise
* gcc.dg/asan/asan.exp: Add a parameter to init to say that
we need a path added for libstdc++.
* gcc.dg/hwasan/hwasan.exp: Likewise.
* gdc.dg/asan/asan.exp: Likewise.
* gfortran.dg/asan/asan.exp: Likewise.
* lib/asan-dg.exp: Handle a single parameter to init that
requests addition of a path to libstdc++ to link flags.
* lib/hwasan-dg.exp: Likewise.
---
 gcc/testsuite/g++.dg/asan/asan.exp  |  3 ++-
 gcc/testsuite/g++.dg/hwasan/hwasan.exp  |  3 ++-
 gcc/testsuite/gcc.dg/asan/asan.exp  |  3 ++-
 gcc/testsuite/gcc.dg/hwasan/hwasan.exp  |  3 ++-
 gcc/testsuite/gdc.dg/asan/asan.exp  |  3 ++-
 gcc/testsuite/gfortran.dg/asan/asan.exp |  3 ++-
 gcc/testsuite/lib/asan-dg.exp   | 21 -
 gcc/testsuite/lib/hwasan-dg.exp |  9 +
 8 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/gcc/testsuite/g++.dg/asan/asan.exp 
b/gcc/testsuite/g++.dg/asan/asan.exp
index 9297bb55eb1..f078bc37800 100644
--- a/gcc/testsuite/g++.dg/asan/asan.exp
+++ b/gcc/testsuite/g++.dg/asan/asan.exp
@@ -22,7 +22,8 @@ load_lib asan-dg.exp
 
 # Initialize `dg'.
 dg-init
-asan_init
+# libasan uses libstdc++ but we assume that's added by the g++ impl.
+asan_init 0
 
 # Main loop.
 if [check_effective_target_fsanitize_address] {
diff --git a/gcc/testsuite/g++.dg/hwasan/hwasan.exp 
b/gcc/testsuite/g++.dg/hwasan/hwasan.exp
index 597033e01d5..12a7a3570a0 100644
--- a/gcc/testsuite/g++.dg/hwasan/hwasan.exp
+++ b/gcc/testsuite/g++.dg/hwasan/hwasan.exp
@@ -22,7 +22,8 @@ load_lib hwasan-dg.exp
 
 # Initialize `dg'.
 dg-init
-hwasan_init
+# libhwasan uses libstdc++ but we assume that's added by the g++ impl.
+hwasan_init 0
 
 # Main loop.
 if [check_effective_target_fsanitize_hwaddress] {
diff --git a/gcc/testsuite/gcc.dg/asan/asan.exp 
b/gcc/testsuite/gcc.dg/asan/asan.exp
index 10c69731fbf..6b8ebf07eae 100644
--- a/gcc/testsuite/gcc.dg/asan/asan.exp
+++ b/gcc/testsuite/gcc.dg/asan/asan.exp
@@ -24,7 +24,8 @@ load_lib asan-dg.exp
 
 # Initialize `dg'.
 dg-init
-asan_init
+# libasan uses libstdc++ so make sure we provide paths for it.
+asan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_address] {
diff --git a/gcc/testsuite/gcc.dg/hwasan/hwasan.exp 
b/gcc/testsuite/gcc.dg/hwasan/hwasan.exp
index 802f1712296..88327d3b223 100644
--- a/gcc/testsuite/gcc.dg/hwasan/hwasan.exp
+++ b/gcc/testsuite/gcc.dg/hwasan/hwasan.exp
@@ -24,7 +24,8 @@ load_lib hwasan-dg.exp
 
 # Initialize `dg'.
 dg-init
-hwasan_init
+# libhwasan uses libstdc++ so make sure we provide paths for it.
+hwasan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_hwaddress] {
diff --git a/gcc/testsuite/gdc.dg/asan/asan.exp 
b/gcc/testsuite/gdc.dg/asan/asan.exp
index 72b36696c4d..89c6bf35ae4 100644
--- a/gcc/testsuite/gdc.dg/asan/asan.exp
+++ b/gcc/testsuite/gdc.dg/asan/asan.exp
@@ -20,7 +20,8 @@ load_lib asan-dg.exp
 
 # Initialize `dg'.
 dg-init
-asan_init
+# libasan uses libstdc++ so make sure we provide paths for it.
+asan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_address] {
diff --git a/gcc/testsuite/gfortran.dg/asan/asan.exp 
b/gcc/testsuite/gfortran.dg/asan/asan.exp
index 25cd19f6133..a1576381e61 100644
--- a/gcc/testsuite/gfortran.dg/asan/asan.exp
+++ b/gcc/testsuite/gfortran.dg/asan/asan.exp
@@ -27,7 +27,8 @@ load_lib asan-dg.exp
 
 # Initialize `dg'.
 dg-init
-asan_init
+# libasan uses libstdc++ so make sure we provide paths for it.
+asan_init 1
 
 # Main loop.
 if [check_effective_target_fsanitize_address] {
diff --git a/gcc/testsuite/lib/asan-dg.exp b/gcc/testsuite/lib/asan-dg.exp
index beb49e500eb..6bd3c211611 100644
--- a/gcc/testsuite/lib/asan-dg.exp
+++ b/gcc/testsuite/lib/asan-dg.exp
@@ -61,7 +61,7 @@ proc asan_include_flags {} {
 # (originally from g++.exp)
 #
 
-proc asan_link_flags_1 { paths lib } {
+proc asan_link_flags_1 { paths lib need_stdcxx} {
 global srcdir
 global ld_library_path
 global shlib_ext
@@ -73,6 

Re: [PATCH] jit: Ensure ssize_t is defined.

2024-01-29 Thread Iain Sandoe
Hi David,

I guess the solution here depends on the scope over which we expect
the header to be used.

> On 28 Jan 2024, at 23:13, Iain Sandoe  wrote:
>> On 28 Jan 2024, at 21:25, Eric Gallager  wrote:
>> On Sun, Jan 28, 2024 at 6:45 AM Iain Sandoe  wrote:
>>> 
>>> Tested on i686, x86_64 Darwin, x86_64 Linux,
>>> OK for trunk?
>>> 
>>> --- 8< ---
>>> 
>>> On some targets it seems that ssize_t is not defined by any of the
>>> headers transitively included by .  This leads to a bootstrap
>>> fail when jit is enabled.
>>> 
>>> The fix proposed here is to include sys/types.h when it is available
>>> since that is where Posix specifies that ssize_t is defined.
>>> 
>>> gcc/jit/ChangeLog:
>>> 
>>>   * libgccjit.h: Conditionally include  where it is
>>>   available to ensure declaration of ssize_t.
>>> 
>>> Signed-off-by: Iain Sandoe 
>>> ---
>>> gcc/jit/libgccjit.h | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
>>> index 235cab053e0..db4f27a48bf 100644
>>> --- a/gcc/jit/libgccjit.h
>>> +++ b/gcc/jit/libgccjit.h
>>> @@ -21,6 +21,9 @@ along with GCC; see the file COPYING3.  If not see
>>> #define LIBGCCJIT_H
>>> 
>>> #include 
>>> +#if __has_include()
>> 
>> Is __has_include() something that we can use unconditionally?
> 
> Hmm.. maybe we cannot, it seems it was introduced in gcc-4.9 and we only ask
> for 4.8, IIRC.
> 
> I guess HAVE_SYS_TYPES_H might be an alternative (I’ll have to retest)

Answering my own question; no that is not going to work  either since the 
header is
installed and config.h is not.

I guess the question is “is this header ever [meaningfully] consumed by a 
compiler
other than the current GCC that it supports”?

e.g. if we expected we could build libgccjit with clang in a 
“—disable-bootstrap”
configuration and expect that to work?

The fallback is
#ifdef __APPLE__
# include   /* For ssize_t.  */
#endif

(which I will test on a number of platform versions).

since this breaks bootstrap at stage 2 on affected platform versions, so we 
need some
fix.

thanks
Iain




Re: [PATCH] jit: Ensure ssize_t is defined.

2024-01-28 Thread Iain Sandoe



> On 28 Jan 2024, at 21:25, Eric Gallager  wrote:
> 
> On Sun, Jan 28, 2024 at 6:45 AM Iain Sandoe  wrote:
>> 
>> Tested on i686, x86_64 Darwin, x86_64 Linux,
>> OK for trunk?
>> 
>> --- 8< ---
>> 
>> On some targets it seems that ssize_t is not defined by any of the
>> headers transitively included by .  This leads to a bootstrap
>> fail when jit is enabled.
>> 
>> The fix proposed here is to include sys/types.h when it is available
>> since that is where Posix specifies that ssize_t is defined.
>> 
>> gcc/jit/ChangeLog:
>> 
>>* libgccjit.h: Conditionally include  where it is
>>available to ensure declaration of ssize_t.
>> 
>> Signed-off-by: Iain Sandoe 
>> ---
>> gcc/jit/libgccjit.h | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
>> index 235cab053e0..db4f27a48bf 100644
>> --- a/gcc/jit/libgccjit.h
>> +++ b/gcc/jit/libgccjit.h
>> @@ -21,6 +21,9 @@ along with GCC; see the file COPYING3.  If not see
>> #define LIBGCCJIT_H
>> 
>> #include 
>> +#if __has_include()
> 
> Is __has_include() something that we can use unconditionally?

Hmm.. maybe we cannot, it seems it was introduced in gcc-4.9 and we only ask
for 4.8, IIRC.

I guess HAVE_SYS_TYPES_H might be an alternative (I’ll have to retest)

Iain

> 
>> +# include   /* For ssize_t.  */
>> +#endif
>> 
>> #ifdef __cplusplus
>> extern "C" {
>> --
>> 2.39.2 (Apple Git-143)



[PATCH] testsuite, Objective-C++: Update link flags [PR112863].

2024-01-28 Thread Iain Sandoe
Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks, 
Iain

--- 8< ---

These regressions are caused by missing or duplicate runpaths which
now fire linker warnings.

We need to add options to locate libobjc (and on Darwin libobjc-gnu)
along with libstdc++.
Usually '-L' options are added to point to the relevant directories for
the uninstalled libraries.

In cases where libraries are available as both shared and convenience
some additional checks are made.

For some targets -static- options are handled by specs substitution
and need a '-B' option rather than '-L'.  For Darwin, when embedded
runpaths are in use (the default for all versions after macOS 10.11),
'-B' is also needed to provide the runpath.

When '-B' is used, this results in a '-L' for each path that exists (so
that appending a '-L' as well is a needless duplicate).  There are also
cases where tools warn for duplicates, leading to spurious fails.

PR target/112863

gcc/testsuite/ChangeLog:

* lib/obj-c++.exp: Decide on whether to present -B or -L to
reference the paths to uninstalled libobjc/libobjc-gnu and
libstdc++ and use that to generate the link flags.
---
 gcc/testsuite/lib/obj-c++.exp | 69 +--
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp
index 397b70cb96a..854dc264f9d 100644
--- a/gcc/testsuite/lib/obj-c++.exp
+++ b/gcc/testsuite/lib/obj-c++.exp
@@ -110,34 +110,43 @@ proc obj-c++_link_flags { paths } {
 set shlib_ext [get_shlib_extension]
 verbose "shared lib extension: $shlib_ext"
 
+# We need to add options to locate libobjc/libobjc-gnu and libstdc++
+# Usually '-L' options are added to point to the relevant directories for
+# the uninstalled libraries.
+
+# In cases where libraries are available as both shared and convenience
+# some additional checks are made.
+
+# For some targets -static- options are handled by specs substitution
+# and need a '-B' option rather than '-L'.  For Darwin, when embedded
+# runpaths are in use (the default for all versions after macOS 10.11),
+# '-B' is also needed to provide the runpath.
+# When '-B' is used, this results in a '-L' for each path that exists (so
+# that appending a '-L' as well is a needless duplicate).  There are also
+# cases where tools warn for duplicates, leading to spurious fails.
+# Therefore the objective of the code below is to add just one '-L' or
+# '-B' for each of the libraries.
+
+set target_wants_B_option 0
+if { [istarget *-*-darwin9* ] || [istarget *-*-darwin\[12\]* ] } {
+  set target_wants_B_option 1
+}
+
 if { $gccpath != "" } {
-  if [file exists "${gccpath}/lib/libstdc++.a"] {
-  append ld_library_path ":${gccpath}/lib"
-  }
-  if [file exists "${gccpath}/libg++/libg++.a"] {
-  append flags " -L${gccpath}/libg++ "
-  append ld_library_path ":${gccpath}/libg++"
-  }
-  if [file exists "${gccpath}/libstdc++/libstdc++.a"] {
-  append flags " -L${gccpath}/libstdc++ "
-  append ld_library_path ":${gccpath}/libstdc++"
-  }
-  if [file exists "${gccpath}/libstdc++-v3/src/.libs/libstdc++.a"] {
-  # Allow for %s spec substitutions
-  append flags " -B${gccpath}/libstdc++-v3/src/.libs "
-  append flags " -L${gccpath}/libstdc++-v3/src/.libs "
-  append ld_library_path ":${gccpath}/libstdc++-v3/src/.libs"
-  }
-  # Look for libstdc++.${shlib_ext}.
-  if [file exists 
"${gccpath}/libstdc++-v3/src/.libs/libstdc++.${shlib_ext}"] {
- # Allow for %s spec substitutions
- append flags " -B${gccpath}/libstdc++-v3/src/.libs "
- append flags " -L${gccpath}/libstdc++-v3/src/.libs "
- append ld_library_path ":${gccpath}/libstdc++-v3/src/.libs"
+  if { [file exists "${gccpath}/libstdc++-v3/src/.libs/libstdc++.a"] ||
+ [file exists 
"${gccpath}/libstdc++-v3/src/.libs/libstdc++.${shlib_ext}"] } {
+   if { $target_wants_B_option } {
+   append flags "-B${gccpath}/libstdc++-v3/src/.libs "
+   } else {
+   append flags "-L${gccpath}/libstdc++-v3/src/.libs "
+   }
+   append ld_library_path ":${gccpath}/libstdc++-v3/src/.libs"
   }
+
   if [file exists "${gccpath}/libiberty/libiberty.a"] {
   append flags " -L${gccpath}/libiberty "
   }
+
   if [file exists "${gccpath}/librx/librx.a"] {
   append flags " -L${gccpath}/librx "
   }
@@ -159,9 +168,11 @@ proc obj-c++_link_flags { paths } {
 
   if { $libobjc_dir != "" } {
  set libobjc_dir [file dirname ${libobjc_dir}]
- # Allow for %s spec substitutions
- append flags " -B${libobjc_dir} "
- append flags " -L${libobjc_dir} "
+ if { $target_wants_B_option } {
+   

[PATCH] testsuite, libphobos: Update link flags [PR112864].

2024-01-28 Thread Iain Sandoe
Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks, 
Iain

--- 8< ---

The regressions here are primarily from duplicated '-B' additions.

We remove the duplicate on the link line.
We also make sure that platforms with extensions other than .so for
shared libs will have the correct paths.

PR target/112864

libphobos/ChangeLog:

* testsuite/lib/libphobos.exp: Use ${shlib_ext} instead of
hard-wiring '.so'.
* testsuite/testsuite_flags.in: Remove duplicate -B option
for spec file path.
---
 libphobos/testsuite/lib/libphobos.exp  | 2 +-
 libphobos/testsuite/testsuite_flags.in | 6 ++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/libphobos/testsuite/lib/libphobos.exp 
b/libphobos/testsuite/lib/libphobos.exp
index 191cafa534d..d4aa433ddc1 100644
--- a/libphobos/testsuite/lib/libphobos.exp
+++ b/libphobos/testsuite/lib/libphobos.exp
@@ -234,7 +234,7 @@ proc libphobos_init { args } {
if { "$mldir" == "." } {
  continue
}
-   if { [llength [glob -nocomplain ${gccdir}/${mldir}/libgcc_s*.so.*]] 
>= 1 } {
+   if { [llength [glob -nocomplain 
${gccdir}/${mldir}/libgcc_s*.${shlib_ext}*]] >= 1 } {
  append ld_library_path ":${gccdir}/${mldir}"
}
  }
diff --git a/libphobos/testsuite/testsuite_flags.in 
b/libphobos/testsuite/testsuite_flags.in
index 528cff4bf13..8a412d2f1fa 100755
--- a/libphobos/testsuite/testsuite_flags.in
+++ b/libphobos/testsuite/testsuite_flags.in
@@ -46,10 +46,8 @@ case ${query} in
   echo ${GDCPATHS_default} ${GDCPATHS_config}
   ;;
 --gdcldflags)
-  GDCLDFLAGS="-B${BUILD_DIR}/src
-  -B${BUILD_DIR}/libdruntime/gcc
-  -B${BUILD_DIR}/src/.libs
-  -L${BUILD_DIR}/src/.libs"
+  GDCLDFLAGS="-B${BUILD_DIR}/libdruntime/gcc
+  -B${BUILD_DIR}/src/.libs"
   echo ${GDCLDFLAGS}
   ;;
 *)
-- 
2.39.2 (Apple Git-143)



[PATCH] testsuite, gfortan: Update link flags [PR112862].

2024-01-28 Thread Iain Sandoe
Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks, 
Iain

--- 8< ---

The regressions here are caused by two issues:
1. In some cases there is no generated runpath for libatomic
2. In other cases there are duplicate paths.

This patch simplifies the addition of the options in the main
gfortran exp and removes the duplicates elewhere.

We need to add options to locate libgfortran and the dependent libs
libquadmath (supporting REAL*16) and libatomic (supporting operations
used by coarrays).  Usually '-L' options are added to point to the
relevant directories for the uninstalled libraries.

In cases where libraries are available as both shared and convenience
some additional checks are made.

For some targets -static- options are handled by specs substitution
and need a '-B' option rather than '-L'.  For Darwin, when embedded
runpaths are in use (the default for all versions after macOS 10.11),
'-B' is also needed to provide the runpath.

When '-B' is used, this results in a '-L' for each path that exists (so
that appending a '-L' as well is a needless duplicate).  There are also
cases where tools warn for duplicates, leading to spurious fails.

PR target/112862

gcc/testsuite/ChangeLog:

* gfortran.dg/coarray/caf.exp: Remove duplicate additions of
libatomic handling.
* gfortran.dg/dg.exp: Likewise.
* lib/gfortran.exp: Decide on whether to present -B or -L to
reference the paths to uninstalled libgfortran, libqadmath and
libatomic and use that to generate the link flags.
---
 gcc/testsuite/gfortran.dg/coarray/caf.exp | 16 +
 gcc/testsuite/gfortran.dg/dg.exp  | 20 ---
 gcc/testsuite/lib/gfortran.exp| 73 +++
 3 files changed, 51 insertions(+), 58 deletions(-)

diff --git a/gcc/testsuite/gfortran.dg/coarray/caf.exp 
b/gcc/testsuite/gfortran.dg/coarray/caf.exp
index dae46bd92fa..31c13cd34e5 100644
--- a/gcc/testsuite/gfortran.dg/coarray/caf.exp
+++ b/gcc/testsuite/gfortran.dg/coarray/caf.exp
@@ -70,18 +70,6 @@ proc dg-compile-aux-modules { args } {
 }
 }
 
-# Add -latomic only where supported.  Assume built-in support elsewhere.
-set maybe_atomic_lib ""
-if [check_effective_target_libatomic_available] {
-if ![is_remote host] {
-   if [info exists TOOL_OPTIONS] {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs 
${TOOL_OPTIONS}]]"
-   } else {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs]]"
-   }
-}
-}
-
 # Main loop.
 foreach test [lsort [glob -nocomplain $srcdir/$subdir/*.\[fF\]{,90,95,03,08} 
]] {
 # If we're only testing specific files and this isn't one of them, skip it.
@@ -105,14 +93,14 @@ foreach test [lsort [glob -nocomplain 
$srcdir/$subdir/*.\[fF\]{,90,95,03,08} ]]
 foreach flags $option_list {
verbose "Testing $nshort (single), $flags" 1
 set gfortran_aux_module_flags "-fcoarray=single $flags"
-   dg-test $test "-fcoarray=single $flags" $maybe_atomic_lib
+   dg-test $test "-fcoarray=single $flags" {}
cleanup-modules ""
 }
 
 foreach flags $option_list {
verbose "Testing $nshort (libcaf_single), $flags" 1
 set gfortran_aux_module_flags "-fcoarray=lib $flags -lcaf_single"
-   dg-test $test "-fcoarray=lib $flags -lcaf_single" $maybe_atomic_lib
+   dg-test $test "-fcoarray=lib $flags -lcaf_single" {}
cleanup-modules ""
 }
 }
diff --git a/gcc/testsuite/gfortran.dg/dg.exp b/gcc/testsuite/gfortran.dg/dg.exp
index f936fd38644..7a9cb89c194 100644
--- a/gcc/testsuite/gfortran.dg/dg.exp
+++ b/gcc/testsuite/gfortran.dg/dg.exp
@@ -54,27 +54,7 @@ proc dg-compile-aux-modules { args } {
 }
 }
 
-# coarray tests might need libatomic.  Assume that it is either not needed or
-# provided by builtins if it's not available.
-set maybe_atomic_lib ""
-if [check_effective_target_libatomic_available] {
-if ![is_remote host] {
-   if [info exists TOOL_OPTIONS] {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs 
${TOOL_OPTIONS}]]"
-   } else {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs]]"
-   }
-} else {
-set maybe_atomic_lib ""
-}
-}
-
 set all_flags $DEFAULT_FFLAGS
-if { $maybe_atomic_lib != "" } {
-   foreach f $maybe_atomic_lib {
- lappend all_flags $f
-   }
-}
 
 # Main loop.
 gfortran-dg-runtest [lsort \
diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
index c3e258b410b..1ccb81ccec5 100644
--- a/gcc/testsuite/lib/gfortran.exp
+++ b/gcc/testsuite/lib/gfortran.exp
@@ -79,6 +79,7 @@ proc gfortran_link_flags { paths } {
 global ld_library_path
 global GFORTRAN_UNDER_TEST
 global shlib_ext
+global ENABLE_DARWIN_AT_RPATH
 
 set gccpath ${paths}
 set libio_dir ""
@@ -87,39 +88,63 @@ proc gfortran_link_flags { paths } {
 set shlib_ext [get_shlib_extension]
 verbose "shared lib extension: $shlib_ext"
 
+ 

[PATCH] testsuite, GDC: Update link flags [PR112861].

2024-01-28 Thread Iain Sandoe
Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks, 
Iain

--- 8< ---

The regressions here are because we do not generate a runpath for
the uninstalled libstdc++.  This patch updates the link flags handling
to simplify it.

We need to add options to locate both libgphobos and libstdc++
Usually '-L' options are added to point to the relevant directories for
the uninstalled libraries.

In cases where libraries are available as both shared and convenience
some additional checks are made.

For some targets -static- options are handled by specs substitution
and need a '-B' option rather than '-L'.  For Darwin, when embedded
runpaths are in use (the default for all versions after macOS 10.11),
'-B' is also needed to provide the runpath.

When '-B' is used, this results in a '-L' for each path that exists (so
that appending a '-L' as well is a needless duplicate).  There are also
cases where tools warn for duplicates, leading to spurious fails.
Therefore the objective is to add a single -B/-L option for each needed
path.

PR target/112861

gcc/testsuite/ChangeLog:

* lib/gdc.exp: Decide on whether to present -B or -L to reference
the paths to uninstalled libphobos and libstdc++ and use that to
generate the link flags.
---
 gcc/testsuite/lib/gdc.exp | 40 ---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp
index 3045aa82918..3c284229609 100644
--- a/gcc/testsuite/lib/gdc.exp
+++ b/gcc/testsuite/lib/gdc.exp
@@ -133,6 +133,28 @@ proc gdc_link_flags { paths } {
set include_cxx_flags 0
 }
 
+# We need to add options to locate libgphobos and libstdc++
+# Usually '-L' options are added to point to the relevant directories for
+# the uninstalled libraries.
+
+# In cases where libraries are available as both shared and convenience
+# some additional checks are made.
+
+# For some targets -static- options are handled by specs substitution
+# and need a '-B' option rather than '-L'.  For Darwin, when embedded
+# runpaths are in use (the default for all versions after macOS 10.11),
+# '-B' is also needed to provide the runpath.
+# When '-B' is used, this results in a '-L' for each path that exists (so
+# that appending a '-L' as well is a needless duplicate).  There are also
+# cases where tools warn for duplicates, leading to spurious fails.
+# Therefore the objective of the code below is to add just one '-L' or
+# '-B' for each of the libraries.
+
+set target_wants_B_option 0
+if { [istarget *-*-darwin9* ] || [istarget *-*-darwin\[12\]* ] } {
+  set target_wants_B_option 1
+}
+
 if { $gccpath != "" } {
# Path to libgphobos.spec.
append flags "-B${gccpath}/libphobos/src "
@@ -143,7 +165,11 @@ proc gdc_link_flags { paths } {
 
if { [file exists "${gccpath}/libphobos/src/.libs/libgphobos.a"] \
 || [file exists 
"${gccpath}/libphobos/src/.libs/libgphobos.${shlib_ext}"] } {
-   append flags "-L${gccpath}/libphobos/src/.libs "
+   if { $target_wants_B_option } {
+   append flags "-B${gccpath}/libphobos/src/.libs "
+   } else {
+   append flags "-L${gccpath}/libphobos/src/.libs "
+   }
append ld_library_path ":${gccpath}/libphobos/src/.libs"
}
# Static linking is default. If only the shared lib is available adjust
@@ -163,7 +189,11 @@ proc gdc_link_flags { paths } {
if $include_cxx_flags {
if { [file exists "${gccpath}/libstdc++-v3/src/.libs/libstdc++.a"] \
 || [file exists 
"${gccpath}/libstdc++-v3/src/.libs/libstdc++.${shlib_ext}"] } {
-   append flags "-L${gccpath}/libstdc++-v3/src/.libs "
+   if { $target_wants_B_option } {
+   append flags "-B${gccpath}/libstdc++-v3/src/.libs "
+   } else {
+   append flags "-L${gccpath}/libstdc++-v3/src/.libs "
+   }
append ld_library_path ":${gccpath}/libstdc++-v3/src/.libs"
}
}
@@ -173,7 +203,11 @@ proc gdc_link_flags { paths } {
 
set libphobos [lookfor_file ${tool_root_dir} libgphobos]
if { $libphobos != "" } {
-   append flags "-B${libphobos} -L${libphobos} "
+   if { $target_wants_B_option } {
+ append flags "-B${libphobos} "
+   } else { 
+ append flags " -L${libphobos} "
+   }
append ld_library_path ":${libphobos}"
}
set libiberty [lookfor_file ${tool_root_dir} libiberty]
-- 
2.39.2 (Apple Git-143)



[PATCH 2/2] libgcc: Make heap trampoline support dynamic [PR113403].

2024-01-28 Thread Iain Sandoe
In order to handle system security constraints during GCC build
and test and that most platform versions cannot link to libgcc_eh
since the unwinder there is incompatible with the system one.

1. We make the support functions weak definitions.
2. We include them as a CRT for platform conditions that do not
   allow libgcc_eh.
3. We ensure that the weak symbols are exported from DSOs (which
   includes exes on Darwin) so that the dynamic linker will
   pick one instance (which avoids duplication of trampoline
   caches).

PR libgcc/113403

gcc/ChangeLog:

* config/darwin.h (DARWIN_SHARED_WEAK_ADDS, DARWIN_WEAK_CRTS): New.
(REAL_LIBGCC_SPEC): Move weak CRT handling to separate spec.
* config/i386/darwin.h (DARWIN_HEAP_T_LIB): New.
* config/i386/darwin32-biarch.h (DARWIN_HEAP_T_LIB): New.
* config/i386/darwin64-biarch.h (DARWIN_HEAP_T_LIB): New.
* config/rs6000/darwin.h (DARWIN_HEAP_T_LIB): New.

libgcc/ChangeLog:

* config.host: Build libheap_t.a for i686/x86_64 Darwin.
* config/aarch64/heap-trampoline.c (HEAP_T_ATTR): New.
(allocate_tramp_ctrl): Allow a target to build this as a weak def.
(__gcc_nested_func_ptr_created): Likewise.
* config/i386/heap-trampoline.c (HEAP_T_ATTR): New.
(allocate_tramp_ctrl): Allow a target to build this as a weak def.
(__gcc_nested_func_ptr_created): Likewise.
* config/t-darwin: Build libheap_t.a (a CRT with heap trampoline
support).
---
 gcc/config/darwin.h | 43 -
 gcc/config/i386/darwin.h|  2 ++
 gcc/config/i386/darwin32-biarch.h   |  3 ++
 gcc/config/i386/darwin64-biarch.h   |  3 ++
 gcc/config/rs6000/darwin.h  |  3 ++
 libgcc/config.host  |  7 ++--
 libgcc/config/aarch64/heap-trampoline.c |  8 +
 libgcc/config/i386/heap-trampoline.c|  8 +
 libgcc/config/t-darwin  | 13 
 9 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index cb96d67b3b1..31019a0c49d 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -314,13 +314,17 @@ extern GTY(()) int darwin_ms_struct;
 # define DARWIN_RPATH_LINK \
 "%{!r:%{!nostdlib:%{!nodefaultrpaths:%(darwin_rpaths)}}}"
 # define DARWIN_SHARED_LIBGCC "-lgcc_s.1.1"
+# define DARWIN_SHARED_WEAK_ADDS " "
 #else
 # define DARWIN_RPATH_LINK ""
 # define DARWIN_SHARED_LIBGCC \
-"%:version-compare(!> 10.11 mmacosx-version-min= -lgcc_s.1.1) \
- %:version-compare(>= 10.11 mmacosx-version-min= -lemutls_w) "
+"%:version-compare(!> 10.11 mmacosx-version-min= -lgcc_s.1.1)"
+# define DARWIN_SHARED_WEAK_ADDS \
+"%{%:version-compare(>= 10.11 mmacosx-version-min= -lemutls_w): \
+ " DARWIN_HEAP_T_LIB "}"
 #endif
 
+
 /* We might elect to add a path even when this compiler does not use embedded
run paths, so that we can use libraries from an alternate compiler that is
using embedded runpaths.  */
@@ -398,7 +402,9 @@ extern GTY(()) int darwin_ms_struct;
 %{e*} %{r} \
 %{o*}%{!o:-o a.out} \
 %{!r:%{!nostdlib:%{!nostartfiles:%S}}} \
-%{L*} %(link_libgcc) %o \
+%{L*} %(link_libgcc) \
+%{!r:%{!nostdlib:%{!nodefaultlibs: " DARWIN_WEAK_CRTS "}}} \
+%o \
 %{!r:%{!nostdlib:%{!nodefaultlibs:\
   %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} \
   %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1): \
@@ -412,15 +418,15 @@ extern GTY(()) int darwin_ms_struct;
   %(link_ssp) \
   %:version-compare(>< 10.6 10.7 mmacosx-version-min= -ld10-uwfef) \
   %(link_gcc_c_sequence) \
-  %{!nodefaultexport:%{dylib|dynamiclib|bundle: \
-   %:version-compare(>= 10.11 asm_macosx_version_min= -U) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= 
___emutls_get_address) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= -exported_symbol) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= 
___emutls_get_address) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= -U) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= 
___emutls_register_common) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= -exported_symbol) \
-   %:version-compare(>= 10.11 asm_macosx_version_min= 
___emutls_register_common) \
+  %{!nodefaultexport: \
+   %{%:version-compare(>= 10.11 asm_macosx_version_min= -U): \
+  ___emutls_get_address -exported_symbol ___emutls_get_address \
+ -U ___emutls_register_common \
+ -exported_symbol ___emutls_register_common \
+ -U ___gcc_nested_func_ptr_created \
+ -exported_symbol ___gcc_nested_func_ptr_created \
+ -U ___gcc_nested_func_ptr_deleted \
+ -exported_symbol ___gcc_nested_func_ptr_deleted \
   }} \
 }}}\
 %{!r:%{!nostdlib:%{!nostartfiles:%E}}} %{T*} %{F*} "\
@@ -542,16 +548,21 @@ extern GTY(()) int darwin_ms_struct;
 #undef REAL_LIBGCC_SPEC
 

[PATCH 1/2] libgcc: Make heap trampoline support dynamic [PR113403].

2024-01-28 Thread Iain Sandoe
This removes the heap trampoline support functions from libgcc.a and
adds them to libgcc_eh.a.  They are also present in libgcc_s.

PR libgcc/113403

libgcc/ChangeLog:

* config/aarch64/t-heap-trampoline: Move the heap trampoline
support functions from libgcc.a to libgcc_eh.a.
* config/i386/t-heap-trampoline: Likewise.
---
 libgcc/config/aarch64/t-heap-trampoline | 3 ++-
 libgcc/config/i386/t-heap-trampoline| 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/libgcc/config/aarch64/t-heap-trampoline 
b/libgcc/config/aarch64/t-heap-trampoline
index b22480800b2..6468fb8704f 100644
--- a/libgcc/config/aarch64/t-heap-trampoline
+++ b/libgcc/config/aarch64/t-heap-trampoline
@@ -16,4 +16,5 @@
 # along with GCC; see the file COPYING3.  If not see
 # .
 
-LIB2ADD += $(srcdir)/config/aarch64/heap-trampoline.c
+LIB2ADDEH += $(srcdir)/config/aarch64/heap-trampoline.c
+LIB2ADDEHSHARED += $(srcdir)/config/aarch64/heap-trampoline.c
diff --git a/libgcc/config/i386/t-heap-trampoline 
b/libgcc/config/i386/t-heap-trampoline
index 613f635b1f6..728c1e26a92 100644
--- a/libgcc/config/i386/t-heap-trampoline
+++ b/libgcc/config/i386/t-heap-trampoline
@@ -16,4 +16,5 @@
 # along with GCC; see the file COPYING3.  If not see
 # .
 
-LIB2ADD += $(srcdir)/config/i386/heap-trampoline.c
+LIB2ADDEH += $(srcdir)/config/i386/heap-trampoline.c
+LIB2ADDEHSHARED += $(srcdir)/config/aarch64/heap-trampoline.c
-- 
2.39.2 (Apple Git-143)



[PATCH 0/2] libgcc: Make heap trampoline support dynamic [PR113403].

2024-01-28 Thread Iain Sandoe
This series follows Jakub's suggestion in the PR (for Linux in the first patch)
and handles Darwin-specific cases (in the second).

Sorry this has taken a while, the Darwin permutations had some glitches which
necessitated re-tests on several OS versions.

Tested on x86_64 (and aarch64) Darwin, x86_64, aarch64 Linux.
OK for trunk for patch 1 and the non-Darwin parts of patch2.
thanks,
Iain




[pushed] Objective-C, Darwin: Do not overalign CFStrings and Objective-C metadata.

2024-01-28 Thread Iain Sandoe
Tested on i686, powerpc, x86_64 Darwin, x86_64, aarch64 Linux, pushed
to trunk, thanks,
Iain

--- 8< ---

We have reports of regressions in both Objective-C and Objective-C++ on
Darwin23 (macOS 14).  In some cases, these are linker warnings about the
alignment of CFString constants; in other cases the built executables
crash during runtime initialization.  The underlying issue is the same in
both cases; since the objects (CFStrings, Objective-C meta-data) are TU-
local, we are choosing to increase their alignment for efficiency - to
values greater than ABI alignment.

However, although these objects are TU-local, they are also visible to the
linker (since they are placed in specific named sections).  In many cases
the metadata can be regarded as tables of data, and thus it is expected
that these sections can be concatenated from multiple TUs and the data
treated as tabular.  In order for this to work the data cannot be allowed
to exceed ABI alignment - which leads to the crashes.

For GCC-15+ it would be nice to find a more elegant solution to this issue
(perhaps by adjusting the concept of binds-locally to exclude specific
named sections) - but I do not want to do that in stage 4.

The solution here is to force the alignment to be preserved as created by
setting DECL_USER_ALIGN on the relevant objects.

gcc/ChangeLog:

* config/darwin.cc (darwin_build_constant_cfstring): Prevent over-
alignment of CFString constants by setting DECL_USER_ALIGN.

gcc/objc/ChangeLog:

* objc-next-runtime-abi-02.cc (build_v2_address_table): Prevent
over-alignment of Objective-C metadata by setting DECL_USER_ALIGN
on relevant variables.
(build_v2_protocol_list_address_table): Likewise.
(generate_v2_protocol_list): Likewise.
(generate_v2_meth_descriptor_table): Likewise.
(generate_v2_meth_type_list): Likewise.
(generate_v2_property_table): Likewise.
(generate_v2_dispatch_table): Likewise.
(generate_v2_ivars_list): Likewise.
(generate_v2_class_structs): Likewise.
(build_ehtype): Likewise.
* objc-runtime-shared-support.cc (generate_strings): Likewise.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.cc|  1 +
 gcc/objc/objc-next-runtime-abi-02.cc| 18 +++---
 gcc/objc/objc-runtime-shared-support.cc |  4 
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index 7f43718820b..9e5d64e6f32 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -3909,6 +3909,7 @@ darwin_build_constant_cfstring (tree str)
   /* global namespace.  */
   DECL_CONTEXT (var) = NULL_TREE;
   DECL_INITIAL (var) = constructor;
+  DECL_USER_ALIGN (var) = 1;
   lang_hooks.decls.pushdecl (var);
   rest_of_decl_compilation (var, 1, 0);
   desc->ccf_str = var;
diff --git a/gcc/objc/objc-next-runtime-abi-02.cc 
b/gcc/objc/objc-next-runtime-abi-02.cc
index a622f4cbf4e..b3de6d789a5 100644
--- a/gcc/objc/objc-next-runtime-abi-02.cc
+++ b/gcc/objc/objc-next-runtime-abi-02.cc
@@ -2249,6 +2249,7 @@ build_v2_address_table (vec *src, const char 
*nam, tree attr)
   DECL_PRESERVE_P (decl) = 1;
   expr = objc_build_constructor (type, initlist);
   OBJCMETA (decl, objc_meta, attr);
+  DECL_USER_ALIGN (decl) = 1;
   finish_var_decl (decl, expr);
 }
 
@@ -2323,8 +2324,9 @@ build_v2_protocol_list_address_table (void)
decl = create_global_decl (objc_protocol_type, buf, /*is def=*/true);
   expr = convert (objc_protocol_type, build_fold_addr_expr (ref->refdecl));
   OBJCMETA (decl, objc_meta, meta_label_protocollist);
-  finish_var_decl (decl, expr);
   DECL_PRESERVE_P (decl) = 1;
+  DECL_USER_ALIGN (decl) = 1;
+  finish_var_decl (decl, expr);
 }
 
 /* TODO: delete the vec.  */
@@ -2402,6 +2404,7 @@ generate_v2_protocol_list (tree i_or_p, tree klass_ctxt)
   /* ObjC2 puts all these in the base section.  */
   OBJCMETA (refs_decl, objc_meta, meta_base);
   DECL_PRESERVE_P (refs_decl) = 1;
+  DECL_USER_ALIGN (refs_decl) = 1;
   finish_var_decl (refs_decl,
   objc_build_constructor (TREE_TYPE (refs_decl),initlist));
   return refs_decl;
@@ -2510,6 +2513,7 @@ generate_v2_meth_descriptor_table (tree chain, tree 
protocol,
   CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, initlist);
   /* Get into the right section.  */
   OBJCMETA (decl, objc_meta, attr);
+  DECL_USER_ALIGN (decl) = 1;
   finish_var_decl (decl, objc_build_constructor (method_list_template, v));
   return decl;
 }
@@ -2528,13 +2532,14 @@ generate_v2_meth_type_list (vec& all_meths, tree 
protocol,
IDENTIFIER_POINTER (PROTOCOL_NAME (protocol)));
   tree decl = start_var_decl (list_type, nam);
   free (nam);
-  OBJCMETA (decl, objc_meta, meta_base);
   vec *v = NULL;
 
   for (unsigned i = 0; i < size; ++i)
 CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
add_objc_string 

[PATCH] jit: Ensure ssize_t is defined.

2024-01-28 Thread Iain Sandoe
Tested on i686, x86_64 Darwin, x86_64 Linux,
OK for trunk?

--- 8< ---

On some targets it seems that ssize_t is not defined by any of the
headers transitively included by .  This leads to a bootstrap
fail when jit is enabled.

The fix proposed here is to include sys/types.h when it is available
since that is where Posix specifies that ssize_t is defined.

gcc/jit/ChangeLog:

* libgccjit.h: Conditionally include  where it is
available to ensure declaration of ssize_t.

Signed-off-by: Iain Sandoe 
---
 gcc/jit/libgccjit.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 235cab053e0..db4f27a48bf 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -21,6 +21,9 @@ along with GCC; see the file COPYING3.  If not see
 #define LIBGCCJIT_H
 
 #include 
+#if __has_include()
+# include   /* For ssize_t.  */
+#endif
 
 #ifdef __cplusplus
 extern "C" {
-- 
2.39.2 (Apple Git-143)



[pushed] testsuite, Objective-C: Fix duplicate libobjc cases.

2024-01-28 Thread Iain Sandoe
tested on i686, powerpc, x86_64 Darwin, x86_64, aarch64 Linux, pushed
to trunk, thanks,
Iain

--- 8< ---

Two of the encode testcases include '-lobjc' as their dg-options.
Since the library is already appended as part of the generic testsuite
handling,  this means that two instances appear on the link line leading
to spurious warnings from Darwin's new linker.

gcc/testsuite/ChangeLog:

* obj-c++.dg/encode-10.mm: Remove unneeded '-lobjc' option addition.
* obj-c++.dg/encode-9.mm: Likewise.

Signed-off-by: Iain Sandoe 
---
 gcc/testsuite/obj-c++.dg/encode-10.mm | 1 -
 gcc/testsuite/obj-c++.dg/encode-9.mm  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/gcc/testsuite/obj-c++.dg/encode-10.mm 
b/gcc/testsuite/obj-c++.dg/encode-10.mm
index 2b3af88dcb1..2477400b7f3 100644
--- a/gcc/testsuite/obj-c++.dg/encode-10.mm
+++ b/gcc/testsuite/obj-c++.dg/encode-10.mm
@@ -1,5 +1,4 @@
 /* Test for @encode in templates.  */
-/* { dg-options "-lobjc" } */
 /* { dg-do run } */
 #include
 #include 
diff --git a/gcc/testsuite/obj-c++.dg/encode-9.mm 
b/gcc/testsuite/obj-c++.dg/encode-9.mm
index 6b064dfdc09..4fdb5fa50fb 100644
--- a/gcc/testsuite/obj-c++.dg/encode-9.mm
+++ b/gcc/testsuite/obj-c++.dg/encode-9.mm
@@ -1,5 +1,4 @@
 /* Test than @encode is properly instantiated. */
-/* { dg-options "-lobjc" } */
 /* { dg-do run } */
 
 #include
-- 
2.39.2 (Apple Git-143)



Re: [PATCH v3] Fix __builtin_nested_func_ptr_{created, deleted} symbol versions [PR113402]

2024-01-28 Thread Iain Sandoe


> On 18 Jan 2024, at 15:05, Jakub Jelinek  wrote:
> 
> On Thu, Jan 18, 2024 at 02:59:23PM +0000, Iain Sandoe wrote:
>> --- a/gcc/builtins.cc
>> +++ b/gcc/builtins.cc
>> @@ -8416,6 +8416,11 @@ expand_builtin (tree exp, rtx target, rtx subtarget, 
>> machine_mode mode,
>> case BUILT_IN_ADJUST_DESCRIPTOR:
>>   return expand_builtin_adjust_descriptor (exp);
>> 
>> +case BUILT_IN_NESTED_PTR_CREATED:
>> +case BUILT_IN_NESTED_PTR_DELETED:
> 
> Unsure if it is ok to have the BUILT_IN_ names so different from the actual
> functions, if they shouldn't be
> BUILT_IN_GCC_NESTED_FUNC_PTR_{CREATED,DELETED} instead.
> The missing __ is what happens even with BUILT_IN_CLEAR_CACHE / __clear_cache.
> 
>> +  break; /* At present, no expansion, just call the function.  */
>> +
>> +
> 
> Just one empty newline, not 2.
> 
>> case BUILT_IN_FORK:
>> case BUILT_IN_EXECL:
>> case BUILT_IN_EXECV:
>> diff --git a/gcc/builtins.def b/gcc/builtins.def
>> index 4d97ca0eec9..fd040eb8d80 100644
>> --- a/gcc/builtins.def
>> +++ b/gcc/builtins.def
>> @@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, 
>> "__builtin_adjust_trampoline")
>> DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
>> DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
>> DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
>> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, 
>> "__builtin_nested_func_ptr_created")
>> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, 
>> "__builtin_nested_func_ptr_deleted")
>> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_CREATED, 
>> "__gcc_nested_func_ptr_created", BT_FN_VOID_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
>> +DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_DELETED, 
>> "__gcc_nested_func_ptr_deleted", BT_FN_VOID, ATTR_NOTHROW_LIST)
> 
> See above.
> 
> Otherwise LGTM

This is what I pushed, thanks
Iain


From 837827f8f2542c36ba5944b4da0a76ea6a64b08b Mon Sep 17 00:00:00 2001
From: Iain Sandoe 
Date: Tue, 16 Jan 2024 10:21:14 +
Subject: [PATCH] Fix __builtin_nested_func_ptr_{created,deleted} symbol
 versions [PR113402]

The symbols for the functions supporting heap-based trampolines were
exported at an incorrect symbol version, the following patch fixes that.

As requested in the PR, this also renames __builtin_nested_func_ptr* to
__gcc_nested_func_ptr*.  In carrying our the rename, we move the builtins
to use DEF_EXT_LIB_BUILTIN.

PR libgcc/113402

gcc/ChangeLog:

* builtins.cc (expand_builtin): Handle BUILT_IN_GCC_NESTED_PTR_CREATED
and BUILT_IN_GCC_NESTED_PTR_DELETED.
* builtins.def (BUILT_IN_GCC_NESTED_PTR_CREATED,
BUILT_IN_GCC_NESTED_PTR_DELETED): Make these builtins LIB-EXT and
rename the library fallbacks to __gcc_nested_func_ptr_created and
__gcc_nested_func_ptr_deleted.
* doc/invoke.texi: Rename these to __gcc_nested_func_ptr_created
and __gcc_nested_func_ptr_deleted.
* tree-nested.cc (finalize_nesting_tree_1): Use builtin_explicit for
BUILT_IN_GCC_NESTED_PTR_CREATED and BUILT_IN_GCC_NESTED_PTR_DELETED.
* tree.cc (build_common_builtin_nodes): Build the
BUILT_IN_GCC_NESTED_PTR_CREATED and BUILT_IN_GCC_NESTED_PTR_DELETED 
local
builtins only for non-explicit.

libgcc/ChangeLog:

* config/aarch64/heap-trampoline.c: Rename
__builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
__builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
    * config/i386/heap-trampoline.c: Likewise.
* libgcc2.h: Likewise.
* libgcc-std.ver.in (GCC_7.0.0): Likewise and then move
__gcc_nested_func_ptr_created and
__gcc_nested_func_ptr_deleted from this symbol version to ...
(GCC_14.0.0): ... this one.

Signed-off-by: Iain Sandoe 
Co-authored-by: Jakub Jelinek  
---
 gcc/builtins.cc |  4 
 gcc/builtins.def|  4 ++--
 gcc/doc/invoke.texi |  4 ++--
 gcc/tree-nested.cc  |  4 ++--
 gcc/tree.cc | 31 ++---
 libgcc/config/aarch64/heap-trampoline.c |  8 +++
 libgcc/config/i386/heap-trampoline.c|  8 +++
 libgcc/libgcc-std.ver.in|  5 ++--
 libgcc/libgcc2.h|  4 ++--
 9 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 09f2354f114..a0bd82c7981 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -8416,6 +8416,10 @@ expand_builtin (tree exp, rtx target, rtx subtarget, 
machine_mode mode,
 case BUIL

Re: [PATCH] jit, Darwin: Implement library exports list.

2024-01-25 Thread Iain Sandoe
Hi David,

> On 24 Jan 2024, at 18:31, David Malcolm  wrote:
> 
> On Tue, 2024-01-16 at 11:10 +0000, Iain Sandoe wrote:
>> Tested on x86_64, i686 Darwin and x86_64 Linux,
>> OK for trunk? when ?
>> thanks,
>> Iain
> 
> Hi Iain, thanks for the patch.
> 
> I'll have to defer to your Darwin expertise here; given that you've
> tested it on the above configurations I'll assume it's correct, but...
> 
>> 
>> --- 8< ---
>> 
>> Currently, we have no exports list for libgccjit, which means that
>> all symbols are exported, including those from libstdc++ which is
>> linked statically into the lib.  This causes failures when the
>> shared libstdc++ is used but some c++ symbols are satisfied from
>> libgccjit.
>> 
>> This implements an export file for Darwin (which is currently
>> manually created by cross-checking libgccjit.map).
> 
> ...I'm a little nervous about this; Antoyo has a number of out-of-tree
> patches we're working towards merging, and almost all of these touch
> libgccjit.map.
> 
> 
>>   Ideally we'd
>> script this, at some point.  
> 
> Yes.  How about a Python 3 script (inside "contrib", or in "gcc/jit")
> that would do that.  

I’m not sure we want to make a build dependency on Python 3.. 
the reason I say ‘build’ is ...

> Then whenever a patch touches libgccjit.map we'd
> run that script to regenerate libgccjit.exp in the source tree.  I can
> have a go at writing it, if you think that's the best way to go.

… there are two other places in the current sources where ld map files
are converted to Darwin (and Solaris) symbol export files [libgcc, libstdc++].

In these cases, the export file is created on-the-fly at build time by scripts
(IIRC a mixture of awk, sh, perl).

This requires more surgery to the Make stuff and that we have a suitable
script - but it does mean that we do not need to commit the Darwin (and
potentially Solaris) versions to the source tree.

I’m actually happy with either solution - since we do not expect this to
be a daily occurance, we could have a maintainter’s python3 script to
update a committed export file or we could try the mechanism used in
the other two places (but then the script would need to use awk/sh/perl
to avoid new build deps).

> I take it .exp is the standard extension for these exports file in the
> Darwin world.  If so, it's a shame (but unavoidable) that it clashes
> with the existing uses of .exp in our source tree for our
> expect/Tcl/DejaGnu sources.

I suspect the linker will accept other extensions, although ‘exp’ is a
convention used elsewhere, it is unfortunate that it clashes indeed.
 - let me try an alternate (e.g. .export) and report back.

> I think the patch as-is is OK for trunk now, assuming that you've
> tested it as above.

I’m going to hold off on this for now (but do want some solution before
14 branches, because there are quite a few new fails from it).

Iain


> 
> Dave
> 
> 
>> Update libtool current and age to
>> reflect the current ABI version (we are not bumping the SO name
>> at this stage).
>> 
>> This fixes a number of new failures in jit testing.
>> 
>> gcc/jit/ChangeLog:
>> 
>> * Make-lang.in: Implement exports list, and use a shared
>> libgcc.
>> * libgccjit.exp: New file.
>> 
>> Signed-off-by: Iain Sandoe 
>> ---
>>  gcc/jit/Make-lang.in  |  38 ---
>>  gcc/jit/libgccjit.exp | 229
>> ++
>>  2 files changed, 251 insertions(+), 16 deletions(-)
>>  create mode 100644 gcc/jit/libgccjit.exp
>> 
>> diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in
>> index b1f0ce73e12..52dc2c24908 100644
>> --- a/gcc/jit/Make-lang.in
>> +++ b/gcc/jit/Make-lang.in
>> @@ -55,7 +55,10 @@ else
>>  
>>  ifneq (,$(findstring darwin,$(host)))
>>  
>> -LIBGCCJIT_AGE = 1
>> +LIBGCCJIT_CURRENT = 26
>> +LIBGCCJIT_REVISION = 0
>> +LIBGCCJIT_AGE = 26
>> +LIBGCCJIT_COMPAT = 0
>>  LIBGCCJIT_BASENAME = libgccjit
>>  
>>  LIBGCCJIT_SONAME = \
>> @@ -63,15 +66,15 @@ LIBGCCJIT_SONAME = \
>>  LIBGCCJIT_FILENAME =
>> $(LIBGCCJIT_BASENAME).$(LIBGCCJIT_VERSION_NUM).dylib
>>  LIBGCCJIT_LINKER_NAME = $(LIBGCCJIT_BASENAME).dylib
>>  
>> -# Conditionalize the use of the LD_VERSION_SCRIPT_OPTION and
>> -# LD_SONAME_OPTION depending if configure found them, using $(if)
>> -# We have to define a COMMA here, otherwise the commas in the "true"
>> -# result are treated as separators by the $(if).
>> -COMMA := ,
>> +# Darwin does not have a version script option. Exported symbols are
>> contr

Re: [PATCH] testsuite, jit: Stabilize error output.

2024-01-22 Thread Iain Sandoe
gentle ping,
with the increasing use of CI, it seems an idea to tackle this sooner rather 
than later.
thanks
Iain

> On 16 Jan 2024, at 11:12, Iain Sandoe  wrote:
> 
> Tested on x86_64, i686 Darwin, x86_64 Linux,
> OK for trunk? When?
> thanks
> Iain
> 
> --- 8< ---
> 
> Currently when a test fails, we print out a lot of information,
> this includes items that are not stable between invocations (e.g.
> the PID for the executable).  That makes automated comparisons
> between test runs flag any persistent fails as new ones each time
> which is not usually what is wanted.
> 
> This patch amends the error output to drop the variable portion
> of the message and retain items that should only change if the
> failure mode changes.
> 
> gcc/testsuite/ChangeLog:
> 
>   * jit.dg/jit.exp: Filter error output to remove per-run
>   variable content.
> 
> Signed-off-by: Iain Sandoe 
> ---
> gcc/testsuite/jit.dg/jit.exp | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
> index 286cfa8192a..893ff5f6dd0 100644
> --- a/gcc/testsuite/jit.dg/jit.exp
> +++ b/gcc/testsuite/jit.dg/jit.exp
> @@ -94,25 +94,34 @@ proc parse_valgrind_logfile {name logfile} {
> # unexpected exits.
> 
> proc verify_exit_status { executable wres } {
> -lassign $wres pid spawnid os_error_flag value
> +set extra [lassign $wres pid spawnid os_error_flag value]
> verbose "pid: $pid" 3
> verbose "spawnid: $spawnid" 3
> verbose "os_error_flag: $os_error_flag" 3
> verbose "value: $value" 3
> 
> # Detect segfaults etc:
> -if { [llength $wres] > 4 } {
> - if { [lindex $wres 4] == "CHILDKILLED" } {
> - fail "$executable killed: $wres"
> +set len [llength $extra]
> +if { $len >= 1 } {
> + if { [lindex $extra 0] == "CHILDKILLED" } {
> + set reason "Unknown Reason"
> + set detail "No Details"
> + if { $len >= 2 } {
> + set reason [lindex $extra 1]
> + if { $len >= 3 } {
> + set detail [lindex $extra 2]
> + }
> + }
> + fail "$executable killed: $reason $detail"
>   return
>   }
> }
> if { $os_error_flag != 0 } {
> - fail "$executable: OS error: $wres"
> + fail "$executable: OS error: $os_error_flag $extra"
>   return
> }
> if { $value != 0 } {
> - fail "$executable: non-zero exit code: $wres"
> + fail "$executable: non-zero exit code: $value $extra"
>   return
> }
> pass "$executable exited cleanly"
> -- 
> 2.39.2 (Apple Git-143)
> 



Re: [PATCH] jit, Darwin: Implement library exports list.

2024-01-22 Thread Iain Sandoe
gentle ping,
this fixes quite a few of the new jit fails on darwin.
thanks
Iain

> On 16 Jan 2024, at 11:10, Iain Sandoe  wrote:
> 
> Tested on x86_64, i686 Darwin and x86_64 Linux,
> OK for trunk? when ?
> thanks,
> Iain
> 
> --- 8< ---
> 
> Currently, we have no exports list for libgccjit, which means that
> all symbols are exported, including those from libstdc++ which is
> linked statically into the lib.  This causes failures when the
> shared libstdc++ is used but some c++ symbols are satisfied from
> libgccjit.
> 
> This implements an export file for Darwin (which is currently
> manually created by cross-checking libgccjit.map).  Ideally we'd
> script this, at some point.  Update libtool current and age to
> reflect the current ABI version (we are not bumping the SO name
> at this stage).
> 
> This fixes a number of new failures in jit testing.
> 
> gcc/jit/ChangeLog:
> 
>   * Make-lang.in: Implement exports list, and use a shared
>   libgcc.
>   * libgccjit.exp: New file.
> 
> Signed-off-by: Iain Sandoe 
> ---
> gcc/jit/Make-lang.in  |  38 ---
> gcc/jit/libgccjit.exp | 229 ++
> 2 files changed, 251 insertions(+), 16 deletions(-)
> create mode 100644 gcc/jit/libgccjit.exp
> 
> diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in
> index b1f0ce73e12..52dc2c24908 100644
> --- a/gcc/jit/Make-lang.in
> +++ b/gcc/jit/Make-lang.in
> @@ -55,7 +55,10 @@ else
> 
> ifneq (,$(findstring darwin,$(host)))
> 
> -LIBGCCJIT_AGE = 1
> +LIBGCCJIT_CURRENT = 26
> +LIBGCCJIT_REVISION = 0
> +LIBGCCJIT_AGE = 26
> +LIBGCCJIT_COMPAT = 0
> LIBGCCJIT_BASENAME = libgccjit
> 
> LIBGCCJIT_SONAME = \
> @@ -63,15 +66,15 @@ LIBGCCJIT_SONAME = \
> LIBGCCJIT_FILENAME = $(LIBGCCJIT_BASENAME).$(LIBGCCJIT_VERSION_NUM).dylib
> LIBGCCJIT_LINKER_NAME = $(LIBGCCJIT_BASENAME).dylib
> 
> -# Conditionalize the use of the LD_VERSION_SCRIPT_OPTION and
> -# LD_SONAME_OPTION depending if configure found them, using $(if)
> -# We have to define a COMMA here, otherwise the commas in the "true"
> -# result are treated as separators by the $(if).
> -COMMA := ,
> +# Darwin does not have a version script option. Exported symbols are 
> controlled
> +# by the following, and library versioning is done using libtool.
> LIBGCCJIT_VERSION_SCRIPT_OPTION = \
> - $(if $(LD_VERSION_SCRIPT_OPTION),\
> -   
> -Wl$(COMMA)$(LD_VERSION_SCRIPT_OPTION)$(COMMA)$(srcdir)/jit/libgccjit.map)
> +  -Wl,-exported_symbols_list,$(srcdir)/jit/libgccjit.exp
> 
> +# Conditionalize the use of  LD_SONAME_OPTION on configure finding it, using
> +# $(if).  We have to define a COMMA here, otherwise the commas in the "true"
> +# result are treated as separators by the $(if).
> +COMMA := ,
> LIBGCCJIT_SONAME_OPTION = \
>   $(if $(LD_SONAME_OPTION), \
>-Wl$(COMMA)$(LD_SONAME_OPTION)$(COMMA)$(LIBGCCJIT_SONAME))
> @@ -143,15 +146,18 @@ ifneq (,$(findstring mingw,$(target)))
> # Create import library
> LIBGCCJIT_EXTRA_OPTS = -Wl,--out-implib,$(LIBGCCJIT_IMPORT_LIB)
> else
> -
> ifneq (,$(findstring darwin,$(host)))
> -# TODO : Construct a Darwin-style symbol export file.
> -LIBGCCJIT_EXTRA_OPTS = -Wl,-compatibility_version,$(LIBGCCJIT_VERSION_NUM) \
> - 
> -Wl,-current_version,$(LIBGCCJIT_VERSION_NUM).$(LIBGCCJIT_MINOR_NUM).$(LIBGCCJIT_AGE)
>  \
> - $(LIBGCCJIT_VERSION_SCRIPT_OPTION) \
> - $(LIBGCCJIT_SONAME_OPTION)
> +LIBGCCJIT_VERS = $(LIBGCCJIT_CURRENT).$(LIBGCCJIT_REVISION).$(LIBGCCJIT_AGE)
> +LIBGCCJIT_EXTRA_OPTS = -Wl,-current_version,$(LIBGCCJIT_VERS) \
> + -Wl,-compatibility_version,$(LIBGCCJIT_COMPAT) \
> +  $(LIBGCCJIT_VERSION_SCRIPT_OPTION) $(LIBGCCJIT_SONAME_OPTION)
> +# Use the default (shared) libgcc.
> +JIT_LDFLAGS = $(filter-out -static-libgcc, $(LDFLAGS))
> +ifeq (,$(findstring darwin8,$(host)))
> +JIT_LDFLAGS += -Wl,-rpath,@loader_path
> +endif
> else
> -
> +JIT_LDFLAGS = $(LDFLAGS)
> LIBGCCJIT_EXTRA_OPTS = $(LIBGCCJIT_VERSION_SCRIPT_OPTION) \
>   $(LIBGCCJIT_SONAME_OPTION)
> endif
> @@ -170,7 +176,7 @@ $(LIBGCCJIT_FILENAME): $(jit_OBJS) \
>   $(LIBDEPS) $(srcdir)/jit/libgccjit.map \
>   $(EXTRA_GCC_OBJS_EXCLUSIVE) $(jit.prev)
>   @$(call LINK_PROGRESS,$(INDEX.jit),start)
> - +$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ -shared \
> + +$(LLINKER) $(ALL_LINKERFLAGS) $(JIT_LDFLAGS) -o $@ -shared \
>$(jit_OBJS) libbackend.a libcommon-target.a libcommon.a \
>$(CPPLIB) $(LIBDECNUMBER) $(EXTRA_GCC_LIBS) $(LIBS) $(BACKENDLIBS) 
> \
>$(EXTRA_GCC_OBJS_EXCLUSIVE) \
> diff --git a/gcc/jit/libgccjit.exp b/gcc/jit/libgccjit.exp
> new file mode 100644
> ind

[pushed] Darwin, configure: Handle a missing substitution.

2024-01-18 Thread Iain Sandoe
Tested on x86_64 Darwin21 (has default rpath) and i686 darwin9 and
x86_64 Linux (no @rpath), pushed to trunk, thanks,
Iain

--- 8< ---

The configure substitution for enable_darwin_at_rpath has been
omitted, which leads to a failure to set ENABLE_DARWIN_AT_RPATH in
the testsuite site.exp (which leads to failure to add -B options
in some cases, breaking uninstalled testing there).

Since we already have substitutions for ENABLE_DARWIN_AT_RPATH_TRUE
we can use that instead, which is what this patch does.

gcc/ChangeLog:

* Makefile.in: Emit ENABLE_DARWIN_AT_RPATH into site.exp
when ENABLE_DARWIN_AT_RPATH_TRUE is not '#'.

Signed-off-by: Iain Sandoe 
---
 gcc/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index deb12e17d25..95caa54a52b 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -4303,7 +4303,7 @@ site.exp: ./config.status Makefile
  echo "set COMPAT_OPTIONS \"$(COMPAT_OPTIONS)\"" >> ./site.tmp; \
else true; \
fi
-   @if test "x@enable_darwin_at_rpath@" = "xyes" ; then \
+   @if test "X@ENABLE_DARWIN_AT_RPATH_TRUE@" != "X#" ; then \
  echo "set ENABLE_DARWIN_AT_RPATH 1" >> ./site.tmp; \
fi
@echo "## All variables above are generated by configure. Do Not Edit 
##" >> ./site.tmp
-- 
2.39.2 (Apple Git-143)



[PATCH v3] Fix __builtin_nested_func_ptr_{created, deleted} symbol versions [PR113402]

2024-01-18 Thread Iain Sandoe
In order to regularise the two new builtins as extension library types
the scope of this patch has grown w.r.t "just rename".

Tested on x86_64-darwin21 (default heap trampolines) and x86_64 Linux and
other Darwin platforms that are default executable stack.

How does this look now?
thanks
Iain

--- 8< ---

The symbols for the functions supporting heap-based trampolines were
exported at an incorrect symbol version, the following patch fixes that.

As requested in the PR, this also renames __builtin_nested_func_ptr* to
__gcc_nested_func_ptr*.  In carrying our the rename, we move the builtins
to use DEF_EXT_LIB_BUILTIN.

PR libgcc/113402

gcc/ChangeLog:

* builtins.cc (expand_builtin): Handle BUILT_IN_NESTED_PTR_CREATED
and BUILT_IN_NESTED_PTR_DELETED.
* builtins.def (BUILT_IN_NESTED_PTR_CREATED,
BUILT_IN_NESTED_PTR_DELETED): Make these builtins LIB-EXT and
rename the library fallbacks to __gcc_nested_func_ptr_created and
__gcc_nested_func_ptr_deleted.
* doc/invoke.texi: Rename these to __gcc_nested_func_ptr_created
and __gcc_nested_func_ptr_deleted.
* tree-nested.cc (finalize_nesting_tree_1): Use builtin_explicit for
BUILT_IN_NESTED_PTR_CREATED and BUILT_IN_NESTED_PTR_DELETED.
* tree.cc (build_common_builtin_nodes): Build the
BUILT_IN_NESTED_PTR_CREATED and BUILT_IN_NESTED_PTR_DELETED local
builtins only for non-explicit.

libgcc/ChangeLog:

* config/aarch64/heap-trampoline.c: Rename
__builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
__builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
* config/i386/heap-trampoline.c: Likewise.
* libgcc2.h: Likewise.
* libgcc-std.ver.in (GCC_7.0.0): Likewise and then move
__gcc_nested_func_ptr_created and
__gcc_nested_func_ptr_deleted from this symbol version to ...
(GCC_14.0.0): ... this one.

Signed-off-by: Iain Sandoe 
Co-authored-by: Jakub Jelinek  
---
 gcc/builtins.cc |  5 
 gcc/builtins.def|  4 ++--
 gcc/doc/invoke.texi |  4 ++--
 gcc/tree-nested.cc  |  4 ++--
 gcc/tree.cc | 31 ++---
 libgcc/config/aarch64/heap-trampoline.c |  8 +++
 libgcc/config/i386/heap-trampoline.c|  8 +++
 libgcc/libgcc-std.ver.in|  5 ++--
 libgcc/libgcc2.h|  4 ++--
 9 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index 09f2354f114..cebd88142b0 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -8416,6 +8416,11 @@ expand_builtin (tree exp, rtx target, rtx subtarget, 
machine_mode mode,
 case BUILT_IN_ADJUST_DESCRIPTOR:
   return expand_builtin_adjust_descriptor (exp);
 
+case BUILT_IN_NESTED_PTR_CREATED:
+case BUILT_IN_NESTED_PTR_DELETED:
+  break; /* At present, no expansion, just call the function.  */
+
+
 case BUILT_IN_FORK:
 case BUILT_IN_EXECL:
 case BUILT_IN_EXECV:
diff --git a/gcc/builtins.def b/gcc/builtins.def
index 4d97ca0eec9..fd040eb8d80 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, 
"__builtin_adjust_trampoline")
 DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
 DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
 DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
-DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, 
"__builtin_nested_func_ptr_created")
-DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, 
"__builtin_nested_func_ptr_deleted")
+DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_CREATED, 
"__gcc_nested_func_ptr_created", BT_FN_VOID_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
+DEF_EXT_LIB_BUILTIN (BUILT_IN_NESTED_PTR_DELETED, 
"__gcc_nested_func_ptr_deleted", BT_FN_VOID, ATTR_NOTHROW_LIST)
 
 /* Implementing __builtin_setjmp.  */
 DEF_BUILTIN_STUB (BUILT_IN_SETJMP_SETUP, "__builtin_setjmp_setup")
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4d43dda9839..7a5ba9e7fb5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -19457,8 +19457,8 @@ for nested functions.
 By default, trampolines are generated on stack.  However, certain platforms
 (such as the Apple M1) do not permit an executable stack.  Compiling with
 @option{-ftrampoline-impl=heap} generate calls to
-@code{__builtin_nested_func_ptr_created} and
-@code{__builtin_nested_func_ptr_deleted} in order to allocate and
+@code{__gcc_nested_func_ptr_created} and
+@code{__gcc_nested_func_ptr_deleted} in order to allocate and
 deallocate trampoline space on the executable heap.  These functions are
 implemented in libgcc, and will only be provided on specific targets:
 x86_64 Darwin, x86_64 

[pushed] Objective-C/C++: Ensure sufficient setup for the preprocessor.

2024-01-18 Thread Iain Sandoe
This is a regression fix where non-trivial Objective-C parses would
ICE when given -save-temps (ICE in the lexer).

This is a short-term fix for stage-4.  ISTM that we should not really
be making use of these functions in lexing and hopefully in GCC-15
we can take a look at moving the functionality to a later phase.

Tested on i686, powerpc, x86_64 Darwin, x86_64 Linux, pushed to trunk,
thanks
Iain

--- 8< ---

The tokenizer makes use of functions that determine if identifiers
are interface or class names, and those functions need a hash map
to be set up.

This ensures that these are initialized before pre-process-only
jobs are run.

gcc/objc/ChangeLog:

* objc-act.cc (objc_init): Initialize interface and class
name hash maps before the preprocessor uses them.

Signed-off-by: Iain Sandoe 
---
 gcc/objc/objc-act.cc | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/objc/objc-act.cc b/gcc/objc/objc-act.cc
index 143134832ff..cec64c4bfbd 100644
--- a/gcc/objc/objc-act.cc
+++ b/gcc/objc/objc-act.cc
@@ -345,6 +345,11 @@ bool
 objc_init (void)
 {
   bool ok;
+
+  /* Set up stuff used by the preprocessor as well as FE parser.  */
+  interface_hash_init ();
+  hash_init ();
+
 #ifdef OBJCPLUS
   if (cxx_init () == false)
 #else
@@ -374,8 +379,6 @@ objc_init (void)
 
   /* Set up stuff used by FE parser and all runtimes.  */
   errbuf = XNEWVEC (char, 1024 * 10);
-  interface_hash_init ();
-  hash_init ();
   objc_encoding_init ();
   /* ... and then check flags and set-up for the selected runtime ... */
   if (flag_next_runtime && flag_objc_abi >= 2)
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Suppress adding embedded rpaths for earlier OS versions.

2024-01-18 Thread Iain Sandoe
The current setup leads to spurious test fails, where we are building for
macOS 10.4 or earlier.

Tested on x86_64, i868, powerpc Darwin, x86_64 Linux, pushed to trunk,
thanks
Iain

--- 8< ---

When we have @rpath support by virtue of the OS version we're hosting on
we still need to omit those rpath entries when targeting < 10.5 (or the
linker will complain).  To do this we (maybe ab-)use a property of the
spec function expansion that a non-null return value can be used as the
true input to a second spec (whereas, unfortunately, we cannot pass specs
to the version function at present).

gcc/ChangeLog:

* config/darwin.h (DARWIN_RPATH_SPEC): Arrange for the %P spec
to be conditional on macosx-version-min.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index e94a29c639c..cb96d67b3b1 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -612,8 +612,7 @@ extern GTY(()) int darwin_ms_struct;
director as one being loaded.  */
 #define DARWIN_RPATH_SPEC \
   "%:version-compare(>= 10.5 mmacosx-version-min= -rpath) \
-   %:version-compare(>= 10.5 mmacosx-version-min= @loader_path) \
-   %P "
+   %{%:version-compare(>= 10.5 mmacosx-version-min= @loader_path): %P }"
 #else
 #define DARWIN_RPATH_SPEC ""
 #endif
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Fix a typo in Objective-C meta-data.

2024-01-18 Thread Iain Sandoe
Tested on i686, powerpc, x86_64 Darwin, x86_64 Linux, pushed to trunk,
thanks,
Iain

--- 8< ---

We have a typo in the metadata for assigning NSStrings to a specific
section for the V1 (32b) ABI.  When that is fixed we should never see
the case where the section needs to be deduced from the properties of
the DECLs.

gcc/ChangeLog:

* config/darwin.cc (darwin_objc1_section): Use the correct
meta-data version for constant strings.
(machopic_select_section): Assert if we fail to handle CFString
sections as Obejctive-C meta-data or drectly.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.cc | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index b15f3b1a1d9..7f43718820b 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -1638,7 +1638,7 @@ darwin_objc1_section (tree decl ATTRIBUTE_UNUSED, tree 
meta, section * base)
   else if (startswith (p, "V1_CEXT"))
 return darwin_sections[objc1_class_ext_section];
 
-  else if (startswith (p, "V2_CSTR"))
+  else if (startswith (p, "V1_CSTR"))
 return darwin_sections[objc_constant_string_object_section];
 
   return base;
@@ -1782,7 +1782,7 @@ machopic_select_section (tree decl,
return base_section; /* GNU runtime is happy with it all in one pot.  */
 }
 
-  /* b) Constant string objects.  */
+  /* b) Constructors for constant NSstring [but not CFString] objects.  */
   if (TREE_CODE (decl) == CONSTRUCTOR
   && TREE_TYPE (decl)
   && TREE_CODE (TREE_TYPE (decl)) == RECORD_TYPE
@@ -1804,6 +1804,12 @@ machopic_select_section (tree decl,
  else
return darwin_sections[objc_string_object_section];
}
+  else if (!strcmp (IDENTIFIER_POINTER (name), "__builtin_CFString"))
+   {
+ /* We should have handled __anon_cfstrings above.  */
+ gcc_checking_assert (0);
+ return darwin_sections[cfstring_constant_object_section];
+   }
   else
return base_section;
 }
-- 
2.39.2 (Apple Git-143)



[pushed] Darwin: Fix constant CFString code-gen [PR105522].

2024-01-18 Thread Iain Sandoe
@Richi, @Andrew - FIO since you were involved in the IRC discussion.

Tested on i686, powerpc, x86_64 Darwin (and x86_64 Linux), pushed
to trunk, thanks,
Iain

--- 8< ---

Although this only fires for one of the Darwin sub-ports, it is latent
elsewhere, it is also a regression c.f. the Darwin system compiler.

In the code we imported from an earlier branch, CFString objects (which
are constant aggregates) are constructed as CONST_DECLs.  Although our
current documentation suggests that these are reserved for enumeration
values, in fact they are used elsewhere in the compiler for constants.
This includes Objective-C where they are used to form NSString constants.

In the particular case, we take the address of the constant and that
triggers varasm.cc:decode_addr_constant, which does not currently support
CONST_DECL.

If there is a general intent to allow/encourage wider use of CONST_DECL,
then we should fix decode_addr_constant to look through these and evaluate
the initializer (a two-line patch, but I'm not suggesting it for stage-4).

We also need to update the GCC internals documentation to allow for the
additional uses.

This patch is Darwin-local and fixes the problem by making the CFString
constants into regular variable but TREE_CONSTANT+TREE_READONLY. I plan
to back-port this to the open branches once it has baked a while on trunk.

Since, for Darwin, the Objective-C default is to construct constant
NSString objects as CFStrings; this will also cover the majority of cases
there (this patch does not make any changes to Objective-C NSStrings).

PR target/105522

gcc/ChangeLog:

* config/darwin.cc (machopic_select_section): Handle C and C++
CFStrings.
(darwin_rename_builtins): Move this out of the CFString code.
(darwin_libc_has_function): Likewise.
(darwin_build_constant_cfstring): Create an anonymous var to
hold each CFString.
* config/darwin.h (ASM_OUTPUT_LABELREF): Handle constant
CFstrings.

Signed-off-by: Iain Sandoe 
---
 gcc/config/darwin.cc| 100 ++--
 gcc/config/darwin.h |   2 +
 gcc/testsuite/gcc.dg/pr105522.c |  17 ++
 3 files changed, 76 insertions(+), 43 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr105522.c

diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index cf203dc4b3e..b15f3b1a1d9 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -1731,7 +1731,16 @@ machopic_select_section (tree decl,
base_section = darwin_sections[zobj_data_section];
}
   else if (ro)
-   base_section = darwin_sections[const_data_section];
+   {
+ if (VAR_P (decl) && TREE_TYPE (decl)
+ && TREE_CODE (TREE_TYPE (decl)) == RECORD_TYPE
+ && DECL_NAME (decl)
+ && strncmp (IDENTIFIER_POINTER (DECL_NAME (decl)),
+ "__anon_cfstring", 15) == 0)
+  base_section = darwin_sections[cfstring_constant_object_section];
+ else
+   base_section = darwin_sections[const_data_section];
+   }
   else
base_section = data_section;
   break;
@@ -1795,8 +1804,6 @@ machopic_select_section (tree decl,
  else
return darwin_sections[objc_string_object_section];
}
-  else if (!strcmp (IDENTIFIER_POINTER (name), "__builtin_CFString"))
-   return darwin_sections[cfstring_constant_object_section];
   else
return base_section;
 }
@@ -3612,6 +3619,29 @@ darwin_patch_builtins (void)
 }
 #endif
 
+void
+darwin_rename_builtins (void)
+{
+}
+
+/* Implementation for the TARGET_LIBC_HAS_FUNCTION hook.  */
+
+bool
+darwin_libc_has_function (enum function_class fn_class,
+ tree type ATTRIBUTE_UNUSED)
+{
+  if (fn_class == function_sincos && darwin_macosx_version_min)
+return (strverscmp (darwin_macosx_version_min, "10.9") >= 0);
+#if DARWIN_PPC && SUPPORT_DARWIN_LEGACY
+  if (fn_class == function_c99_math_complex
+  || fn_class == function_c99_misc)
+return (TARGET_64BIT
+   || (darwin_macosx_version_min &&
+   strverscmp (darwin_macosx_version_min, "10.3") >= 0));
+#endif
+  return default_libc_has_function (fn_class, type);
+}
+
 /*  CFStrings implementation.  */
 static GTY(()) tree cfstring_class_reference = NULL_TREE;
 static GTY(()) tree cfstring_type_node = NULL_TREE;
@@ -3629,7 +3659,7 @@ typedef struct GTY ((for_user)) cfstring_descriptor {
   /* The string literal.  */
   tree literal;
   /* The resulting constant CFString.  */
-  tree constructor;
+  tree ccf_str;
 } cfstring_descriptor;
 
 struct cfstring_hasher : ggc_ptr_hash
@@ -3704,7 +3734,7 @@ darwin_init_cfstring_builtins (unsigned builtin_cfstring)
   /* Make a lang-specific section - dup_lang_specific_decl makes a new node
  in place of the existing, whic

Re: [COMMITTED] rust_debug: Cast size_t values to unsigned long before printing.

2024-01-18 Thread Iain Sandoe
Hi Arthur,

> On 18 Jan 2024, at 10:30, Arthur Cohen  wrote:

> On 1/18/24 10:13, Rainer Orth wrote:
>> Arthur Cohen  writes:
>>> Using %lu to format size_t values breaks 32 bit targets, and %zu is not
>>> supported by one of the hosts GCC aims to support - HPUX
>> But we do have uses of %zu in gcc/rust already!
>>> diff --git a/gcc/rust/expand/rust-proc-macro.cc 
>>> b/gcc/rust/expand/rust-proc-macro.cc
>>> index e8618485b71..09680733e98 100644
>>> --- a/gcc/rust/expand/rust-proc-macro.cc
>>> +++ b/gcc/rust/expand/rust-proc-macro.cc
>>> @@ -171,7 +171,7 @@ load_macros (std::string path)
>>>if (array == nullptr)
>>>  return {};
>>>  -  rust_debug ("Found %lu procedural macros", array->length);
>>> +  rust_debug ("Found %lu procedural macros", (unsigned long) 
>>> array->length);
>> Not the best way either: array->length is std::uint64_t, so the format
>> should use
>> ... %" PRIu64 " procedural...
>> instead.
>> I've attached my patch to PR rust/113461.
> 
> Yes, I was talking about this on IRC the other day - if we do run in a 
> situation where we have more than UINT32_MAX procedural macros in memory we 
> have big issues. These debug prints will probably end up getting removed soon 
> as they clutter the output a lot for little information.
> 
> I don't mind doing it the right way for our regular prints, but we have not 
> been using PRIu64 in our codebase so far, so I'd rather change all those 
> incriminating format specifiers at once later down the line - this patch was 
> pushed so that 32bit targets could bootstrap the Rust frontend for now.

For the sake of completeness, the issue does not just affect 32b hosts;  If a 
64b host chooses (as Darwin does, so that 32b and 64b targets have the same 
representation) to make uint64_t “unsigned long long int”, then %lu breaks 
there too.
thanks
Iain



Re: [PATCH v2] Fix __builtin_nested_func_ptr_{created, deleted} symbol versions [PR113402]

2024-01-17 Thread Iain Sandoe



> On 17 Jan 2024, at 08:55, Iain Sandoe  wrote:
> 
> Tested on x86_64, aarch64 Darwin21 (which default to heap-based trampolines)
> and on x86_64 Darwin19 and Linux (which default to executable stack
> trampolines).
> OK for trunk?

Hmm.. maybe this is not right and the builtins should still be named __builtin 
(with
the fallback function only renamed) or alternatively, add these as libfuncs 
only?

> Iain
> 
> --- 8< ---
> 
> The symbols for the functions supporting heap-based trampolines were
> exported at an incorrect symbol version, the following patch fixes that.
> 
> As requested in the PR, this also renames __builtin_nested_func_ptr* to
> __gcc_nested_func_ptr*.
> 
>   PR libgcc/113402
> 
> gcc/ChangeLog:
> 
>   * builtins.def
>   (BUILT_IN_NESTED_PTR_CREATED): Rename __builtin_nested_func_ptr_created
>   to __gcc_nested_func_ptr_created.
>   (BUILT_IN_NESTED_PTR_DELETED): Rename __builtin_nested_func_ptr_deleted
>   to __gcc_nested_func_ptr_deleted.
>   * doc/invoke.texi: Likewise.
>   * tree.cc (build_common_builtin_nodes): Likewise.
> 
> libgcc/ChangeLog:
> 
>   * config/aarch64/heap-trampoline.c: Rename
>   __builtin_nested_func_ptr_created to __gcc_nested_func_ptr_created and
>   __builtin_nested_func_ptr_deleted to __gcc_nested_func_ptr_deleted.
>   * config/i386/heap-trampoline.c: Likewise.
>   * libgcc2.h: Likewise.
>   * libgcc-std.ver.in (GCC_7.0.0): Likewise and then move
>   __gcc_nested_func_ptr_created and
>   __gcc_nested_func_ptr_deleted from this symbol version to ...
>   (GCC_14.0.0): ... this one.
> 
> Signed-off-by: Iain Sandoe 
> Co-authored-by: Jakub Jelinek  
> ---
> gcc/builtins.def| 4 ++--
> gcc/doc/invoke.texi | 4 ++--
> gcc/tree.cc | 8 
> libgcc/config/aarch64/heap-trampoline.c | 8 
> libgcc/config/i386/heap-trampoline.c| 8 
> libgcc/libgcc-std.ver.in| 5 ++---
> libgcc/libgcc2.h| 4 ++--
> 7 files changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/gcc/builtins.def b/gcc/builtins.def
> index 4d97ca0eec9..e8a88ee8bf7 100644
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -1084,8 +1084,8 @@ DEF_BUILTIN_STUB (BUILT_IN_ADJUST_TRAMPOLINE, 
> "__builtin_adjust_trampoline")
> DEF_BUILTIN_STUB (BUILT_IN_INIT_DESCRIPTOR, "__builtin_init_descriptor")
> DEF_BUILTIN_STUB (BUILT_IN_ADJUST_DESCRIPTOR, "__builtin_adjust_descriptor")
> DEF_BUILTIN_STUB (BUILT_IN_NONLOCAL_GOTO, "__builtin_nonlocal_goto")
> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, 
> "__builtin_nested_func_ptr_created")
> -DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, 
> "__builtin_nested_func_ptr_deleted")
> +DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_CREATED, 
> "__gcc_nested_func_ptr_created")
> +DEF_BUILTIN_STUB (BUILT_IN_NESTED_PTR_DELETED, 
> "__gcc_nested_func_ptr_deleted")
> 
> /* Implementing __builtin_setjmp.  */
> DEF_BUILTIN_STUB (BUILT_IN_SETJMP_SETUP, "__builtin_setjmp_setup")
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 16e31a3c6db..9727f1de71d 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -19450,8 +19450,8 @@ for nested functions.
> By default, trampolines are generated on stack.  However, certain platforms
> (such as the Apple M1) do not permit an executable stack.  Compiling with
> @option{-ftrampoline-impl=heap} generate calls to
> -@code{__builtin_nested_func_ptr_created} and
> -@code{__builtin_nested_func_ptr_deleted} in order to allocate and
> +@code{__gcc_nested_func_ptr_created} and
> +@code{__gcc_nested_func_ptr_deleted} in order to allocate and
> deallocate trampoline space on the executable heap.  These functions are
> implemented in libgcc, and will only be provided on specific targets:
> x86_64 Darwin, x86_64 and aarch64 Linux.  @emph{PLEASE NOTE}: Heap
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 8aee3ef18d8..6fa99ad7fe4 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -9934,15 +9934,15 @@ build_common_builtin_nodes (void)
>   ptr_type_node, // void *func
>   ptr_ptr_type_node, // void **dst
>   NULL_TREE);
> -  local_define_builtin ("__builtin_nested_func_ptr_created", ftype,
> +  local_define_builtin ("__gcc_nested_func_ptr_created", ftype,
>   BUILT_IN_NESTED_PTR_CREATED,
> - "__builtin_nested_func_ptr_created", ECF_NOTHROW);
> + 

  1   2   3   4   5   6   7   8   9   10   >