c++: Copy over DECL_DISREGARD_INLINE_LIMITS flag to inheriting ctors [PR114784]

Message ID ZiaIHDRhnDHVs36U@tucnak
State New
Headers
Series c++: Copy over DECL_DISREGARD_INLINE_LIMITS flag to inheriting ctors [PR114784] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed

Commit Message

Jakub Jelinek April 22, 2024, 3:54 p.m. UTC
  Hi!

The following testcase is rejected with
error: inlining failed in call to 'always_inline' '...': call is unlikely and code size would grow
errors.  The problem is that starting with the r14-2149 change
we try to copy most of the attributes from the inherited to
inheriting ctor, but don't copy associated flags that decl_attributes
sets.

Now, the other clone_attrs user, cp/optimize.cc (maybe_clone_body)
copies over
      DECL_COMDAT (clone) = DECL_COMDAT (fn);
      DECL_WEAK (clone) = DECL_WEAK (fn);
      if (DECL_ONE_ONLY (fn))
        cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone));
      DECL_USE_TEMPLATE (clone) = DECL_USE_TEMPLATE (fn);
      DECL_EXTERNAL (clone) = DECL_EXTERNAL (fn);
      DECL_INTERFACE_KNOWN (clone) = DECL_INTERFACE_KNOWN (fn);
      DECL_NOT_REALLY_EXTERN (clone) = DECL_NOT_REALLY_EXTERN (fn);
      DECL_VISIBILITY (clone) = DECL_VISIBILITY (fn);
      DECL_VISIBILITY_SPECIFIED (clone) = DECL_VISIBILITY_SPECIFIED (fn);
      DECL_DLLIMPORT_P (clone) = DECL_DLLIMPORT_P (fn);
      DECL_DISREGARD_INLINE_LIMITS (clone) = DECL_DISREGARD_INLINE_LIMITS (fn);
The following patch just copies DECL_DISREGARD_INLINE_LIMITS to fix
this exact bug, not really sure which other flags should be copied
and which shouldn't.
Plus there are tons of other flags, some of which might need to be copied
too, some of which might not, perhaps in both places, like:
DECL_UNINLINABLE, maybe DECL_PRESERVE_P, TREE_USED, maybe
DECL_USER_ALIGN/DECL_ALIGN, maybe DECL_WEAK, maybe
DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT, DECL_NO_LIMIT_STACK.
TREE_READONLY, DECL_PURE_P, TREE_THIS_VOLATILE (for const, pure and
noreturn attributes) probably makes no sense, DECL_IS_RETURNS_TWICE neither
(returns_twice ctor?).  What about TREE_NOTHROW?
DECL_FUNCTION_SPECIFIC_OPTIMIZATION, DECL_FUNCTION_SPECIFIC_TARGET...

Anyway, another problem is that if inherited_ctor is a TEMPLATE_DECL, as
also can be seen in the using D<T>::D; case in the testcase, then
DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor));
attempts to copy the attributes from the TEMPLATE_DECL which doesn't have
them.  The following patch copies them from STRIP_TEMPLATE (inherited_ctor)
which does.  E.g. DECL_DECLARED_CONSTEXPR_P works fine as the macro
itself uses STRIP_TEMPLATE too, but not 100% sure about other macros used
on inherited_ctor earlier.

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

Do you want to copy other flags (and which ones and in which places),
is that ok to be deferred till stage 1? 

2024-04-22  Jakub Jelinek  <jakub@redhat.com>

	PR c++/114784
	* method.cc (implicitly_declare_fn): Call clone_attrs
	on DECL_ATTRIBUTES on STRIP_TEMPLATE (inherited_ctor) rather than
	inherited_ctor.  Also copy DECL_DISREGARD_INLINE_LIMITS flag from it.

	* g++.dg/cpp0x/inh-ctor39.C: New test.


	Jakub
  

Comments

