c++: comptypes ICE with BOUND_TEMPLATE_TEMPLATE_PARMs [PR107539]

  Here the two BOUND_TEMPLATE_TEMPLATE_PARMs written as C<decltype(t)> end
up having the same TYPE_CANONICAL since the ctp_table (which interns the
canonical form of a template type parameter) doesn't set the
comparing_specializations flag which controls how PARM_DECLs from
different DECL_CONTEXTs compare equal.

Later (from spec_hasher::equal for the two specializations of i) we end
up calling comptypes on these two types with comparing_specializations
set, which notices their TYPE_CANONICAL is the same despite them no
longer structurally comparing equal (thanks to the flag) and so we ICE:

  internal compiler error: same canonical type node for different types
    'C<decltype (t)>' and 'C<decltype (t)>'

This suggests that we should be setting comparing_specializations from
ctp_hasher::equal as well.  But doing so introduces an ICE in
cpp2a/concepts-placeholder3.C:

  internal compiler error: canonical types differ for identical types
  'auto [requires ::same_as<<placeholder>, decltype(f::x)>]' and
  'auto [requires ::same_as<<placeholder>, decltype(g::x)>]'

since norm_hasher::equal doesn't set comparing_specializations.  I'm not
sure when excatly we need to set comparing_specializations given that it
controls three things (TYPENAME_TYPE equality/hashing and PARM_DECL
equality) but it seems to be the conservative choice to set the flag
whenever we have a global hash table that relies on structural equality
of trees.  To that end this patch sets comparing_specializations in
ctp_hasher and the normalization/satisfaction hashers.  This turns out
to be a performance win of about 2% in some concepts tests, probably
because improved TYPENAME_TYPE hashing enabled by the flag.

Bootstrapped and regtested on x86_64-pc-linux-gnu, deos this look OK for
trunk?

	PR c++/107539

gcc/cp/ChangeLog:

	* constraint.cc (norm_hasher::hash, norm_hasher::equal): Set
	comparing_specializations.
	(sat_hasher::hash, sat_hasher::equal): Likewise.
	* cp-tree.h (atom_hasher::hash, atom_hasher::equal): Likewise.
	* pt.cc (ctp_hasher::hash, ctp_hasher::equal): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/template/canon-type-19.C: New test.
---
 gcc/cp/constraint.cc                          | 18 +++++++++++++++---
 gcc/cp/cp-tree.h                              | 10 ++++++++--
 gcc/cp/pt.cc                                  |  7 ++++++-
 gcc/testsuite/g++.dg/template/canon-type-19.C | 18 ++++++++++++++++++
 4 files changed, 47 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/canon-type-19.C
  

On 12/1/22 15:57, Patrick Palka wrote:
> Here the two BOUND_TEMPLATE_TEMPLATE_PARMs written as C<decltype(t)> end
> up having the same TYPE_CANONICAL since the ctp_table (which interns the
> canonical form of a template type parameter) doesn't set the
> comparing_specializations flag which controls how PARM_DECLs from
> different DECL_CONTEXTs compare equal.
> 
> Later (from spec_hasher::equal for the two specializations of i) we end
> up calling comptypes on these two types with comparing_specializations
> set, which notices their TYPE_CANONICAL is the same despite them no
> longer structurally comparing equal (thanks to the flag) and so we ICE:
> 
>    internal compiler error: same canonical type node for different types
>      'C<decltype (t)>' and 'C<decltype (t)>'
> 
> This suggests that we should be setting comparing_specializations from
> ctp_hasher::equal as well.  But doing so introduces an ICE in
> cpp2a/concepts-placeholder3.C:
> 
>    internal compiler error: canonical types differ for identical types
>    'auto [requires ::same_as<<placeholder>, decltype(f::x)>]' and
>    'auto [requires ::same_as<<placeholder>, decltype(g::x)>]'
> 
> since norm_hasher::equal doesn't set comparing_specializations.  I'm not
> sure when excatly we need to set comparing_specializations given that it
> controls three things (TYPENAME_TYPE equality/hashing and PARM_DECL
> equality) but it seems to be the conservative choice to set the flag
> whenever we have a global hash table that relies on structural equality
> of trees.  To that end this patch sets comparing_specializations in
> ctp_hasher and the normalization/satisfaction hashers.  This turns out
> to be a performance win of about 2% in some concepts tests, probably
> because improved TYPENAME_TYPE hashing enabled by the flag.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, deos this look OK for
> trunk?

OK.

