Re: [PATCH] d: Fix #error You must define PREFERRED_DEBUGGING_TYPE if DWARF is not supported (PR105659)

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Aug 16, 2022 at 11:17 PM Iain Buclaw via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> Because targetdm contains hooks pertaining to both the target platform
> and cpu, it tries to pull in both platform and cpu headers via tm_d.h in
> the source file where TARGETDM_INITIALIZER is used.
>
> Since 12.0, this has caused problems when there is no platform (*-elf),
> resulting in default-d.cc failing to build due to triggering a
> PREFERRED_DEBUGGING_TYPE #error.
>
> This patch removes the CPU-specific hooks from targetdm, documenting
> them instead as target macros.  Also removing the generation of tm_d.h
> as its role is redundant.
>
> I also notice that Rust maintainers initially copied what I did in
> devel/rust/master, but ended up reverting back to using macros to get at
> target OS and CPU information as well, possibly because they ran into
> the same problems as reported in PR105659.
>
> I'm not sure whether calling these hooks via function-like macros is
> really desirable, I do recall early on during the review process of the
> D front-end that putting target-specific language features behind a
> targetdm hook was the preferred/encouraged way to expose these things.
>
> One alternative perhaps would be to break out CPU-specific hooks in
> targetdm into a separate targetdm_cpu hook vector.  This would mean
> there would be no need to include tm_p.h anywhere in D-specific target
> sources (only tm.h where needed), and all D-specific prototypes in
> $cpu_type-protos.h can be removed.  Though tm_d.h would still be
> redundant, so either way it gets the chop.
>
> OK? Thoughts?  I don't expect this to go in for 12.2, but backporting
> some time before 12.3 would be nice.

I was hoping Joseph would chime in here - I recollect debugging this kind
of thing and a thread about this a while back but unfortunately I do not
remember the details here (IIRC some things get included where they
better should not be).

This is all D specific changes so I'd say OK for trunk unless Joseph has
any comments.  Please wait a while before backporting so we can see
if it breaks any configuration.

Thanks,
Richard.