Jason Merrill April 23, 2024, 3:04 a.m. UTC | #1
On 4/22/24 08:54, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase is rejected with
> error: inlining failed in call to 'always_inline' '...': call is unlikely and code size would grow
> errors.  The problem is that starting with the r14-2149 change
> we try to copy most of the attributes from the inherited to
> inheriting ctor, but don't copy associated flags that decl_attributes
> sets.
> 
> Now, the other clone_attrs user, cp/optimize.cc (maybe_clone_body)
> copies over
>        DECL_COMDAT (clone) = DECL_COMDAT (fn);
>        DECL_WEAK (clone) = DECL_WEAK (fn);
>        if (DECL_ONE_ONLY (fn))
>          cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone));
>        DECL_USE_TEMPLATE (clone) = DECL_USE_TEMPLATE (fn);
>        DECL_EXTERNAL (clone) = DECL_EXTERNAL (fn);
>        DECL_INTERFACE_KNOWN (clone) = DECL_INTERFACE_KNOWN (fn);
>        DECL_NOT_REALLY_EXTERN (clone) = DECL_NOT_REALLY_EXTERN (fn);
>        DECL_VISIBILITY (clone) = DECL_VISIBILITY (fn);
>        DECL_VISIBILITY_SPECIFIED (clone) = DECL_VISIBILITY_SPECIFIED (fn);
>        DECL_DLLIMPORT_P (clone) = DECL_DLLIMPORT_P (fn);
>        DECL_DISREGARD_INLINE_LIMITS (clone) = DECL_DISREGARD_INLINE_LIMITS (fn);
> The following patch just copies DECL_DISREGARD_INLINE_LIMITS to fix
> this exact bug, not really sure which other flags should be copied
> and which shouldn't.
> Plus there are tons of other flags, some of which might need to be copied
> too, some of which might not, perhaps in both places, like:
> DECL_UNINLINABLE, maybe DECL_PRESERVE_P, TREE_USED, maybe
> DECL_USER_ALIGN/DECL_ALIGN, maybe DECL_WEAK, maybe
> DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT, DECL_NO_LIMIT_STACK.
> TREE_READONLY, DECL_PURE_P, TREE_THIS_VOLATILE (for const, pure and
> noreturn attributes) probably makes no sense, DECL_IS_RETURNS_TWICE neither
> (returns_twice ctor?).  What about TREE_NOTHROW?
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION, DECL_FUNCTION_SPECIFIC_TARGET...
> 
> Anyway, another problem is that if inherited_ctor is a TEMPLATE_DECL, as
> also can be seen in the using D<T>::D; case in the testcase, then
> DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor));
> attempts to copy the attributes from the TEMPLATE_DECL which doesn't have
> them.  The following patch copies them from STRIP_TEMPLATE (inherited_ctor)
> which does.  E.g. DECL_DECLARED_CONSTEXPR_P works fine as the macro
> itself uses STRIP_TEMPLATE too, but not 100% sure about other macros used
> on inherited_ctor earlier.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> Do you want to copy other flags (and which ones and in which places),
> is that ok to be deferred till stage 1?

Most of the others don't seem like they should be copied, but rather 
determined from the function body as usual.  Maybe 
DECL_FUNCTION_SPECIFIC_TARGET would make sense.  Either way, it can wait 
for stage 1.

