c++: Add target hook for emit_support_tinfos [PR108883]

Message ID Y/c+mpY9VdAiIVFO@tucnak
State Unresolved
Headers
Series c++: Add target hook for emit_support_tinfos [PR108883] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Feb. 23, 2023, 10:23 a.m. UTC
  Hi!

_Float16 and decltype(0.0bf16) types are on x86 supported only with
-msse2.  On x86_64 that is the default, but on ia32 it is not.
We should still emit fundamental type tinfo for those types in
libsupc++.a/libstdc++.*, regardless of whether libsupc++/libstdc++
is compiled with -msse2 or not, as user programs can be compiled
with different ISA flags from libsupc++/libstdc++ and if they
are compiled with -msse2 and use std::float16_t or std::bfloat16_t
and need RTTI for it, it should work out of the box.  Furthermore,
libstdc++ ABI on ia32 shouldn't depend on whether the library
is compiled with -mno-sse or -msse2.

Unfortunately, just hacking up libsupc++ Makefile/configure so that
a single source is compiled with -msse2 isn't appropriate, because
that TU emits also code and the code should be able to run on CPUs
which libstdc++ supports.  We could add [[gnu::attribute ("no-sse2")]]
there perhaps conditionally, but it all gets quite ugly.

The following patch instead adds a target hook which allows the backend
to temporarily tweak registered types such that emit_support_tinfos
emits whatever is needed.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-02-23  Jakub Jelinek  <jakub@redhat.com>

	PR target/108883
	* hooks.h (hook_void_bool): Declare.
	* hooks.cc (hook_void_bool): New function.
	* target.def (emit_support_tinfos): New target hook.
	* doc/tm.texi.in (emit_support_tinfos): Document it.
	* doc/tm.texi: Regenerated.
	* config/i386/i386.cc (ix86_emit_support_tinfos): New function.
	(TARGET_EMIT_SUPPORT_TINFOS): Redefine.

	* rtti.cc (emit_support_tinfos): Call targetm.emit_support_tinfos
	before and after emit_support_tinfo_1 calls.


	Jakub
  

Comments

Jason Merrill Feb. 27, 2023, 11:26 p.m. UTC | #1
On 2/23/23 05:23, Jakub Jelinek wrote:
> Hi!
> 
> _Float16 and decltype(0.0bf16) types are on x86 supported only with
> -msse2.  On x86_64 that is the default, but on ia32 it is not.
> We should still emit fundamental type tinfo for those types in
> libsupc++.a/libstdc++.*, regardless of whether libsupc++/libstdc++
> is compiled with -msse2 or not, as user programs can be compiled
> with different ISA flags from libsupc++/libstdc++ and if they
> are compiled with -msse2 and use std::float16_t or std::bfloat16_t
> and need RTTI for it, it should work out of the box.  Furthermore,
> libstdc++ ABI on ia32 shouldn't depend on whether the library
> is compiled with -mno-sse or -msse2.
> 
> Unfortunately, just hacking up libsupc++ Makefile/configure so that
> a single source is compiled with -msse2 isn't appropriate, because
> that TU emits also code and the code should be able to run on CPUs
> which libstdc++ supports.  We could add [[gnu::attribute ("no-sse2")]]
> there perhaps conditionally, but it all gets quite ugly.
> 
> The following patch instead adds a target hook which allows the backend
> to temporarily tweak registered types such that emit_support_tinfos
> emits whatever is needed.

