On Mon, Nov 2, 2020 at 10:11 AM Jason Merrill <ja...@redhat.com> wrote:
>
> On 11/2/20 9:03 AM, Patrick Palka wrote:
> > On Tue, 27 Oct 2020, Patrick Palka wrote:
> >
> >> The adoption of P2104 means we can memoize the result of satisfaction
> >> indefinitely and no longer have to clear the satisfaction caches on
> >> various events that would affect satisfaction. To that end, this patch
> >> removes clear_satisfaction_cache and adjusts its callers appropriately.
> >>
> >> This provides a massive reduction in compile time and memory use in some
> >> cases. For example, on the libstdc++ test std/ranges/adaptor/join.cc,
> >> compile time and memory usage drops nearly 75%, from 7.5s/770MB to
> >> 2s/230MB, with a --enable-checking=release compiler.
> >>
> >> [ This patch depends on
> >>
> >> c++: Check constraints only for candidate conversion functions.
> >>
> >> Without it, many of the libstdc++ range adaptor tests fail due to
> >> us now indefinitely memoizing a bogus satisfaction result caused by
> >> checking the constraints of a conversion function when we weren't
> >> supposed to, which led to a "use of foo_view::end() before deduction
> >> of auto" SFINAE error. This went unnoticed without this patch because
> >> by the time we needed this satisfaction result again, we had cleared
> >> the satisfaction cache and finished deducing the return type of
> >> foo_view::end(), so on subsequent tries we computed the correct
> >> satisfaction result. ]
> >
> > With the library-side workaround r11-4584 for the range adaptor test
> > failures now committed, the mentioned frontend patch about candidate
> > conversion functions is no longer a prerequisite (and was not we want
> > anyway).
> >
> >>
> >> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> >> trunk (pending approval of the prerequisite patch)? Also tested on
> >> cmcstl2 and range-v3.
> >
> > Now successfully bootstrapped and regtested on top of r11-4584, and also
> > tested on cmcstl2 and range-v3. Does this look OK for trunk?
>
> OK. It would be nice to warn when a (now remembered) satisfaction value
> is determined by a class being incomplete. Maybe by checking whether
> we're in the middle of satisfaction when complete_type fails? Maybe
> only warn by defaul if the type is complete at EOF.
Thanks, committed as r11-4624. A warning like that sounds handy; I'll
look into implementing it for stage 1.
>
> >>
> >> gcc/cp/ChangeLog:
> >>
> >> * class.c (finish_struct_1): Don't call clear_satisfaction_cache.
> >> * constexpr.c (clear_cv_and_fold_caches): Likewise. Remove bool
> >> parameter.
> >> * constraint.cc (clear_satisfaction_cache): Remove definition.
> >> * cp-tree.h (clear_satisfaction_cache): Remove declaration.
> >> (clear_cv_and_fold_caches): Remove bool parameter.
> >> * typeck2.c (store_init_value): Remove argument to
> >> clear_cv_and_fold_caches.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> * g++.dg/cpp2a/concept-complete1.C: Delete ill-formed test.
> >> ---
> >> gcc/cp/class.c | 3 ---
> >> gcc/cp/constexpr.c | 6 ++----
> >> gcc/cp/constraint.cc | 9 ---------
> >> gcc/cp/cp-tree.h | 3 +--
> >> gcc/cp/typeck2.c | 2 +-
> >> gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C | 11 -----------
> >> 6 files changed, 4 insertions(+), 30 deletions(-)
> >> delete mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> >>
> >> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> >> index 26f996b7f4b..6c21682a3e5 100644
> >> --- a/gcc/cp/class.c
> >> +++ b/gcc/cp/class.c
> >> @@ -7472,9 +7472,6 @@ finish_struct_1 (tree t)
> >> /* Finish debugging output for this type. */
> >> rest_of_type_compilation (t, ! LOCAL_CLASS_P (t));
> >>
> >> - /* Recalculate satisfaction that might depend on completeness. */
> >> - clear_satisfaction_cache ();
> >> -
> >> if (TYPE_TRANSPARENT_AGGR (t))
> >> {
> >> tree field = first_field (t);
> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> >> index 7ebdd308dcd..ec60db4a44b 100644
> >> --- a/gcc/cp/constexpr.c
> >> +++ b/gcc/cp/constexpr.c
> >> @@ -7085,15 +7085,13 @@ clear_cv_cache (void)
> >> cv_cache->empty ();
> >> }
> >>
> >> -/* Dispose of the whole CV_CACHE, FOLD_CACHE, and satisfaction caches. */
> >> +/* Dispose of the whole CV_CACHE and FOLD_CACHE. */
> >>
> >> void
> >> -clear_cv_and_fold_caches (bool sat /*= true*/)
> >> +clear_cv_and_fold_caches ()
> >> {
> >> clear_cv_cache ();
> >> clear_fold_cache ();
> >> - if (sat)
> >> - clear_satisfaction_cache ();
> >> }
> >>
> >> /* Internal function handling expressions in templates for
> >> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> >> index 75457a2dd60..8c0111a6409 100644
> >> --- a/gcc/cp/constraint.cc
> >> +++ b/gcc/cp/constraint.cc
> >> @@ -2354,15 +2354,6 @@ save_satisfaction (tree constr, tree args, tree
> >> result)
> >> *slot = entry;
> >> }
> >>
> >> -void
> >> -clear_satisfaction_cache ()
> >> -{
> >> - if (sat_cache)
> >> - sat_cache->empty ();
> >> - if (decl_satisfied_cache)
> >> - decl_satisfied_cache->empty ();
> >> -}
> >> -
> >> /* A tool to help manage satisfaction caching in satisfy_constraint_r.
> >> Note the cache is only used when not diagnosing errors. */
> >>
> >> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> >> index 1ce20989e13..7a6efca6121 100644
> >> --- a/gcc/cp/cp-tree.h
> >> +++ b/gcc/cp/cp-tree.h
> >> @@ -7834,7 +7834,6 @@ extern tree evaluate_concept_check
> >> (tree, tsubst_flags_t);
> >> extern tree satisfy_constraint_expression (tree);
> >> extern bool constraints_satisfied_p (tree);
> >> extern bool constraints_satisfied_p (tree, tree);
> >> -extern void clear_satisfaction_cache ();
> >> extern bool* lookup_subsumption_result (tree, tree);
> >> extern bool save_subsumption_result (tree, tree, bool);
> >> extern tree find_template_parameters (tree, tree);
> >> @@ -7904,7 +7903,7 @@ extern bool var_in_maybe_constexpr_fn
> >> (tree);
> >> extern void explain_invalid_constexpr_fn (tree);
> >> extern vec<tree> cx_error_context (void);
> >> extern tree fold_sizeof_expr (tree);
> >> -extern void clear_cv_and_fold_caches (bool = true);
> >> +extern void clear_cv_and_fold_caches (void);
> >> extern tree unshare_constructor (tree
> >> CXX_MEM_STAT_INFO);
> >>
> >> /* An RAII sentinel used to restrict constexpr evaluation so that it
> >> diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
> >> index e259a4201be..445e2a211c8 100644
> >> --- a/gcc/cp/typeck2.c
> >> +++ b/gcc/cp/typeck2.c
> >> @@ -954,7 +954,7 @@ store_init_value (tree decl, tree init, vec<tree,
> >> va_gc>** cleanups, int flags)
> >> return split_nonconstant_init (decl, value);
> >>
> >> /* DECL may change value; purge caches. */
> >> - clear_cv_and_fold_caches (TREE_STATIC (decl));
> >> + clear_cv_and_fold_caches ();
> >>
> >> /* If the value is a constant, just put it in DECL_INITIAL. If DECL
> >> is an automatic variable, the middle end will turn this into a
> >> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> >> b/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> >> deleted file mode 100644
> >> index 63f36965f00..00000000000
> >> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-complete1.C
> >> +++ /dev/null
> >> @@ -1,11 +0,0 @@
> >> -// { dg-do compile { target c++20 } }
> >> -
> >> -template <class T> concept has_mem_type = requires { typename T::type; };
> >> -
> >> -template <has_mem_type T> int f () { return 0; }
> >> -template <class T> char f() { return 0; }
> >> -
> >> -struct A;
> >> -static_assert (sizeof (f<A>()) == 1);
> >> -struct A { typedef int type; };
> >> -static_assert (sizeof (f<A>()) > 1);
> >> --
> >> 2.29.0.rc0
> >>
> >>
> >
>