> 2024-04-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/114784
> 	* method.cc (implicitly_declare_fn): Call clone_attrs
> 	on DECL_ATTRIBUTES on STRIP_TEMPLATE (inherited_ctor) rather than
> 	inherited_ctor.  Also copy DECL_DISREGARD_INLINE_LIMITS flag from it.
> 
> 	* g++.dg/cpp0x/inh-ctor39.C: New test.
> 
> --- gcc/cp/method.cc.jj	2024-04-20 00:02:56.702387748 +0200
> +++ gcc/cp/method.cc	2024-04-22 12:51:36.395722484 +0200
> @@ -3307,8 +3307,11 @@ implicitly_declare_fn (special_function_
>         /* Copy constexpr from the inherited constructor even if the
>   	 inheriting constructor doesn't satisfy the requirements.  */
>         constexpr_p = DECL_DECLARED_CONSTEXPR_P (inherited_ctor);
> +      tree inherited_ctor_fn = STRIP_TEMPLATE (inherited_ctor);
>         /* Also copy any attributes.  */
> -      DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor));
> +      DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor_fn));
> +      DECL_DISREGARD_INLINE_LIMITS (fn)
> +	= DECL_DISREGARD_INLINE_LIMITS (inherited_ctor_fn);
>       }
>   
>     /* Add the "this" parameter.  */
> --- gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C.jj	2024-04-22 13:02:10.490836357 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C	2024-04-22 13:01:50.088122255 +0200
> @@ -0,0 +1,55 @@
> +// PR c++/114784
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "-O2" }
> +
> +template <typename T>
> +struct A {
> +  [[gnu::always_inline]] A (int t) { foo ().bar (t, {}); }
> +  [[gnu::always_inline]] A (long long t) { foo ().bar (t, {}); }
> +  T foo ();
> +};
> +
> +struct B : A<B> {
> +  using A<B>::A;
> +  [[gnu::always_inline]] B (long long v) : A (v) {}
> +  template <typename T>
> +  void bar (T &&, int);
> +  char b;
> +};
> +
> +struct C {
> +  C (int v) : a(v) { }
> +  C (long long v) : a(v) { }
> +  B a;
> +};
> +
> +static C
> +baz ()
> +{
> +  C x(0);
> +  C y(0LL);
> +  return 0;
> +}
> +
> +[[gnu::cold]] int
> +qux ()
> +{
> +  baz ();
> +  return 0;
> +}
> +
> +template <typename>
> +struct D {
> +  template <typename T>
> +  [[gnu::always_inline]] D (T) { d = sizeof (T); }
> +  D();
> +  int d;
> +};
> +template <typename T>
> +struct E : D<T> {
> +  using D<T>::D;
> +};
> +
> +E<char> c = {};
> +E<char> d = 1;
> +E<char> e = 1.0;
> 
> 	Jakub
>
  

Patch

--- gcc/cp/method.cc.jj	2024-04-20 00:02:56.702387748 +0200
+++ gcc/cp/method.cc	2024-04-22 12:51:36.395722484 +0200
@@ -3307,8 +3307,11 @@  implicitly_declare_fn (special_function_
       /* Copy constexpr from the inherited constructor even if the
 	 inheriting constructor doesn't satisfy the requirements.  */
       constexpr_p = DECL_DECLARED_CONSTEXPR_P (inherited_ctor);
+      tree inherited_ctor_fn = STRIP_TEMPLATE (inherited_ctor);
       /* Also copy any attributes.  */
-      DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor));
+      DECL_ATTRIBUTES (fn) = clone_attrs (DECL_ATTRIBUTES (inherited_ctor_fn));
+      DECL_DISREGARD_INLINE_LIMITS (fn)
+	= DECL_DISREGARD_INLINE_LIMITS (inherited_ctor_fn);
     }
 
   /* Add the "this" parameter.  */
--- gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C.jj	2024-04-22 13:02:10.490836357 +0200
+++ gcc/testsuite/g++.dg/cpp0x/inh-ctor39.C	2024-04-22 13:01:50.088122255 +0200
@@ -0,0 +1,55 @@ 
+// PR c++/114784
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-O2" }
+
+template <typename T>
+struct A {
+  [[gnu::always_inline]] A (int t) { foo ().bar (t, {}); }
+  [[gnu::always_inline]] A (long long t) { foo ().bar (t, {}); }
+  T foo ();
+};
+
+struct B : A<B> {
+  using A<B>::A;
+  [[gnu::always_inline]] B (long long v) : A (v) {}
+  template <typename T>
+  void bar (T &&, int);
+  char b;
+};
+
+struct C {
+  C (int v) : a(v) { }
+  C (long long v) : a(v) { }
+  B a;
+};
+
+static C
+baz ()
+{
+  C x(0);
+  C y(0LL);
+  return 0;
+}
+
+[[gnu::cold]] int
+qux ()
+{
+  baz ();
+  return 0;
+}
+
+template <typename>
+struct D {
+  template <typename T>
+  [[gnu::always_inline]] D (T) { d = sizeof (T); }
+  D();
+  int d;
+};
+template <typename T>
+struct E : D<T> {
+  using D<T>::D;
+};
+
+E<char> c = {};
+E<char> d = 1;
+E<char> e = 1.0;