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. > > 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. */ > >