Why handle these types differently from the DFP handling at the end of 
emit_support_tinfos?

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2023-02-23  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/108883
> 	* hooks.h (hook_void_bool): Declare.
> 	* hooks.cc (hook_void_bool): New function.
> 	* target.def (emit_support_tinfos): New target hook.
> 	* doc/tm.texi.in (emit_support_tinfos): Document it.
> 	* doc/tm.texi: Regenerated.
> 	* config/i386/i386.cc (ix86_emit_support_tinfos): New function.
> 	(TARGET_EMIT_SUPPORT_TINFOS): Redefine.
> 
> 	* rtti.cc (emit_support_tinfos): Call targetm.emit_support_tinfos
> 	before and after emit_support_tinfo_1 calls.
> 
> --- gcc/hooks.h.jj	2023-01-02 09:32:29.418183667 +0100
> +++ gcc/hooks.h	2023-02-22 12:34:32.144973558 +0100
> @@ -77,6 +77,7 @@ extern bool hook_bool_wint_wint_uint_boo
>   						unsigned int, bool);
>   
>   extern void hook_void_void (void);
> +extern void hook_void_bool (bool);
>   extern void hook_void_constcharptr (const char *);
>   extern void hook_void_rtx_insn_int (rtx_insn *, int);
>   extern void hook_void_FILEptr_constcharptr (FILE *, const char *);
> --- gcc/hooks.cc.jj	2023-01-02 09:32:49.675890970 +0100
> +++ gcc/hooks.cc	2023-02-22 12:36:46.241035355 +0100
> @@ -271,6 +271,11 @@ hook_hwi_void_0 (void)
>   }
>   
>   void
> +hook_void_bool (bool)
> +{
> +}
> +
> +void
>   hook_void_tree (tree)
>   {
>   }
> --- gcc/target.def.jj	2023-01-04 10:45:50.117881714 +0100
> +++ gcc/target.def	2023-02-22 12:33:39.715731356 +0100
> @@ -2606,6 +2606,20 @@ types.",
>    const char *, (const_tree type),
>    hook_constcharptr_const_tree_null)
>   
> +/* Temporarily add conditional target specific types for the purpose of
> +   emitting C++ fundamental type tinfos.  */
> +DEFHOOK
> +(emit_support_tinfos,
> + "If your target defines any fundamental types which depend on ISA flags,\n\
> +they might need C++ tinfo symbols in libsupc++/libstdc++ regardless of\n\
> +ISA flags the library is compiled with.\n\
> +The hook is called with @var{add} @code{true} at the start of C++ FE\n\
> +@code{emit_support_tinfos} and with @var{add} @code{false} at the end of it\n\
> +and can temporarily create fundamental types that are not supposed to be\n\
> +otherwise created due to the ISA options.",
> + void, (bool add),
> + hook_void_bool)
> +
>   /* Make any adjustments to libfunc names needed for this target.  */
>   DEFHOOK
>   (init_libfuncs,
> --- gcc/doc/tm.texi.in.jj	2023-01-16 11:52:16.124733460 +0100
> +++ gcc/doc/tm.texi.in	2023-02-22 12:46:37.951482849 +0100
> @@ -1286,6 +1286,8 @@ pattern needs to support both a 32- and
>   
>   @hook TARGET_MANGLE_TYPE
>   
> +@hook TARGET_EMIT_SUPPORT_TINFOS
> +
>   @node Type Layout
>   @section Layout of Source Language Data Types
>   
> --- gcc/doc/tm.texi.jj	2023-01-16 11:52:16.123733475 +0100
> +++ gcc/doc/tm.texi	2023-02-22 12:46:41.741428081 +0100
> @@ -1525,6 +1525,16 @@ appropriate for a target that does not d
>   types.
>   @end deftypefn
>   
> +@deftypefn {Target Hook} void TARGET_EMIT_SUPPORT_TINFOS (bool @var{add})
> +If your target defines any fundamental types which depend on ISA flags,
> +they might need C++ tinfo symbols in libsupc++/libstdc++ regardless of
> +ISA flags the library is compiled with.
> +The hook is called with @var{add} @code{true} at the start of C++ FE
> +@code{emit_support_tinfos} and with @var{add} @code{false} at the end of it
> +and can temporarily create fundamental types that are not supposed to be
> +otherwise created due to the ISA options.
> +@end deftypefn
> +
>   @node Type Layout
>   @section Layout of Source Language Data Types
>   
> --- gcc/config/i386/i386.cc.jj	2023-02-16 10:13:23.701210667 +0100
> +++ gcc/config/i386/i386.cc	2023-02-22 12:59:55.110960505 +0100
> @@ -22775,6 +22775,30 @@ ix86_mangle_type (const_tree type)
>       }
>   }
>   
> +/* Temporarily tweak the set of fundamental types for C++
> +   emit_support_tinfos purposes.  */
> +
> +static void
> +ix86_emit_support_tinfos (bool add)
> +{
> +  extern tree ix86_float16_type_node;
> +  extern tree ix86_bf16_type_node;
> +
> +  if (TARGET_SSE2)
> +    return;
> +  if (add)
> +    {
> +      gcc_checking_assert (!float16_type_node && !bfloat16_type_node);
> +      float16_type_node = ix86_float16_type_node;
> +      bfloat16_type_node = ix86_bf16_type_node;
> +    }
> +  else
> +    {
> +      float16_type_node = NULL_TREE;
> +      bfloat16_type_node = NULL_TREE;
> +    }
> +}
> +
>   static GTY(()) tree ix86_tls_stack_chk_guard_decl;
>   
>   static tree
> @@ -24954,6 +24978,9 @@ ix86_libgcc_floating_mode_supported_p
>   #undef TARGET_MANGLE_TYPE
>   #define TARGET_MANGLE_TYPE ix86_mangle_type
>   
> +#undef TARGET_EMIT_SUPPORT_TINFOS
> +#define TARGET_EMIT_SUPPORT_TINFOS ix86_emit_support_tinfos
> +
>   #undef TARGET_STACK_PROTECT_GUARD
>   #define TARGET_STACK_PROTECT_GUARD ix86_stack_protect_guard
>   
> --- gcc/cp/rtti.cc.jj	2023-01-16 11:52:16.090733961 +0100
> +++ gcc/cp/rtti.cc	2023-02-22 12:40:10.078089124 +0100
> @@ -1623,6 +1623,9 @@ emit_support_tinfos (void)
>     if (!dtor || DECL_EXTERNAL (dtor))
>       return;
>   
> +  /* Tell target code that support tinfos are about to be emitted.  */
> +  targetm.emit_support_tinfos (true);
> +
>     /* All these are really builtins.  So set the location.  */
>     location_t saved_loc = input_location;
>     input_location = BUILTINS_LOCATION;
> @@ -1652,6 +1655,9 @@ emit_support_tinfos (void)
>         emit_support_tinfo_1 (fallback_dfloat128_type);
>       }
>     input_location = saved_loc;
> +
> +  /* Tell target code that support tinfos have been emitted.  */
> +  targetm.emit_support_tinfos (false);
>   }
>   
>   /* Finish a type info decl. DECL_PTR is a pointer to an unemitted
> 
> 	Jakub
>
  
Jakub Jelinek Feb. 27, 2023, 11:51 p.m. UTC | #2
On Mon, Feb 27, 2023 at 06:26:04PM -0500, Jason Merrill wrote:
> > The following patch instead adds a target hook which allows the backend
> > to temporarily tweak registered types such that emit_support_tinfos
> > emits whatever is needed.
> 
> Why handle these types differently from the DFP handling at the end of
> emit_support_tinfos?

One thing is that the fallback_* nodes look like a waste to me,
the tinfo decls are mangled right away, and the fallback_* nodes need to be
walked by GC, so I think we could get away even for the decimal tinfos
to just do:
      dfloat32_type_node = make_node (REAL_TYPE);
      emit_support_tinfo_1 (dfloat32_type_node);
      dfloat32_type_node = NULL_TREE;
etc. and drop the fallback stuff.
If we wanted to do fallback_* even for the _Float*/decltype(0.0bf16)
nodes, which are at least sometimes mangled in target hooks it would
make stuff harder because fallback_* is C++ FE private.

