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