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

[Richard Biener <rguenther@suse.de>]

> Am 03.05.2024 um 20:37 schrieb Martin Uecker <uecker@tugraz.at>:
> 
> Am Freitag, dem 03.05.2024 um 20:18 +0200 schrieb Jakub Jelinek:
>>> 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),
> 
> The C type systems requires us to form equivalence classes though.
> For example
> 
> int (*r)[1];
> int (*q)[];
> int (*p)[3];
> 
> need to be in the same equivalence class even though r and p are
> not compatible, while at the same time r and q and q and p
> are compatible.

TYPE_CANONICAL as used by the middle-end cannot express this but useless_type_conversion_p is directed and has similar behavior.  Note the dual-use for TBAA and compatibility was convenient but maybe we have to separate both since making the equivalence class for TBAA larger is more conservative while for compatibility it’s the other way around…

Richard 

> 
>> 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.
> 
> Maybe we could limit for purposes of computing TYPE_CANONICAL of derived
> types, e.g. TYPE_CANONICAL of structs stays the same with the transition
> from TYPE_STRUCT_EQUALITY to TYPE_CANONICAL but all the derived types
> remain stable.
> 
> Martin
> 
>> 
>> 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
>> 
>