And then there is a question whether we want to emit rtti for
_Float{16,32,64,128}, _Float{32,64,128}x and decltype(0.0bf16) regardless
of whether the target supports them at all or not.
Emitting them always would have an advantage, if say bfloat16_t support
isn't added for aarch64 for GCC 13 (it is still pending review), we wouldn't
need to deal with symbol versioning for it in GCC 14 or later.
On the other side, on some arches some types are very unlikely to be
supported.  And e.g. _Float128x isn't supported on any arch right now.

Though, if we can get rid of the fallback_* stuff and we wanted to emit
all _Float{16,32,64,128}, _Float{32,64,128}x and decltype(0.0bf16) tinfos
on all arches (or say for now all but _Float128x), we could do it simply
by splitting the fundamentals array in emit_support_tinfos into
one without fallback and one with fallback, put say
    &dfloat32_type_node, &dfloat64_type_node, &dfloat128_type_node,
    &bfloat16_type_node, &float16_type_node, &float32_type_node,
    &float64_type_node, &float128_type_node, &float32x_type_node,
    &float64x_type_node, &float128x_type_node, 0
into the latter and simply handle the NULL case with a temporary fallback,
like:
  tree fallback = NULL_TREE;
  for (ix = 0; fundamentals_with_fallback[ix]; ix++)
    if (*fundamentals_with_fallback[ix])
      emit_support_tinfo_1 (*fundamentals_with_fallback[ix]);
    else
      {
	if (fallback == NULL_TREE)
	  fallback = make_node (REAL_TYPE);
	*fundamentals_with_fallback[ix] = fallback;
	emit_support_tinfo_1 (fallback);
	*fundamentals_with_fallback[ix] = NULL_TREE;
      }

	Jakub
  