> 	PR c++/107539
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (norm_hasher::hash, norm_hasher::equal): Set
> 	comparing_specializations.
> 	(sat_hasher::hash, sat_hasher::equal): Likewise.
> 	* cp-tree.h (atom_hasher::hash, atom_hasher::equal): Likewise.
> 	* pt.cc (ctp_hasher::hash, ctp_hasher::equal): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/canon-type-19.C: New test.
> ---
>   gcc/cp/constraint.cc                          | 18 +++++++++++++++---
>   gcc/cp/cp-tree.h                              | 10 ++++++++--
>   gcc/cp/pt.cc                                  |  7 ++++++-
>   gcc/testsuite/g++.dg/template/canon-type-19.C | 18 ++++++++++++++++++
>   4 files changed, 47 insertions(+), 6 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/canon-type-19.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index ab0f66b3d7e..37eae03afdb 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -715,14 +715,20 @@ struct norm_hasher : ggc_ptr_hash<norm_entry>
>   {
>     static hashval_t hash (norm_entry *e)
>     {
> -    hashval_t hash = iterative_hash_template_arg (e->tmpl, 0);
> -    return iterative_hash_template_arg (e->args, hash);
> +    ++comparing_specializations;
> +    hashval_t val = iterative_hash_template_arg (e->tmpl, 0);
> +    val = iterative_hash_template_arg (e->args, val);
> +    --comparing_specializations;
> +    return val;
>     }
>   
>     static bool equal (norm_entry *e1, norm_entry *e2)
>     {
> -    return e1->tmpl == e2->tmpl
> +    ++comparing_specializations;
> +    bool eq = e1->tmpl == e2->tmpl
>         && template_args_equal (e1->args, e2->args);
> +    --comparing_specializations;
> +    return eq;
>     }
>   };
>   
> @@ -2530,6 +2536,9 @@ struct sat_hasher : ggc_ptr_hash<sat_entry>
>   {
>     static hashval_t hash (sat_entry *e)
>     {
> +    auto cso = make_temp_override (comparing_specializations);
> +    ++comparing_specializations;
> +
>       if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e->atom))
>         {
>   	/* Atoms with instantiated mappings are built during satisfaction.
> @@ -2564,6 +2573,9 @@ struct sat_hasher : ggc_ptr_hash<sat_entry>
>   
>     static bool equal (sat_entry *e1, sat_entry *e2)
>     {
> +    auto cso = make_temp_override (comparing_specializations);
> +    ++comparing_specializations;
> +
>       if (ATOMIC_CONSTR_MAP_INSTANTIATED_P (e1->atom)
>   	!= ATOMIC_CONSTR_MAP_INSTANTIATED_P (e2->atom))
>         return false;
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 548b533266a..addd26ea077 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8418,12 +8418,18 @@ struct atom_hasher : default_hash_traits<tree>
>   {
>     static hashval_t hash (tree t)
>     {
> -    return hash_atomic_constraint (t);
> +    ++comparing_specializations;
> +    hashval_t val = hash_atomic_constraint (t);
> +    --comparing_specializations;
> +    return val;
>     }
>   
>     static bool equal (tree t1, tree t2)
>     {
> -    return atomic_constraints_identical_p (t1, t2);
> +    ++comparing_specializations;
> +    bool eq = atomic_constraints_identical_p (t1, t2);
> +    --comparing_specializations;
> +    return eq;
>     }
>   };
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 08de273a900..31691618d1b 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -4492,18 +4492,23 @@ struct ctp_hasher : ggc_ptr_hash<tree_node>
>   {
>     static hashval_t hash (tree t)
>     {
> +    ++comparing_specializations;
>       tree_code code = TREE_CODE (t);
>       hashval_t val = iterative_hash_object (code, 0);
>       val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val);
>       val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val);
>       if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM)
>         val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val);
> +    --comparing_specializations;
>       return val;
>     }
>   
>     static bool equal (tree t, tree u)
>     {
> -    return comptypes (t, u, COMPARE_STRUCTURAL);
> +    ++comparing_specializations;
> +    bool eq = comptypes (t, u, COMPARE_STRUCTURAL);
> +    --comparing_specializations;
> +    return eq;
>     }
>   };
>   
> diff --git a/gcc/testsuite/g++.dg/template/canon-type-19.C b/gcc/testsuite/g++.dg/template/canon-type-19.C
> new file mode 100644
> index 00000000000..06b0f08931d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/canon-type-19.C
> @@ -0,0 +1,18 @@
> +// PR c++/107539
> +// { dg-do compile { target c++11 } }
> +
> +template<class T>
> +struct i { };
> +
> +template<template<class> class C>
> +struct a {
> +  template<class T>
> +  void f(T t) {
> +    i<C<decltype(t)>> i1;
> +  }
> +
> +  template<class T>
> +  void g(T t) {
> +    i<C<decltype(t)>> i2;
> +  }
> +};