public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jonathan Wakely <jwakely@redhat.com>,
	Jason Merrill <jason@redhat.com>, Jan Hubicka <jh@suse.cz>
Cc: gcc-patches@gcc.gnu.org, Richard Biener <rguenther@suse.de>,
	Patrick Palka <ppalka@redhat.com>
Subject: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]
Date: Wed, 24 Apr 2024 18:16:51 +0200	[thread overview]
Message-ID: <Zikwc9BTXQErR++G@tucnak> (raw)
In-Reply-To: <CACb0b4kC=P34Faq6R14X-mq2KA_NUbLS1uXLs06fDp_Ro0Rw8Q@mail.gmail.com>

On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote:
> > That fixes the testcases too, but seems to regress
> > +FAIL: libstdc++-abi/abi_check

> There are explicit instantiation definitions that should instantiate
> those types:
> 
> src/c++17/fs_dir.cc:template class std::__shared_ptr<fs::_Dir>;
> src/c++17/fs_dir.cc:template class
> std::__shared_ptr<fs::recursive_directory_iterator::_Dir_stack>;
> src/c++17/fs_path.cc:template class std::__shared_ptr<const
> fs::filesystem_error::_Impl>;
> 
> So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o

So this boils down to (cvise reduced):
namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic } const __default_lock_policy = _S_atomic; }
namespace std {
using __gnu_cxx::__default_lock_policy;
using __gnu_cxx::_Lock_policy;
template <typename, _Lock_policy = __default_lock_policy> struct __shared_ptr { constexpr __shared_ptr() {} };
namespace filesystem {
struct _Dir;
struct directory_iterator { __shared_ptr<_Dir> _M_dir; };
void end() { directory_iterator(); } }
extern template class __shared_ptr<filesystem::_Dir>;
}
namespace fs = std::filesystem;
template class std::__shared_ptr<fs::_Dir>;
at -O2, previously it used to emit
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev
but now no longer does with the yesterday posted PR113208 patch.

The following updated patch fixes that by calling note_vague_linkage_fn for
the cdtor clones from maybe_clone_body if the flags suggest that the
maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn
called too.  And then I've noticed that in some cases the updated comdat
group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by
maybe_make_one_only.  So the patch tweaks cxx_comdat_group, so that when
some comdat group has been chosen already it doesn't try to use some
different one.

Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't
regress anything unlike the earlier patch.

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

	PR lto/113208
	* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
	for implicit instantiations of maybe in charge ctors/dtors
	if -fimplicit-templates or -fimplicit-inline-templates and
	-fweak and target supports aliases.
	* optimize.cc (maybe_clone_body): Call note_vague_linkage_fn
	on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN
	and DECL_DEFER_OUTPUT flags set.
	* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
	functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
	set.

	* g++.dg/abi/comdat2.C: New test.
	* g++.dg/abi/comdat3.C: New test.
	* g++.dg/lto/pr113208_0.C: New test.
	* g++.dg/lto/pr113208_1.C: New file.
	* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj	2024-04-23 14:49:41.933186265 +0200
+++ gcc/cp/decl2.cc	2024-04-24 15:17:09.043625729 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
 	     to mark the functions at this point.  */
 	  if (DECL_DECLARED_INLINE_P (decl)
 	      && (!DECL_IMPLICIT_INSTANTIATION (decl)
-		  || DECL_DEFAULTED_FN (decl)))
+		  || DECL_DEFAULTED_FN (decl)
+		  /* For implicit instantiations of cdtors,
+		     if import_export_decl would use comdat linkage,
+		     make sure to use it right away, so that maybe_clone_body
+		     can use aliases.  See PR113208.  */
+		  || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+		      && (flag_implicit_templates
+			  || flag_implicit_inline_templates)
+		      && flag_weak
+		      && TARGET_SUPPORTS_ALIASES)))
 	    {
 	      /* This function must have external linkage, as
 		 otherwise DECL_INTERFACE_KNOWN would have been
--- gcc/cp/optimize.cc.jj	2024-04-24 13:44:26.456634100 +0200
+++ gcc/cp/optimize.cc	2024-04-24 14:46:13.050371557 +0200
@@ -717,6 +717,10 @@ maybe_clone_body (tree fn)
       else
 	expand_or_defer_fn (clone);
       first = false;
+      if (DECL_INTERFACE_KNOWN (fn)
+	  && DECL_NOT_REALLY_EXTERN (fn)
+	  && DECL_DEFER_OUTPUT (fn))
+	note_vague_linkage_fn (clone);
     }
   pop_from_top_level ();
 
--- gcc/cp/decl.cc.jj	2024-04-23 08:31:05.515161033 +0200
+++ gcc/cp/decl.cc	2024-04-24 15:15:34.401951491 +0200
@@ -19254,6 +19254,14 @@ cxx_comdat_group (tree decl)
 	  else
 	    break;
 	}
+      /* If a ctor/dtor has already set the comdat group by
+	 maybe_clone_body, don't override it.  */
+      if (SUPPORTS_ONE_ONLY
+	  && TREE_CODE (decl) == FUNCTION_DECL
+	  && DECL_CLONED_FUNCTION_P (decl)
+	  && SUPPORTS_ONE_ONLY)
+	if (tree comdat = DECL_COMDAT_GROUP (decl))
+	  return comdat;
     }
 
   return decl;
