On Wed, Jul 17, 2024 at 04:31:21PM -0400, Jason Merrill wrote:
> On 7/7/24 9:29 AM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > Or should I include a testcase? I haven't reduced one from using the
> > full contents of C++23 <iostream> yet but I can do so if you prefer.
>
> Would adding -ftime-report to the pr99166 test reproduce the issue?
>
Unfortunately not, since lazy loading is not triggered in this test.
I've ended up just reducing a testcase anyway, here's a new version of
the patch with a test included (that fails beforehand and not after); OK
for trunk? (And even 14, actually?)
-- >8 --
While lazy loading, instantiation of pendings can sometimes recursively
perform name lookup and begin further lazy loading. When using the
'-ftime-report' functionality this causes ICEs as we could start an
already-running timer for the importing.
This patch fixes the issue by using the 'timevar_cond*' API instead to
support such recursive calls.
PR c++/115165
gcc/cp/ChangeLog:
* module.cc (lazy_load_binding): Use 'timevar_cond*' APIs.
(lazy_load_pendings): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/timevar-1_a.H: New test.
* g++.dg/modules/timevar-1_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/module.cc | 8 ++++----
gcc/testsuite/g++.dg/modules/timevar-1_a.H | 14 ++++++++++++++
gcc/testsuite/g++.dg/modules/timevar-1_b.C | 10 ++++++++++
3 files changed, 28 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/timevar-1_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/timevar-1_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d385b422168..69764fd772d 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19604,7 +19604,7 @@ lazy_load_binding (unsigned mod, tree ns, tree id,
binding_slot *mslot)
{
int count = errorcount + warningcount;
- timevar_start (TV_MODULE_IMPORT);
+ bool timer_running = timevar_cond_start (TV_MODULE_IMPORT);
/* Make sure lazy loading from a template context behaves as if
from a non-template context. */
@@ -19634,7 +19634,7 @@ lazy_load_binding (unsigned mod, tree ns, tree id,
binding_slot *mslot)
function_depth--;
- timevar_stop (TV_MODULE_IMPORT);
+ timevar_cond_stop (TV_MODULE_IMPORT, timer_running);
if (!ok)
fatal_error (input_location,
@@ -19673,7 +19673,7 @@ lazy_load_pendings (tree decl)
int count = errorcount + warningcount;
- timevar_start (TV_MODULE_IMPORT);
+ bool timer_running = timevar_cond_start (TV_MODULE_IMPORT);
bool ok = !recursive_lazy ();
if (ok)
{
@@ -19707,7 +19707,7 @@ lazy_load_pendings (tree decl)
function_depth--;
}
- timevar_stop (TV_MODULE_IMPORT);
+ timevar_cond_stop (TV_MODULE_IMPORT, timer_running);
if (!ok)
fatal_error (input_location, "failed to load pendings for %<%E%s%E%>",
diff --git a/gcc/testsuite/g++.dg/modules/timevar-1_a.H
b/gcc/testsuite/g++.dg/modules/timevar-1_a.H
new file mode 100644
index 00000000000..efd7f0ecced
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/timevar-1_a.H
@@ -0,0 +1,14 @@
+// PR c++/115165
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename> struct A { virtual ~A(); };
+struct B : A<char> {};
+struct C : B { C() {} };
+
+class D { C c; };
+void f(D);
+
+struct X {
+ friend void f(X);
+};
diff --git a/gcc/testsuite/g++.dg/modules/timevar-1_b.C
b/gcc/testsuite/g++.dg/modules/timevar-1_b.C
new file mode 100644
index 00000000000..645f01697eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/timevar-1_b.C
@@ -0,0 +1,10 @@
+// PR c++/115165
+// { dg-additional-options "-fmodules-ts -ftime-report" }
+// { dg-allow-blank-lines-in-output 1 }
+// { dg-prune-output "Time variable" }
+// { dg-prune-output "\[0-9\]+%" }
+// { dg-prune-output "TOTAL" }
+// { dg-prune-output "checks" }
+
+import "timevar-1_a.H";
+X x;
--
2.43.2