> Bootstrapped and regression tested on x86_64-linux-gnu, and checked that
> it indeed fixes the referenced PR by building an aarch64-rtems cross.
>
> Regards,
> Iain.
>
> ---
>         PR d/105659
>
> gcc/ChangeLog:
>
>         * Makefile.in (tm_d_file_list): Remove.
>         (tm_d_include_list): Remove.
>         (TM_D_H): Remove.
>         (tm_d.h): Remove.
>         (cs-tm_d.h): Remove.
>         (generated_files): Remove TM_D_H.
>         * config.gcc (tm_d_file): Remove.
>         * config/darwin-d.cc: Include memmodel.h and tm_p.h instead of tm_d.h.
>         * config/default-d.cc: Remove includes of memmodel.h and tm_d.h.
>         * config/dragonfly-d.cc: Include tm_p.h instead of tm_d.h.
>         * configure: Regenerate.
>         * configure.ac (tm_d_file): Remove.
>         (tm_d_file_list): Remove substitution.
>         (tm_d_include_list): Remove substitution.
>         * doc/tm.texi: Regenerate.
>         * doc/tm.texi.in (TARGET_D_CPU_VERSIONS): Document hook as being a
>         function-like macro.
>         (TARGET_D_REGISTER_CPU_TARGET_INFO): Likewise.
>
> gcc/d/ChangeLog:
>
>         * d-builtins.cc: Include memmodel.h and tm_p.h.
>         (d_init_versions): Call TARGET_D_CPU_VERSIONS via macro.
>         * d-target.cc (Target::_init): Call TARGET_D_REGISTER_CPU_TARGET_INFO
>         via macro.
>         * d-target.def (d_cpu_versions): Remove hook.
>         (d_register_cpu_target_info): Remove hook.
> ---
>  gcc/Makefile.in           | 11 +----------
>  gcc/config.gcc            |  7 -------
>  gcc/config/darwin-d.cc    |  3 ++-
>  gcc/config/default-d.cc   |  9 +++++++--
>  gcc/config/dragonfly-d.cc |  2 +-
>  gcc/configure             | 32 ++++++++------------------------
>  gcc/configure.ac          | 18 ------------------
>  gcc/d/d-builtins.cc       |  6 +++++-
>  gcc/d/d-target.cc         |  4 +++-
>  gcc/d/d-target.def        | 22 ----------------------
>  gcc/doc/tm.texi           | 22 ++++++++++++----------
>  gcc/doc/tm.texi.in        | 18 ++++++++++++++++--
>  12 files changed, 55 insertions(+), 99 deletions(-)
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 203f0a15187..12d9b5a3be4 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -571,8 +571,6 @@ tm_include_list=@tm_include_list@
>  tm_defines=@tm_defines@
>  tm_p_file_list=@tm_p_file_list@
>  tm_p_include_list=@tm_p_include_list@
> -tm_d_file_list=@tm_d_file_list@
> -tm_d_include_list=@tm_d_include_list@
>  build_xm_file_list=@build_xm_file_list@
>  build_xm_include_list=@build_xm_include_list@
>  build_xm_defines=@build_xm_defines@
> @@ -865,7 +863,6 @@ BCONFIG_H = bconfig.h $(build_xm_file_list)
>  CONFIG_H  = config.h  $(host_xm_file_list)
>  TCONFIG_H = tconfig.h $(xm_file_list)
>  TM_P_H    = tm_p.h    $(tm_p_file_list)
> -TM_D_H    = tm_d.h    $(tm_d_file_list)
>  GTM_H     = tm.h      $(tm_file_list) insn-constants.h
>  TM_H      = $(GTM_H) insn-flags.h $(OPTIONS_H)
>
> @@ -1937,7 +1934,6 @@ bconfig.h: cs-bconfig.h ; @true
>  tconfig.h: cs-tconfig.h ; @true
>  tm.h: cs-tm.h ; @true
>  tm_p.h: cs-tm_p.h ; @true
> -tm_d.h: cs-tm_d.h ; @true
>
>  cs-config.h: Makefile
>         TARGET_CPU_DEFAULT="" \
> @@ -1964,11 +1960,6 @@ cs-tm_p.h: Makefile
>         HEADERS="$(tm_p_include_list)" DEFINES="" \
>         $(SHELL) $(srcdir)/mkconfig.sh tm_p.h
>
> -cs-tm_d.h: Makefile
> -       TARGET_CPU_DEFAULT="" \
> -       HEADERS="$(tm_d_include_list)" DEFINES="" \
> -       $(SHELL) $(srcdir)/mkconfig.sh tm_d.h
> -
>  # Don't automatically run autoconf, since configure.ac might be accidentally
>  # newer than configure.  Also, this writes into the source directory which
>  # might be on a read-only file system.  If configured for maintainer mode
> @@ -2783,7 +2774,7 @@ s-gtype: $(EXTRA_GTYPE_DEPS) build/gengtype$(build_exeext) \
>                      -r gtype.state
>         $(STAMP) s-gtype
>
> -generated_files = config.h tm.h $(TM_P_H) $(TM_D_H) $(TM_H) multilib.h \
> +generated_files = config.h tm.h $(TM_P_H) $(TM_H) multilib.h \
>         $(simple_generated_h) specs.h \
>         tree-check.h genrtl.h insn-modes.h insn-modes-inline.h \
>         tm-preds.h tm-constrs.h \
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 4e3b15bb5e9..927d50b4355 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -87,9 +87,6 @@
>  #  tm_p_file           Location of file with declarations for functions
>  #                      in $out_file.
>  #
> -#  tm_d_file           A list of headers with definitions of target hook
> -#                      macros for the D compiler.
> -#
>  #  out_file            The name of the machine description C support
>  #                      file, if different from "$cpu_type/$cpu_type.c".
>  #
> @@ -559,11 +556,9 @@ xtensa*-*-*)
>  esac
>
>  tm_file=${cpu_type}/${cpu_type}.h
> -tm_d_file=${cpu_type}/${cpu_type}.h
>  if test -f ${srcdir}/config/${cpu_type}/${cpu_type}-protos.h
>  then
>         tm_p_file=${cpu_type}/${cpu_type}-protos.h
> -       tm_d_file="${tm_d_file} ${cpu_type}/${cpu_type}-protos.h"
>  fi
>
>  extra_modes=
> @@ -669,7 +664,6 @@ case ${target} in
>  *-*-darwin*)
>    tmake_file="t-darwin "
>    tm_file="${tm_file} darwin.h"
> -  tm_d_file="${tm_d_file} tm-dwarf2.h"
>    darwin_os=`echo ${target} | sed 's/.*darwin\([0-9.]*\).*$/\1/'`
>    darwin_maj=`expr "$darwin_os" : '\([0-9]*\).*'`
>    macos_min=`expr "$darwin_os" : '[0-9]*\.\([0-9]*\).*'`
> @@ -3524,7 +3518,6 @@ xstormy16-*-elf)
>         # For historical reasons, the target files omit the 'x'.
>         tm_file="dbxelf.h elfos.h newlib-stdint.h stormy16/stormy16.h"
>         tm_p_file=stormy16/stormy16-protos.h
> -       tm_d_file="elfos.h stormy16/stormy16.h"
>         md_file=stormy16/stormy16.md
>         out_file=stormy16/stormy16.cc
>         extra_options=stormy16/stormy16.opt
> diff --git a/gcc/config/darwin-d.cc b/gcc/config/darwin-d.cc
> index e983883dba6..e88ddc189c3 100644
> --- a/gcc/config/darwin-d.cc
> +++ b/gcc/config/darwin-d.cc
> @@ -18,9 +18,10 @@ along with GCC; see the file COPYING3.  If not see
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> -#include "tm_d.h"
>  #include "d/d-target.h"
>  #include "d/d-target-def.h"
> +#include "memmodel.h"
> +#include "tm_p.h"
>
>  /* Implement TARGET_D_OS_VERSIONS for Darwin targets.  */
>
> diff --git a/gcc/config/default-d.cc b/gcc/config/default-d.cc
> index 2d7abfcba96..6674373793c 100644
> --- a/gcc/config/default-d.cc
> +++ b/gcc/config/default-d.cc
> @@ -18,9 +18,14 @@ along with GCC; see the file COPYING3.  If not see
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> -#include "memmodel.h"
> -#include "tm_d.h"
>  #include "d/d-target.h"
>  #include "d/d-target-def.h"
>
> +/* Do not include tm.h or tm_p.h here; if it is useful for a target to
> +   define some macros for the initializer in a header without defining
> +   targetcm itself (for example, because of interactions with some
> +   hooks depending on the target OS and others on the target
> +   architecture), create a separate tm_d.h for only the relevant
> +   definitions.  */
> +
>  struct gcc_targetdm targetdm = TARGETDM_INITIALIZER;
> diff --git a/gcc/config/dragonfly-d.cc b/gcc/config/dragonfly-d.cc
> index d431638f7da..2ee35c42e99 100644
> --- a/gcc/config/dragonfly-d.cc
> +++ b/gcc/config/dragonfly-d.cc
> @@ -18,9 +18,9 @@ along with GCC; see the file COPYING3.  If not see
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> -#include "tm_d.h"
>  #include "d/d-target.h"
>  #include "d/d-target-def.h"
> +#include "tm_p.h"
>
>  /* Implement TARGET_D_OS_VERSIONS for DragonFly targets.  */
>
> diff --git a/gcc/configure b/gcc/configure
> index 05efa5b01ef..7d0445178bb 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -652,8 +652,6 @@ use_gcc_stdint
>  xm_defines
>  xm_include_list
>  xm_file_list
> -tm_d_include_list
> -tm_d_file_list
>  tm_p_include_list
>  tm_p_file_list
>  tm_defines
> @@ -5226,12 +5224,16 @@ else
>    GNATMAKE="$ac_cv_prog_GNATMAKE"
>  fi
>
> -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether compiler driver understands Ada" >&5
> -$as_echo_n "checking whether compiler driver understands Ada... " >&6; }
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether compiler driver understands Ada and is recent enough" >&5
> +$as_echo_n "checking whether compiler driver understands Ada and is recent enough... " >&6; }
>  if ${acx_cv_cc_gcc_supports_ada+:} false; then :
>    $as_echo_n "(cached) " >&6
>  else
>    cat >conftest.adb <<EOF
> +pragma Warnings (Off);
> +with System.CRTL;
> +pragma Warnings (On);
> +use type System.CRTL.int64;
>  procedure conftest is begin null; end conftest;
>  EOF
>  acx_cv_cc_gcc_supports_ada=no
> @@ -13009,7 +13011,6 @@ fi
>
>  tm_file="${tm_file} defaults.h"
>  tm_p_file="${tm_p_file} tm-preds.h"
> -tm_d_file="${tm_d_file} defaults.h"
>  host_xm_file="auto-host.h ansidecl.h ${host_xm_file}"
>  build_xm_file="${build_auto} ansidecl.h ${build_xm_file}"
>  # We don't want ansidecl.h in target files, write code there in ISO/GNU C.
> @@ -13403,21 +13404,6 @@ for f in $tm_p_file; do
>    esac
>  done
>
> -tm_d_file_list=
> -tm_d_include_list="options.h insn-constants.h"
> -for f in $tm_d_file; do
> -  case $f in
> -    defaults.h )
> -       tm_d_file_list="${tm_d_file_list} \$(srcdir)/$f"
> -       tm_d_include_list="${tm_d_include_list} $f"
> -       ;;
> -    * )
> -       tm_d_file_list="${tm_d_file_list} \$(srcdir)/config/$f"
> -       tm_d_include_list="${tm_d_include_list} config/$f"
> -       ;;
> -  esac
> -done
> -
>  xm_file_list=
>  xm_include_list=
>  for f in $xm_file; do
> @@ -19674,7 +19660,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19677 "configure"
> +#line 19663 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -19780,7 +19766,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19783 "configure"
> +#line 19769 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -31731,8 +31717,6 @@ fi
>
>
>
> -
> -
>
>
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index f70b6c24fda..a9006885663 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -2113,7 +2113,6 @@ AC_SUBST(HAVE_AUTO_BUILD)
>
>  tm_file="${tm_file} defaults.h"
>  tm_p_file="${tm_p_file} tm-preds.h"
> -tm_d_file="${tm_d_file} defaults.h"
>  host_xm_file="auto-host.h ansidecl.h ${host_xm_file}"
>  build_xm_file="${build_auto} ansidecl.h ${build_xm_file}"
>  # We don't want ansidecl.h in target files, write code there in ISO/GNU C.
> @@ -2356,21 +2355,6 @@ for f in $tm_p_file; do
>    esac
>  done
>
> -tm_d_file_list=
> -tm_d_include_list="options.h insn-constants.h"
> -for f in $tm_d_file; do
> -  case $f in
> -    defaults.h )
> -       tm_d_file_list="${tm_d_file_list} \$(srcdir)/$f"
> -       tm_d_include_list="${tm_d_include_list} $f"
> -       ;;
> -    * )
> -       tm_d_file_list="${tm_d_file_list} \$(srcdir)/config/$f"
> -       tm_d_include_list="${tm_d_include_list} config/$f"
> -       ;;
> -  esac
> -done
> -
>  xm_file_list=
>  xm_include_list=
>  for f in $xm_file; do
> @@ -7361,8 +7345,6 @@ AC_SUBST(tm_include_list)
>  AC_SUBST(tm_defines)
>  AC_SUBST(tm_p_file_list)
>  AC_SUBST(tm_p_include_list)
> -AC_SUBST(tm_d_file_list)
> -AC_SUBST(tm_d_include_list)
>  AC_SUBST(xm_file_list)
>  AC_SUBST(xm_include_list)
>  AC_SUBST(xm_defines)
> diff --git a/gcc/d/d-builtins.cc b/gcc/d/d-builtins.cc
> index c2ef0c836e1..a5491316599 100644
> --- a/gcc/d/d-builtins.cc
> +++ b/gcc/d/d-builtins.cc
> @@ -34,6 +34,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "diagnostic.h"
>  #include "langhooks.h"
>  #include "target.h"
> +#include "memmodel.h"
> +#include "tm_p.h"
>  #include "common/common-target.h"
>  #include "stringpool.h"
>  #include "stor-layout.h"
> @@ -508,7 +510,9 @@ d_init_versions (void)
>    VersionCondition::addPredefinedGlobalIdent ("all");
>
>    /* Emit all target-specific version identifiers.  */
> -  targetdm.d_cpu_versions ();
> +#ifdef TARGET_D_CPU_VERSIONS
> +  TARGET_D_CPU_VERSIONS ();
> +#endif
>    targetdm.d_os_versions ();
>
>    VersionCondition::addPredefinedGlobalIdent ("CppRuntime_Gcc");
> diff --git a/gcc/d/d-target.cc b/gcc/d/d-target.cc
> index d4350e593e4..7632a0af37b 100644
> --- a/gcc/d/d-target.cc
> +++ b/gcc/d/d-target.cc
> @@ -200,7 +200,9 @@ Target::_init (const Param &)
>
>    /* Initialize target info tables, the keys required by the language are added
>       last, so that the OS and CPU handlers can override.  */
> -  targetdm.d_register_cpu_target_info ();
> +#ifdef TARGET_D_REGISTER_CPU_TARGET_INFO
> +  TARGET_D_REGISTER_CPU_TARGET_INFO ();
> +#endif
>    targetdm.d_register_os_target_info ();
>    d_add_target_info_handlers (d_language_target_info);
>  }
> diff --git a/gcc/d/d-target.def b/gcc/d/d-target.def
> index 7805942b91e..10b0fe8171a 100644
> --- a/gcc/d/d-target.def
> +++ b/gcc/d/d-target.def
> @@ -28,16 +28,6 @@ HOOK_VECTOR (TARGETDM_INITIALIZER, gcc_targetdm)
>  #undef HOOK_PREFIX
>  #define HOOK_PREFIX "TARGET_"
>
> -/* Environmental version identifiers relating to the target CPU.  */
> -DEFHOOK
> -(d_cpu_versions,
> - "Declare all environmental version identifiers relating to the target CPU\n\
> -using the function @code{builtin_version}, which takes a string representing\n\
> -the name of the version.  Version identifiers predefined by this hook apply\n\
> -to all modules that are being compiled and imported.",
> - void, (void),
> - hook_void_void)
> -
>  /* Environmental version identifiers relating to the target OS.  */
>  DEFHOOK
>  (d_os_versions,
> @@ -46,18 +36,6 @@ relating to the target operating system.",
>   void, (void),
>   hook_void_void)
>
> -/* getTargetInfo keys relating to the target CPU.  */
> -DEFHOOK
> -(d_register_cpu_target_info,
> - "Register all target information keys relating to the target CPU using the\n\
> -function @code{d_add_target_info_handlers}, which takes a\n\
> -@samp{struct d_target_info_spec} (defined in @file{d/d-target.h}).  The keys\n\
> -added by this hook are made available at compile time by the\n\
> -@code{__traits(getTargetInfo)} extension, the result is an expression\n\
> -describing the requested target information.",
> - void, (void),
> - hook_void_void)
> -
>  /* getTargetInfo keys relating to the target OS.  */
>  DEFHOOK
>  (d_register_os_target_info,
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 92bda1a7e14..e2c4e98ed7c 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -11005,26 +11005,28 @@ Return target-specific mangling context of @var{decl} or @code{NULL_TREE}.
>  @section D ABI parameters
>  @cindex parameters, d abi
>
> -@deftypefn {D Target Hook} void TARGET_D_CPU_VERSIONS (void)
> -Declare all environmental version identifiers relating to the target CPU
> -using the function @code{builtin_version}, which takes a string representing
> -the name of the version.  Version identifiers predefined by this hook apply
> -to all modules that are being compiled and imported.
> -@end deftypefn
> +@defmac TARGET_D_CPU_VERSIONS ()
> +This function-like macro expands to a block of code that declares all
> +environmental version identifiers relating to the target CPU using the function
> +@code{builtin_version}, which takes a string representing the name of the
> +version.  Version identifiers predefined by this hook apply to all modules that
> +are being compiled and imported.
> +@end defmac
>
>  @deftypefn {D Target Hook} void TARGET_D_OS_VERSIONS (void)
>  Similarly to @code{TARGET_D_CPU_VERSIONS}, but is used for versions
>  relating to the target operating system.
>  @end deftypefn
>
> -@deftypefn {D Target Hook} void TARGET_D_REGISTER_CPU_TARGET_INFO (void)
> -Register all target information keys relating to the target CPU using the
> -function @code{d_add_target_info_handlers}, which takes a
> +@defmac TARGET_D_REGISTER_CPU_TARGET_INFO ()
> +This function-like macro expands to a block of code that registers all target
> +information keys relating to the target CPU using the function
> +@code{d_add_target_info_handlers}, which takes a
>  @samp{struct d_target_info_spec} (defined in @file{d/d-target.h}).  The keys
>  added by this hook are made available at compile time by the
>  @code{__traits(getTargetInfo)} extension, the result is an expression
>  describing the requested target information.
> -@end deftypefn
> +@end defmac
>
>  @deftypefn {D Target Hook} void TARGET_D_REGISTER_OS_TARGET_INFO (void)
>  Same as @code{TARGET_D_CPU_TARGET_INFO}, but is used for keys relating to
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 112462310b1..69a4188c4b2 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -7309,11 +7309,25 @@ floating-point support; they are not included in this mechanism.
>  @section D ABI parameters
>  @cindex parameters, d abi
>
> -@hook TARGET_D_CPU_VERSIONS
> +@defmac TARGET_D_CPU_VERSIONS ()
> +This function-like macro expands to a block of code that declares all
> +environmental version identifiers relating to the target CPU using the function
> +@code{builtin_version}, which takes a string representing the name of the
> +version.  Version identifiers predefined by this hook apply to all modules that
> +are being compiled and imported.
> +@end defmac
>
>  @hook TARGET_D_OS_VERSIONS
>
> -@hook TARGET_D_REGISTER_CPU_TARGET_INFO
> +@defmac TARGET_D_REGISTER_CPU_TARGET_INFO ()
> +This function-like macro expands to a block of code that registers all target
> +information keys relating to the target CPU using the function
> +@code{d_add_target_info_handlers}, which takes a
> +@samp{struct d_target_info_spec} (defined in @file{d/d-target.h}).  The keys
> +added by this hook are made available at compile time by the
> +@code{__traits(getTargetInfo)} extension, the result is an expression
> +describing the requested target information.
> +@end defmac
>
>  @hook TARGET_D_REGISTER_OS_TARGET_INFO
>
> --
> 2.34.1
>