Jason Merrill March 1, 2023, 10:50 p.m. UTC | #3
On 2/27/23 18:51, Jakub Jelinek wrote:
> On Mon, Feb 27, 2023 at 06:26:04PM -0500, Jason Merrill wrote:
>>> The following patch instead adds a target hook which allows the backend
>>> to temporarily tweak registered types such that emit_support_tinfos
>>> emits whatever is needed.
>>
>> Why handle these types differently from the DFP handling at the end of
>> emit_support_tinfos?
> 
> One thing is that the fallback_* nodes look like a waste to me,
> the tinfo decls are mangled right away, and the fallback_* nodes need to be
> walked by GC, so I think we could get away even for the decimal tinfos
> to just do:
>        dfloat32_type_node = make_node (REAL_TYPE);
>        emit_support_tinfo_1 (dfloat32_type_node);
>        dfloat32_type_node = NULL_TREE;
> etc. and drop the fallback stuff.

I think you're right.

> If we wanted to do fallback_* even for the _Float*/decltype(0.0bf16)
> nodes, which are at least sometimes mangled in target hooks it would
> make stuff harder because fallback_* is C++ FE private.
> 
> And then there is a question whether we want to emit rtti for
> _Float{16,32,64,128}, _Float{32,64,128}x and decltype(0.0bf16) regardless
> of whether the target supports them at all or not.
> Emitting them always would have an advantage, if say bfloat16_t support
> isn't added for aarch64 for GCC 13 (it is still pending review), we wouldn't
> need to deal with symbol versioning for it in GCC 14 or later.
> On the other side, on some arches some types are very unlikely to be
> supported.  And e.g. _Float128x isn't supported on any arch right now.

A good point.  Incidentally, it seems problematic for embedded users 
that all the fundamental type_infos are emitted in the same .o, making 
it hard to link in only the ones you care about.  And new floating-point 
variants add to that problem.  So perhaps until that is addressed, it's 
better to avoid adding a bunch more on targets that don't support them.

Hmm.

> Though, if we can get rid of the fallback_* stuff and we wanted to emit
> all _Float{16,32,64,128}, _Float{32,64,128}x and decltype(0.0bf16) tinfos
> on all arches (or say for now all but _Float128x), we could do it simply
> by splitting the fundamentals array in emit_support_tinfos into
> one without fallback and one with fallback, put say
>      &dfloat32_type_node, &dfloat64_type_node, &dfloat128_type_node,
>      &bfloat16_type_node, &float16_type_node, &float32_type_node,
>      &float64_type_node, &float128_type_node, &float32x_type_node,
>      &float64x_type_node, &float128x_type_node, 0
> into the latter and simply handle the NULL case with a temporary fallback,
> like:
>    tree fallback = NULL_TREE;
>    for (ix = 0; fundamentals_with_fallback[ix]; ix++)
>      if (*fundamentals_with_fallback[ix])
>        emit_support_tinfo_1 (*fundamentals_with_fallback[ix]);
>      else
>        {
> 	if (fallback == NULL_TREE)
> 	  fallback = make_node (REAL_TYPE);
> 	*fundamentals_with_fallback[ix] = fallback;
> 	emit_support_tinfo_1 (fallback);
> 	*fundamentals_with_fallback[ix] = NULL_TREE;
>        }
> 
> 	Jakub
>
  

Patch

--- gcc/hooks.h.jj	2023-01-02 09:32:29.418183667 +0100
+++ gcc/hooks.h	2023-02-22 12:34:32.144973558 +0100
@@ -77,6 +77,7 @@  extern bool hook_bool_wint_wint_uint_boo
 						unsigned int, bool);
 
 extern void hook_void_void (void);
+extern void hook_void_bool (bool);
 extern void hook_void_constcharptr (const char *);
 extern void hook_void_rtx_insn_int (rtx_insn *, int);
 extern void hook_void_FILEptr_constcharptr (FILE *, const char *);
--- gcc/hooks.cc.jj	2023-01-02 09:32:49.675890970 +0100
+++ gcc/hooks.cc	2023-02-22 12:36:46.241035355 +0100
@@ -271,6 +271,11 @@  hook_hwi_void_0 (void)
 }
 
 void
+hook_void_bool (bool)
+{
+}
+
+void
 hook_void_tree (tree)
 {
 }
--- gcc/target.def.jj	2023-01-04 10:45:50.117881714 +0100
+++ gcc/target.def	2023-02-22 12:33:39.715731356 +0100
@@ -2606,6 +2606,20 @@  types.",
  const char *, (const_tree type),
  hook_constcharptr_const_tree_null)
 