--- gcc/testsuite/g++.dg/abi/comdat2.C.jj	2024-04-24 13:44:11.054849035 +0200
+++ gcc/testsuite/g++.dg/abi/comdat2.C	2024-04-24 13:44:11.054849035 +0200
@@ -0,0 +1,26 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2 -fkeep-inline-functions" }
+// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } }
+
+template <typename T>
+struct A {
+  int foo () const;
+  A (int, int);
+};
+template <typename T>
+struct B : A<T> {
+  constexpr B (const B &x) : A<T> (1, x.foo ()) {}
+  B () : A<T> (1, 2) {}
+};
+struct C;
+struct D : B<C> {};
+void bar (D);
+
+void
+baz (D x)
+{
+  bar (x);
+}
--- gcc/testsuite/g++.dg/abi/comdat3.C.jj	2024-04-24 15:23:58.829885362 +0200
+++ gcc/testsuite/g++.dg/abi/comdat3.C	2024-04-24 15:24:46.088223353 +0200
@@ -0,0 +1,22 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2" }
+// { dg-final { scan-assembler "_ZN1M1SINS_1P1TELN1N1LE2EEC5Ev,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC1Ev,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC2Ev,comdat" } }
+
+namespace N { enum L { L1, L2, L3 } const O = L3; }
+namespace M {
+  using N::O;
+  using N::L;
+  template <typename, L = O>
+  struct S { constexpr S () {} };
+  namespace P {
+    struct T;
+    struct U { S<T> u; };
+    void foo () { U (); }
+  }
+  extern template class S<P::T>;
+}
+namespace p = M::P;
+template class M::S<p::T>;
--- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj	2024-04-24 13:43:17.887591040 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_0.C	2024-04-24 13:44:11.055849021 +0200
@@ -0,0 +1,13 @@
+// { dg-lto-do link }
+// { dg-lto-options { {-O1 -std=c++20 -flto}} }
+// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
+// { dg-require-linker-plugin "" }
+
+#define CONSTEXPR constexpr
+#include "pr113208.h"
+
+struct QualityValue;
+struct k : vector<QualityValue> {};
+
+void m(k);
+void n(k i) { m(i); }
--- gcc/testsuite/g++.dg/lto/pr113208_1.C.jj	2024-04-24 13:43:17.887591040 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_1.C	2024-04-24 13:44:11.055849021 +0200
@@ -0,0 +1,6 @@
+#define CONSTEXPR 
+#include "pr113208.h"
+
+struct QualityValue;
+vector<QualityValue> values1;
+vector<QualityValue> values{values1};
--- gcc/testsuite/g++.dg/lto/pr113208.h.jj	2024-04-24 13:43:17.887591040 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208.h	2024-04-24 13:44:11.054849035 +0200
@@ -0,0 +1,10 @@
+template <typename _Tp> struct _Vector_base {
+  int g() const;
+  _Vector_base(int, int);
+};
+template <typename _Tp>
+struct vector : _Vector_base<_Tp> {
+  CONSTEXPR vector(const vector &__x)
+      : _Vector_base<_Tp>(1, __x.g()) {}
+ vector() : _Vector_base<_Tp>(1, 2) {}
+};


	Jakub


  reply	other threads:[~2024-04-24 16:16 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17  7:42 [PATCH] c++: " Jakub Jelinek
2024-04-17  9:04 ` Jan Hubicka
2024-04-17 12:32   ` Jakub Jelinek
2024-04-17 13:26     ` Jan Hubicka
2024-04-17 14:07       ` Jakub Jelinek
2024-04-17 14:34         ` Jan Hubicka
2024-04-17 14:39           ` Jakub Jelinek
2024-04-22 15:42 ` [PATCH] c++, v2: " Jakub Jelinek
2024-04-23  3:14   ` Jason Merrill
2024-04-23 16:04     ` Jakub Jelinek
2024-04-24  9:16       ` Jonathan Wakely
2024-04-24 16:16         ` Jakub Jelinek [this message]
2024-04-24 22:39           ` [PATCH] c++, v3: " Jason Merrill
2024-04-24 22:47             ` Jakub Jelinek
2024-04-25  0:43               ` Jason Merrill
2024-04-25 12:02                 ` [PATCH] c++, v4: " Jakub Jelinek
2024-04-25 14:22                   ` Jakub Jelinek
2024-04-25 15:30                     ` Jason Merrill
2024-04-25 18:42                       ` [PATCH] c++, v5: " Jakub Jelinek
2024-05-09 18:20                       ` [PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [PR113208] Jakub Jelinek
2024-05-09 18:58                         ` Marek Polacek
2024-05-09 19:05                           ` Jakub Jelinek
2024-05-10 19:59                         ` Jason Merrill
2024-05-13 10:19                           ` Jakub Jelinek
2024-05-14 22:20                             ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zikwc9BTXQErR++G@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=jh@suse.cz \
    --cc=jwakely@redhat.com \
    --cc=ppalka@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).