> On 14 May 2024, at 14:29, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Wed, May 8, 2024 at 9:37 PM Iain Sandoe <i...@sandoe.co.uk> 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 <iains....@gmail.com> 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 <i...@sandoe.co.uk>
>>> ---
>>> 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 <version> Specify that ld64 <version> 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 100644
>>> --- a/gcc/doc/invoke.texi
>>> +++ b/gcc/doc/invoke.texi
>>> @@ -954,7 +954,7 @@ Objective-C and Objective-C++ Dialects}.
>>> -twolevel_namespace -umbrella -undefined
>>> -unexported_symbols_list -weak_reference_mismatches
>>> -whatsloaded -F -gused -gfull -mmacosx-version-min=@var{version}
>>> --mkernel -mone-byte-bool}
>>> +-mkernel -mone-byte-bool -munreachable-traps}
>>>
>>> @emph{DEC Alpha Options}
>>> @gccoptlist{-mno-fp-regs -msoft-float
>>> @@ -25136,6 +25136,11 @@ without that switch. Using this switch may
>>> require recompiling all
>>> other modules in a program, including system libraries. Use this
>>> switch to conform to a non-default data model.
>>>
>>> +@opindex munreachable-traps
>>> +@item -munreachable-traps
>>> +Causes @code{__builtin_unreachable} to be rendered as a trap. This is the
>>> +default for all Darwin architectures.
>>> +
>>> @opindex mfix-and-continue
>>> @opindex ffix-and-continue
>>> @opindex findirect-data
>>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>>> index 551463d3a2d..ce1d393ec70 100644
>>> --- a/gcc/doc/tm.texi
>>> +++ b/gcc/doc/tm.texi
>>> @@ -12725,6 +12725,11 @@ This target hook can be used to generate a
>>> target-specific code
>>> If selftests are enabled, run any selftests for this target.
>>> @end deftypefn
>>>
>>> +@deftypefn {Target Hook} bool TARGET_UNREACHABLE_SHOULD_TRAP (void)
>>> +This hook should return @code{true} if the target wants
>>> @code{__builtin_unreachable} to expand to a trap or @code{abort ()}.
>>> + The default value is false.
>>> +@end deftypefn
>>> +
>>> @deftypefn {Target Hook} bool TARGET_MEMTAG_CAN_TAG_ADDRESSES ()
>>> True if the backend architecture naturally supports ignoring some region
>>> of pointers. This feature means that @option{-fsanitize=hwaddress} can
>>> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
>>> index a9ff86d4216..a2156ec99ff 100644
>>> --- a/gcc/doc/tm.texi.in
>>> +++ b/gcc/doc/tm.texi.in
>>> @@ -8103,6 +8103,8 @@ maintainer is familiar with.
>>>
>>> @hook TARGET_RUN_TARGET_SELFTESTS
>>>
>>> +@hook TARGET_UNREACHABLE_SHOULD_TRAP
>>> +
>>> @hook TARGET_MEMTAG_CAN_TAG_ADDRESSES
>>>
>>> @hook TARGET_MEMTAG_TAG_SIZE
>>> diff --git a/gcc/target.def b/gcc/target.def
>>> index 05722801c95..e2f7d107e49 100644
>>> --- a/gcc/target.def
>>> +++ b/gcc/target.def
>>> @@ -7409,6 +7409,16 @@ DEFHOOKPOD
>>> libatomic. The default value is false.",
>>> bool, false)
>>>
>>> +/* This value represents whether __builtin_unreachable should be expanded
>>> + as a trap instruction (or an abort() if the trap is not available). */
>>> +DEFHOOK
>>> +(unreachable_should_trap,
>>> + "This hook should return @code{true} if the target wants \
>>> + @code{__builtin_unreachable} to expand to a trap or @code{abort ()}.\n\
>>> + The default value is false.",
>>> + bool, (void),
>>> + hook_bool_void_false)
>>> +
>>> /* Close the 'struct gcc_target' definition. */
>>> HOOK_VECTOR_END (C90_EMPTY_HACK)
>>>
>>> --
>>> 2.39.2 (Apple Git-143)
>>>
>>
