Re: [PATCH] middle-end/114931 - type_hash_canon and structual equality types

[Jakub Jelinek <jakub@redhat.com>]

On Fri, May 03, 2024 at 08:04:18PM +0200, Martin Uecker wrote:
> A change that is not optimal but would avoid a lot of trouble is to
> only use the tag of the struct for computing a TYPE_CANONICAL, which
> could then be set already for incomplete types and never needs to
> change again. We would not differentiate between different struct
> types with the same tag for aliasing analysis, but in most cases
> I would expect different structs to have a different tag.

Having incompatible types have the same TYPE_CANONICAL would lead to wrong
code IMHO, while for aliasing purposes that might be conservative (though
not sure, the alias set computation is based on what types the element have
etc., so if the alias set is computed for say struct S { int s; }; and
then the same alias set used for struct S { long long a; double b; union {
short c; float d; } c; };, I think nothing good will come out of that),
but middle-end also uses TYPE_CANONICAL to see if types are the same,
say e.g. useless_type_conversion_p says that conversions from one
RECORD_TYPE to a different RECORD_TYPE are useless if they have the
same TYPE_CANONICAL.
  /* For aggregates we rely on TYPE_CANONICAL exclusively and require
     explicit conversions for types involving to be structurally
     compared types.  */
  else if (AGGREGATE_TYPE_P (inner_type)
           && TREE_CODE (inner_type) == TREE_CODE (outer_type))
    return TYPE_CANONICAL (inner_type)
           && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type);
So, if you have struct S { int s; } and struct S { short a, b; }; and
VIEW_CONVERT_EXPR between them, that VIEW_CONVERT_EXPR will be removed
as useless, etc.

BTW, the idea of lazily updating TYPE_CANONICAL is basically what I've
described as the option to update all the derived types where it would
pretty much do that for all TYPE_STRUCTURAL_EQUALITY_P types in the
hash table (see if they are derived from the type in question and recompute
the TYPE_CANONICAL after recomputing all the TYPE_CANONICAL of its base
types), except perhaps even more costly (if the trigger would be some
build_array_type/build_function_type/... function is called and found
a cached TYPE_STRUCTURAL_EQUALITY_P type).  Note also that
TYPE_STRUCTURAL_EQUALITY_P isn't the case just for the C23 types which
are marked that way when incomplete and later completed, but by various
other cases for types which will be permanently like that, so doing
expensive checks each time some build*_type* is called that refers
to those would be expensive.

	Jakub