* c++: inter-cluster import order [PR 99283]
@ 2021-04-01 12:36 Nathan Sidwell
0 siblings, 0 replies; only message in thread
From: Nathan Sidwell @ 2021-04-01 12:36 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]
I finally managed to reduce the testcase without hitting other bugs.
This problem is caused by discovering a duplicate in the middle of
reading in the entity in question. I had thougt the import seeding at
the beginning of a cluster prevented that, but it is insufficient.
Specifically an earlier cluster in the same module can cause the import
of a duplicate. Although clusters within a module are well-ordered,
there is no ordering between clusters of one module and clusters of
another module. And thus we can get duplicate declaration loops. This
prevents the problem by also seeding references to earlier clusters in
the same module. As the FIXME notes, it is sufficient to reference a
single entity in any particular earlier cluster, plus, we also could
determine the implicit dependencies and prune that seeding even further.
I do not do that -- it decrease the loading that will happen, but
would reduce the serialization size. As ever, let's get correctness first.
PR c++/99283
gcc/cp/
* module.cc (trees_out::decl_node): Adjust importedness reference
assert.
(module_state::intercluster_seed): New. Seed both imports and
inter-cluster references. Broken out of ...
(module_state::write_cluster): ... here. Call it.
gcc/testsuite/
* g++.dg/modules/pr99283-6.h: New.
* g++.dg/modules/pr99283-6_a.H: New.
* g++.dg/modules/pr99283-6_b.H: New.
* g++.dg/modules/pr99283-6_c.C: New.
* g++.dg/modules/hdr-init-1_c.C: Adjust scan.
* g++.dg/modules/indirect-3_c.C: Adjust scan.
* g++.dg/modules/indirect-4_c.C: Adjust scan.
* g++.dg/modules/lambda-3_b.C: Adjust scan.
* g++.dg/modules/late-ret-3_c.C: Adjust scan.
* g++.dg/modules/pr99425-1_b.H: Adjust scan.
* g++.dg/modules/pr99425-1_c.C: Adjust scan.
--
Nathan Sidwell
[-- Attachment #2: pr99283-6.diff --]
[-- Type: text/x-patch, Size: 15237 bytes --]
diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc
index fab6b573d24..c87ddd16a80 100644
--- c/gcc/cp/module.cc
+++ w/gcc/cp/module.cc
@@ -3570,6 +3570,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
unsigned, unsigned *crc_ptr);
bool read_namespaces (unsigned);
+ void intercluster_seed (trees_out &sec, unsigned index, depset *dep);
unsigned write_cluster (elf_out *to, depset *depsets[], unsigned size,
depset::hash &, unsigned *counts, unsigned *crc_ptr);
bool read_cluster (unsigned snum);
@@ -8548,8 +8549,7 @@ trees_out::decl_node (tree decl, walk_kind ref)
gcc_checking_assert (index == ~import_entity_index (decl));
#if CHECKING_P
- if (importedness)
- gcc_assert (!import == (importedness < 0));
+ gcc_assert (!import || importedness >= 0);
#endif
i (tt_entity);
u (import);
@@ -14419,7 +14419,33 @@ enum ct_bind_flags
cbf_wrapped = 0x8, /* ... that is wrapped. */
};
-/* Write the cluster of depsets in SCC[0-SIZE). */
+/* DEP belongs to a different cluster, seed it to prevent
+ unfortunately timed duplicate import. */
+// FIXME: QOI For inter-cluster references we could just only pick
+// one entity from an earlier cluster. Even better track
+// dependencies between earlier clusters
+
+void
+module_state::intercluster_seed (trees_out &sec, unsigned index_hwm, depset *dep)
+{
+ if (dep->is_import ()
+ || dep->cluster < index_hwm)
+ {
+ tree ent = dep->get_entity ();
+ if (!TREE_VISITED (ent))
+ {
+ sec.tree_node (ent);
+ dump (dumper::CLUSTER)
+ && dump ("Seeded %s %N",
+ dep->is_import () ? "import" : "intercluster", ent);
+ }
+ }
+}
+
+/* Write the cluster of depsets in SCC[0-SIZE).
+ dep->section -> section number
+ dep->cluster -> entity number
+ */
unsigned
module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
@@ -14431,6 +14457,7 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
trees_out sec (to, this, table, table.section);
sec.begin ();
+ unsigned index_lwm = counts[MSC_entities];
/* Determine entity numbers, mark for writing. */
dump (dumper::CLUSTER) && dump ("Cluster members:") && (dump.indent (), true);
@@ -14484,10 +14511,10 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
}
dump (dumper::CLUSTER) && (dump.outdent (), true);
- /* Ensure every imported decl is referenced before we start
- streaming. This ensures that we never encounter the situation
- where this cluster instantiates some implicit member that
- importing some other decl causes to be instantiated. */
+ /* Ensure every out-of-cluster decl is referenced before we start
+ streaming. We must do both imports *and* earlier clusters,
+ because the latter could reach into the former and cause a
+ duplicate loop. */
sec.set_importing (+1);
for (unsigned ix = 0; ix != size; ix++)
{
@@ -14505,30 +14532,14 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
depset *bind = dep->deps[ix];
if (bind->get_entity_kind () == depset::EK_USING)
bind = bind->deps[1];
- if (bind->is_import ())
- {
- tree import = bind->get_entity ();
- if (!TREE_VISITED (import))
- {
- sec.tree_node (import);
- dump (dumper::CLUSTER)
- && dump ("Seeded import %N", import);
- }
- }
+
+ intercluster_seed (sec, index_lwm, bind);
}
/* Also check the namespace itself. */
dep = dep->deps[0];
}
- if (dep->is_import ())
- {
- tree import = dep->get_entity ();
- if (!TREE_VISITED (import))
- {
- sec.tree_node (import);
- dump (dumper::CLUSTER) && dump ("Seeded import %N", import);
- }
- }
+ intercluster_seed (sec, index_lwm, dep);
}
}
sec.tree_node (NULL_TREE);
diff --git c/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C w/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C
index efcc4854314..2a103c3d0bb 100644
--- c/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C
+++ w/gcc/testsuite/g++.dg/modules/hdr-init-1_c.C
@@ -17,8 +17,8 @@ int main ()
// { dg-final { scan-lang-dump-times {Reading 1 initializers} 2 module } }
-// { dg-final { scan-lang-dump {Read:-1's named merge key \(new\) var_decl:'::var'} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(new\) var_decl:'::var'} module } }
// { dg-final { scan-lang-dump-times {Reading definition var_decl '::var@[^\n]*/hdr-init-1_a.H:1'} 2 module } }
-// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) var_decl:'::var'} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) var_decl:'::var'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/indirect-3_c.C w/gcc/testsuite/g++.dg/modules/indirect-3_c.C
index 9c5cb230ad2..ec2fc768373 100644
--- c/gcc/testsuite/g++.dg/modules/indirect-3_c.C
+++ w/gcc/testsuite/g++.dg/modules/indirect-3_c.C
@@ -19,6 +19,6 @@ int main ()
// { dg-final { scan-lang-dump-not {Instantiation:-[0-9]* function_decl:'::foo::X@foo:.::frob@.()<0x0>'} module } }
// { dg-final { scan-lang-dump {Lazily binding '::bar@bar:.::toto'@'bar' section:} module } }
-// { dg-final { scan-lang-dump {>Loading entity foo\[1\] section:1} module } }
+// { dg-final { scan-lang-dump {>Loading entity foo\[.\] section:1} module } }
// { dg-final { scan-lang-dump {Imported:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } }
// { dg-final { scan-lang-dump {Reading definition type_decl '::foo@foo:.::TPL@bar:.<0x0>'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/indirect-4_c.C w/gcc/testsuite/g++.dg/modules/indirect-4_c.C
index 7efcc115e71..d55a2216fb3 100644
--- c/gcc/testsuite/g++.dg/modules/indirect-4_c.C
+++ w/gcc/testsuite/g++.dg/modules/indirect-4_c.C
@@ -10,7 +10,7 @@ int main ()
}
// { dg-final { scan-lang-dump {Lazily binding '::bar@bar:.::quux'@'bar' section:} module } }
-// { dg-final { scan-lang-dump {>Loading entity foo\[2\] section:1} module } }
+// { dg-final { scan-lang-dump {>Loading entity foo\[.\] section:1} module } }
// { dg-final { scan-lang-dump {Imported:-[0-9]* template_decl:'::foo@foo:.::template TPL@foo:.'@foo} module } }
// { dg-final { scan-lang-dump {Reading definition function_decl '::foo@foo:.::TPL@bar:.<0x1>::frob@bar:.<0x2>'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/lambda-3_b.C w/gcc/testsuite/g++.dg/modules/lambda-3_b.C
index 25a418bb44a..17afd967745 100644
--- c/gcc/testsuite/g++.dg/modules/lambda-3_b.C
+++ w/gcc/testsuite/g++.dg/modules/lambda-3_b.C
@@ -4,6 +4,6 @@
import "lambda-3_a.H";
// { dg-final { scan-lang-dump-not {merge key \(new\)} module } }
-// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::template ._anon_0<#unnamed#>'} module } }
-// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::._anon_2'} module } }
-// { dg-final { scan-lang-dump {Read -1\[0\] matched attached decl '::._anon_1'} module } }
+// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::template ._anon_0<#unnamed#>'} module } }
+// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::._anon_2'} module } }
+// { dg-final { scan-lang-dump {Read -[0-9]*\[0\] matched attached decl '::._anon_1'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/late-ret-3_c.C w/gcc/testsuite/g++.dg/modules/late-ret-3_c.C
index fae956542f9..8ab23a9e338 100644
--- c/gcc/testsuite/g++.dg/modules/late-ret-3_c.C
+++ w/gcc/testsuite/g++.dg/modules/late-ret-3_c.C
@@ -19,4 +19,4 @@ int main ()
return 0;
}
-// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) template_decl:'::template Foo'\n Deduping '::template Foo@[^\n]*/late-ret-3_a.H:.'\n} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template Foo'\n Deduping '::template Foo@[^\n]*/late-ret-3_a.H:.'\n} module } }
diff --git c/gcc/testsuite/g++.dg/modules/pr99283-6.h w/gcc/testsuite/g++.dg/modules/pr99283-6.h
new file mode 100644
index 00000000000..383b66c081e
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99283-6.h
@@ -0,0 +1,23 @@
+template<bool, typename, typename>
+struct conditional;
+
+template<typename> struct incrementable_traits;
+
+template<typename _Iter, typename _Tp>
+struct __iter_traits_impl;
+
+class __max_diff_type;
+
+template<typename _Iter>
+concept weakly_incrementable
+ = __is_same (__iter_traits_impl<_Iter, incrementable_traits<_Iter>>,
+ __max_diff_type);
+
+template<typename _Iterator>
+class reverse_iterator
+{
+public:
+ using iterator_concept
+ = typename conditional<weakly_incrementable<_Iterator>,
+ int, int>::type;
+};
diff --git c/gcc/testsuite/g++.dg/modules/pr99283-6_a.H w/gcc/testsuite/g++.dg/modules/pr99283-6_a.H
new file mode 100644
index 00000000000..176ad9e9e8c
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99283-6_a.H
@@ -0,0 +1,33 @@
+// { dg-additional-options {-std=c++2a -fmodule-header} }
+// { dg-module-cmi {}
+
+#include "pr99283-6.h"
+
+template<typename _IteratorL, typename _IteratorR>
+constexpr bool
+ operator<(const reverse_iterator<_IteratorL>& __x,
+ const reverse_iterator<_IteratorR>& __y);
+
+template<typename _Tp>
+ struct numeric_limits;
+
+class __max_size_type
+{
+public:
+ template<typename _Tp>
+ constexpr
+ __max_size_type(_Tp __i) noexcept
+ : _M_val(__i), _M_msb(__i < 0)
+ { }
+
+ using __rep = __UINT64_TYPE__;
+private:
+ __rep _M_val = 0;
+ unsigned _M_msb:1 = 0;
+};
+
+class __max_diff_type
+{
+private:
+ __max_size_type _M_rep = 0;
+};
diff --git c/gcc/testsuite/g++.dg/modules/pr99283-6_b.H w/gcc/testsuite/g++.dg/modules/pr99283-6_b.H
new file mode 100644
index 00000000000..123f3534673
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99283-6_b.H
@@ -0,0 +1,164 @@
+// { dg-additional-options {-std=c++2a -fmodule-header} }
+// { dg-module-cmi {}
+
+#include "pr99283-6.h"
+
+template<typename _Tp>
+void __addressof(_Tp& __r) ;
+
+template<typename _Tp, _Tp __v>
+struct integral_constant
+{
+ static constexpr _Tp value = __v;
+};
+
+template<typename _Tp, _Tp __v>
+constexpr _Tp integral_constant<_Tp, __v>::value;
+
+typedef integral_constant<bool, true> true_type;
+typedef integral_constant<bool, false> false_type;
+
+template<typename _B1, typename _B2>
+struct __and_
+ : public conditional<_B1::value, _B2, _B1>::type
+{ };
+
+template<typename _From, typename _To>
+struct is_convertible
+ : public true_type
+{ };
+
+template<bool, typename _Tp = void>
+struct enable_if
+{ };
+
+template<typename _Tp>
+struct enable_if<true, _Tp>
+{ typedef _Tp type; };
+
+template<bool _Cond, typename _Tp = void>
+using __enable_if_t = typename enable_if<_Cond, _Tp>::type;
+
+template<typename A, typename B>
+using _Require = __enable_if_t<__and_<A, B>::value>;
+
+template<bool _Cond, typename _Iftrue, typename _Iffalse>
+struct conditional
+{ typedef _Iftrue type; };
+
+template<typename> struct iterator_traits;
+
+
+template<typename _IteratorL, typename _IteratorR>
+constexpr bool
+ operator!=(const reverse_iterator<_IteratorL>& __x,
+ const reverse_iterator<_IteratorR>& __y);
+
+typedef __INT64_TYPE__ int64_t;
+typedef int64_t intmax_t;
+
+template<intmax_t _Num>
+struct ratio
+{
+};
+
+namespace chrono
+{
+template<typename _Rep>
+struct duration;
+
+template<typename _ToDur, typename _Rep>
+constexpr _ToDur
+ duration_cast(const duration<_Rep>& __d)
+{
+ typedef typename _ToDur::rep __to_rep;
+ return _ToDur(static_cast<__to_rep>(static_cast<intmax_t>(__d.count())));
+}
+
+template<typename _Rep>
+struct duration
+{
+
+public:
+ using rep = _Rep;
+
+ constexpr duration() = default;
+
+ duration(const duration&) = default;
+
+ template<typename _Rep2, typename
+ = _Require<is_convertible<const _Rep2&, rep>,
+ true_type>>
+ constexpr explicit duration(const _Rep2& __rep)
+ : __r (__rep) {}
+
+ ~duration() = default;
+ duration& operator=(const duration&) = default;
+
+ rep count() const;
+
+private:
+ rep __r;
+};
+
+using seconds = duration<int64_t>;
+
+template<typename _Clock, typename _Dur>
+struct time_point
+{
+ typedef _Dur duration;
+
+ duration time_since_epoch() const;
+
+private:
+ duration __d;
+};
+
+struct system_clock
+{
+ typedef chrono::seconds duration;
+
+ typedef chrono::time_point<system_clock, duration> time_point;
+
+ static void
+ to_time_t(const time_point& __t) noexcept
+ {
+ duration_cast<chrono::seconds>
+ (__t.time_since_epoch()).count();
+ }
+};
+
+}
+
+template<typename>
+class allocator;
+
+template<typename _ForwardIterator>
+constexpr void
+ __destroy(_ForwardIterator __first, _ForwardIterator __last)
+{
+ for (; __first != __last; ++__first)
+ __addressof (*__first);
+}
+
+template<typename _Alloc>
+struct allocator_traits
+{
+private:
+ template<typename _Alloc2, typename _Tp>
+ static constexpr void
+ _S_destroy(_Alloc2&, _Tp* __p, ...)
+ noexcept
+ {
+ __destroy(__p);
+ }
+};
+
+template<typename _Tp>
+struct allocator_traits<allocator<_Tp>>
+{
+ using value_type = _Tp;
+ using pointer = _Tp*;
+};
+
+import "pr99283-6_a.H";
diff --git c/gcc/testsuite/g++.dg/modules/pr99283-6_c.C w/gcc/testsuite/g++.dg/modules/pr99283-6_c.C
new file mode 100644
index 00000000000..9492554617d
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99283-6_c.C
@@ -0,0 +1,10 @@
+// { dg-additional-options {-std=c++2a -fmodules-ts} }
+
+import "pr99283-6_b.H";
+
+template<typename _Alloc>
+struct __allocated_ptr
+{
+ using value_type = allocator_traits<_Alloc>;
+};
+
diff --git c/gcc/testsuite/g++.dg/modules/pr99425-1_b.H w/gcc/testsuite/g++.dg/modules/pr99425-1_b.H
index 98303a0c687..53d28b4ef5e 100644
--- c/gcc/testsuite/g++.dg/modules/pr99425-1_b.H
+++ w/gcc/testsuite/g++.dg/modules/pr99425-1_b.H
@@ -15,5 +15,5 @@ inline void widget (Cont parm)
ssize (parm);
}
-// { dg-final { scan-lang-dump {Read:-[0-9]*'s alias spec merge key \(new\) type_decl:'::make_signed_t'\n ... Read:-[0-9]*'s type spec merge key \(new\) type_decl:'::make_signed'\n Read:-1's named merge key \(matched\) template_decl:'::template ssize'} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s alias spec merge key \(new\) type_decl:'::make_signed_t'\n ... Read:-[0-9]*'s type spec merge key \(new\) type_decl:'::make_signed'\n Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template ssize'} module } }
diff --git c/gcc/testsuite/g++.dg/modules/pr99425-1_c.C w/gcc/testsuite/g++.dg/modules/pr99425-1_c.C
index 28ef3a1ff30..77a35a88b39 100644
--- c/gcc/testsuite/g++.dg/modules/pr99425-1_c.C
+++ w/gcc/testsuite/g++.dg/modules/pr99425-1_c.C
@@ -7,5 +7,5 @@ void frob (Cont parm)
ssize (parm);
}
-// { dg-final { scan-lang-dump {Read:-1's named merge key \(new\) template_decl:'::template ssize'} module } }
-// { dg-final { scan-lang-dump {Read:-1's named merge key \(matched\) template_decl:'::template ssize'} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(new\) template_decl:'::template ssize'} module } }
+// { dg-final { scan-lang-dump {Read:-[0-9]*'s named merge key \(matched\) template_decl:'::template ssize'} module } }
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-04-01 12:36 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01 12:36 c++: inter-cluster import order [PR 99283] Nathan Sidwell
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).