On Mon, 10 Aug 2026, Patrick Palka wrote:
>
> On Mon, 10 Aug 2026, Jason Merrill wrote:
>
> > On 8/7/26 9:55 AM, Richard Biener wrote:
> > > From: Bernhard M. Wiedemann <[email protected]>
> > >
> > > typename_htab is written to a precompiled header. A hash table is
> > > streamed out slot array and all: gt_pch_nx() relocates the pointers
> > > inside the entries but leaves every entry in the slot it happened to
> > > occupy. typename_hasher hashed on the addresses of the scope and the
> > > fullname, so the slots were chosen from addresses that ASLR randomises
> > > in the process writing the header, and they no longer correspond to the
> > > hash of anything once the header has been read back at a different
> > > address.
> > >
> > > Lookups then find a restored TYPENAME_TYPE only when it happens to lie
> > > on the probe sequence of the slot the new hash points at, so most miss
> > > and build a duplicate, and which ones miss depends on the layout the
> > > writing process had. That makes a compile using a PCH differ from the
> > > same compile without one, and differ from itself between runs: the
> > > duplicates consume DECL_UIDs, every later DECL_UID shifts, and
> > > var-tracking hashes on DECL_UID, so .debug_loclists comes out different.
> > >
> > > Hash on TYPE_UID/DECL_UID and IDENTIFIER_HASH_VALUE instead, which the
> > > header preserves. The name replaces the fullname in the hash because a
> > > TEMPLATE_ID_EXPR fullname has no address-independent hash of its own;
> > > both are compared by equal() either way.
> >
> > We could handle TEMPLATE_ID_EXPR by using iterative_hash_template_arg
> > instead
> > of iterative_hash_object?
> >
> > But I suppose it's unlikely that we'd need a bunch of typenames that differ
> > only in template args, so OK either way.
>
> IIRC the PR65328 compile-time-hog testcase had a lot of TEMPLATE_ID_EXPR
> typenames, and indeed this patch as-is significantly regresses compile
> time of the PR65328#c6 testcase since it effectively undoes part of
> r13-1047-g343d83c7a89d0c. So I think we need to go with using
> iterative_hash_template_arg instead (with comparing_specializations
> set so that we produce distinct hashes for nested TYPENAME_TYPE).
>
> >
> > > Compiling qgstopologicalmesh.cpp of qgis 4.2.1, 4757 lookups in the
> > > table:
> > >
> > > no PCH 3046 misses, 3046 entries
> > > PCH, before 483-484 misses, 3135-3136 entries, varies per
> > > run
> > > PCH, after 394 misses, 3046 entries
> > >
> > > and the object file now matches the one built without a PCH, byte for
> > > byte, from any of eight independently generated headers.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > >
> > > OK?
> > >
> > > Thanks,
> > > Richard.
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > PR pch/124811
> > > * decl.cc (typename_hasher::hash): Hash the UIDs of the context
> > > and the name rather than their addresses.
> > >
> > > Assisted-by: Claude
> > > ---
> > > gcc/cp/decl.cc | 20 +++++++++++++-------
> > > 1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > > index b5d9ed05874..c66f59fef79 100644
> > > --- a/gcc/cp/decl.cc
> > > +++ b/gcc/cp/decl.cc
> > > @@ -4920,27 +4920,33 @@ struct typename_hasher : ggc_ptr_hash<tree_node>
> > > {
> > > typedef typename_info *compare_type;
> > > - /* Hash a TYPENAME_TYPE. */
> > > + /* Hash a TYPENAME_TYPE. This table goes into a precompiled header,
> > > which
> > > + moves everything it contains to a different address; entries keep
> > > the
> > > + slot they were put in, so hashing on the addresses of CONTEXT and
> > > NAME
> > > + would leave them unfindable once the header has been read back.
> > > Hash
> > > on
> > > + the UIDs instead, which the header preserves. */
>
> Since this is a common problem with GC'd hash tables and PCH and not
> specific to this hash table, I don't think this comment is necessary
> here FWIW. It'd be better suited in a more central piece of
> documentation such as in the gccint manual, if it's not already
> documented there.
ISTR seeing such comment in other places, so yes it's redundant.
Can you take the patch from here? I'd have to second-guess
which tree is the TEMPLATE_ID_EXPR while it's probably obvious to
you.
The PR identifies two other GC'd and PCH streamed hash tables that
hash pointers (but any incremental improvement is good!), which
are cp/tree.cc:list_hash_table and cp/constraint.cc:atom_cache
Thanks,
Richard.
> > > static hashval_t
> > > - hash (tree context, tree fullname)
> > > + hash (tree context, tree name)
> > > {
> > > hashval_t hash = 0;
> > > - hash = iterative_hash_object (context, hash);
> > > - hash = iterative_hash_object (fullname, hash);
> > > - return hash;
> > > + if (context)
> > > + hash = iterative_hash_hashval_t (TYPE_P (context) ? TYPE_UID
> > > (context)
> > > + : DECL_P (context) ? DECL_UID (context)
> > > + : 0, hash);
> > > + return iterative_hash_hashval_t (IDENTIFIER_HASH_VALUE (name), hash);
> > > }
> > > static hashval_t
> > > hash (const typename_info *ti)
> > > {
> > > - return typename_hasher::hash (ti->scope, ti->template_id);
> > > + return typename_hasher::hash (ti->scope, ti->name);
> > > }
> > > static hashval_t
> > > hash (tree t)
> > > {
> > > - return typename_hasher::hash (TYPE_CONTEXT (t),
> > > TYPENAME_TYPE_FULLNAME
> > > (t));
> > > + return typename_hasher::hash (TYPE_CONTEXT (t), TYPE_IDENTIFIER (t));
> > > }
> > > /* Compare two TYPENAME_TYPEs. */
> >
> >
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
