| Message ID | 01020191ec70fbc0-12b70323-086b-4394-953d-e93afaae5d84-000000@eu-west-1.amazonses.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Don't mix timevar_start and auto_cond_timevar for TV_NAME_LOOKUP [PR116681] | expand |
On 9/13/24 1:31 PM, Simon Martin wrote: > We currently ICE upon the following testcase when using -ftime-report > > === cut here === > template < int> using __conditional_t = int; > template < typename _Iter > > concept random_access_iterator = requires { new _Iter; }; > template < typename _Iterator > > struct reverse_iterator { > using iterator_concept = > __conditional_t< random_access_iterator< _Iterator>>; > }; > void RemoveBottom() { > int iter; > for (reverse_iterator< int > iter;;) > ; > } > === cut here === > > The problem is that qualified_namespace_lookup does a plain start() of > the TV_NAME_LOOKUP timer (that asserts that the timer is not already > started). However this timer has already been cond_start()'d in the call > stack - by pushdecl - so the assert fails. > > This patch simply ensures that we always conditionally start this timer > (which is done in all other places that use it). OK, thanks. > Successfully tested on x86_64-pc-linux-gnu. > > PR c++/116681 > > gcc/cp/ChangeLog: > > * name-lookup.cc (qualified_namespace_lookup): Use an > auto_cond_timer instead of using timevar_start and timevar_stop. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-pr116681.C: New test. > > --- > gcc/cp/name-lookup.cc | 3 +-- > .../g++.dg/cpp2a/concepts-pr116681.C | 20 +++++++++++++++++++ > 2 files changed, 21 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index bfe17b7cb2f..c7a693e02d5 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -7421,10 +7421,9 @@ tree lookup_qualified_name (tree t, const char *p, LOOK_want w, bool c) > static bool > qualified_namespace_lookup (tree scope, name_lookup *lookup) > { > - timevar_start (TV_NAME_LOOKUP); > + auto_cond_timevar tv (TV_NAME_LOOKUP); > query_oracle (lookup->name); > bool found = lookup->search_qualified (ORIGINAL_NAMESPACE (scope)); > - timevar_stop (TV_NAME_LOOKUP); > return found; > } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C > new file mode 100644 > index 00000000000..f1b47797f1e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C > @@ -0,0 +1,20 @@ > +// PR c++/116681 > +// { dg-do compile { target c++20 } } > +// { dg-additional-options "-ftime-report" } > +// { dg-allow-blank-lines-in-output 1 } > +// { dg-prune-output "Time variable" } > +// { dg-prune-output "k" } > +// { dg-prune-output "\[0-9\]+%" } > + > +template < int> using __conditional_t = int; > +template < typename _Iter > > +concept random_access_iterator = requires { new _Iter; }; > +template < typename _Iterator > struct reverse_iterator { > + using iterator_concept = __conditional_t< random_access_iterator< _Iterator >>; > +}; > +void RemoveBottom() > +{ > + int iter; > + for (reverse_iterator< int > iter;;) > + ; > +}
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index bfe17b7cb2f..c7a693e02d5 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -7421,10 +7421,9 @@ tree lookup_qualified_name (tree t, const char *p, LOOK_want w, bool c) static bool qualified_namespace_lookup (tree scope, name_lookup *lookup) { - timevar_start (TV_NAME_LOOKUP); + auto_cond_timevar tv (TV_NAME_LOOKUP); query_oracle (lookup->name); bool found = lookup->search_qualified (ORIGINAL_NAMESPACE (scope)); - timevar_stop (TV_NAME_LOOKUP); return found; } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C new file mode 100644 index 00000000000..f1b47797f1e --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C @@ -0,0 +1,20 @@ +// PR c++/116681 +// { dg-do compile { target c++20 } } +// { dg-additional-options "-ftime-report" } +// { dg-allow-blank-lines-in-output 1 } +// { dg-prune-output "Time variable" } +// { dg-prune-output "k" } +// { dg-prune-output "\[0-9\]+%" } + +template < int> using __conditional_t = int; +template < typename _Iter > +concept random_access_iterator = requires { new _Iter; }; +template < typename _Iterator > struct reverse_iterator { + using iterator_concept = __conditional_t< random_access_iterator< _Iterator >>; +}; +void RemoveBottom() +{ + int iter; + for (reverse_iterator< int > iter;;) + ; +}
We currently ICE upon the following testcase when using -ftime-report === cut here === template < int> using __conditional_t = int; template < typename _Iter > concept random_access_iterator = requires { new _Iter; }; template < typename _Iterator > struct reverse_iterator { using iterator_concept = __conditional_t< random_access_iterator< _Iterator>>; }; void RemoveBottom() { int iter; for (reverse_iterator< int > iter;;) ; } === cut here === The problem is that qualified_namespace_lookup does a plain start() of the TV_NAME_LOOKUP timer (that asserts that the timer is not already started). However this timer has already been cond_start()'d in the call stack - by pushdecl - so the assert fails. This patch simply ensures that we always conditionally start this timer (which is done in all other places that use it). Successfully tested on x86_64-pc-linux-gnu. PR c++/116681 gcc/cp/ChangeLog: * name-lookup.cc (qualified_namespace_lookup): Use an auto_cond_timer instead of using timevar_start and timevar_stop. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-pr116681.C: New test. --- gcc/cp/name-lookup.cc | 3 +-- .../g++.dg/cpp2a/concepts-pr116681.C | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr116681.C