+/* Temporarily add conditional target specific types for the purpose of
+   emitting C++ fundamental type tinfos.  */
+DEFHOOK
+(emit_support_tinfos,
+ "If your target defines any fundamental types which depend on ISA flags,\n\
+they might need C++ tinfo symbols in libsupc++/libstdc++ regardless of\n\
+ISA flags the library is compiled with.\n\
+The hook is called with @var{add} @code{true} at the start of C++ FE\n\
+@code{emit_support_tinfos} and with @var{add} @code{false} at the end of it\n\
+and can temporarily create fundamental types that are not supposed to be\n\
+otherwise created due to the ISA options.",
+ void, (bool add),
+ hook_void_bool)
+
 /* Make any adjustments to libfunc names needed for this target.  */
 DEFHOOK
 (init_libfuncs,
--- gcc/doc/tm.texi.in.jj	2023-01-16 11:52:16.124733460 +0100
+++ gcc/doc/tm.texi.in	2023-02-22 12:46:37.951482849 +0100
@@ -1286,6 +1286,8 @@  pattern needs to support both a 32- and
 
 @hook TARGET_MANGLE_TYPE
 
+@hook TARGET_EMIT_SUPPORT_TINFOS
+
 @node Type Layout
 @section Layout of Source Language Data Types
 
--- gcc/doc/tm.texi.jj	2023-01-16 11:52:16.123733475 +0100
+++ gcc/doc/tm.texi	2023-02-22 12:46:41.741428081 +0100
@@ -1525,6 +1525,16 @@  appropriate for a target that does not d
 types.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_EMIT_SUPPORT_TINFOS (bool @var{add})
+If your target defines any fundamental types which depend on ISA flags,
+they might need C++ tinfo symbols in libsupc++/libstdc++ regardless of
+ISA flags the library is compiled with.
+The hook is called with @var{add} @code{true} at the start of C++ FE
+@code{emit_support_tinfos} and with @var{add} @code{false} at the end of it
+and can temporarily create fundamental types that are not supposed to be
+otherwise created due to the ISA options.
+@end deftypefn
+
 @node Type Layout
 @section Layout of Source Language Data Types
 
--- gcc/config/i386/i386.cc.jj	2023-02-16 10:13:23.701210667 +0100
+++ gcc/config/i386/i386.cc	2023-02-22 12:59:55.110960505 +0100
@@ -22775,6 +22775,30 @@  ix86_mangle_type (const_tree type)
     }
 }
 
+/* Temporarily tweak the set of fundamental types for C++
+   emit_support_tinfos purposes.  */
+
+static void
+ix86_emit_support_tinfos (bool add)
+{
+  extern tree ix86_float16_type_node;
+  extern tree ix86_bf16_type_node;
+
+  if (TARGET_SSE2)
+    return;
+  if (add)
+    {
+      gcc_checking_assert (!float16_type_node && !bfloat16_type_node);
+      float16_type_node = ix86_float16_type_node;
+      bfloat16_type_node = ix86_bf16_type_node;
+    }
+  else
+    {
+      float16_type_node = NULL_TREE;
+      bfloat16_type_node = NULL_TREE;
+    }
+}
+
 static GTY(()) tree ix86_tls_stack_chk_guard_decl;
 
 static tree
@@ -24954,6 +24978,9 @@  ix86_libgcc_floating_mode_supported_p
 #undef TARGET_MANGLE_TYPE
 #define TARGET_MANGLE_TYPE ix86_mangle_type
 
+#undef TARGET_EMIT_SUPPORT_TINFOS
+#define TARGET_EMIT_SUPPORT_TINFOS ix86_emit_support_tinfos
+
 #undef TARGET_STACK_PROTECT_GUARD
 #define TARGET_STACK_PROTECT_GUARD ix86_stack_protect_guard
 
--- gcc/cp/rtti.cc.jj	2023-01-16 11:52:16.090733961 +0100
+++ gcc/cp/rtti.cc	2023-02-22 12:40:10.078089124 +0100
@@ -1623,6 +1623,9 @@  emit_support_tinfos (void)
   if (!dtor || DECL_EXTERNAL (dtor))
     return;
 
+  /* Tell target code that support tinfos are about to be emitted.  */
+  targetm.emit_support_tinfos (true);
+
   /* All these are really builtins.  So set the location.  */
   location_t saved_loc = input_location;
   input_location = BUILTINS_LOCATION;
@@ -1652,6 +1655,9 @@  emit_support_tinfos (void)
       emit_support_tinfo_1 (fallback_dfloat128_type);
     }
   input_location = saved_loc;
+
+  /* Tell target code that support tinfos have been emitted.  */
+  targetm.emit_support_tinfos (false);
 }
 
 /* Finish a type info decl. DECL_PTR is a pointer to an unemitted