[Bug tree-optimization/97360] [11 Regression] ICE in range_on_exit

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97360

--- Comment #20 from rguenther at suse dot de <rguenther at suse dot de> ---
On October 16, 2020 5:46:28 PM GMT+02:00, amacleod at redhat dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97360
>
>--- Comment #19 from Andrew Macleod <amacleod at redhat dot com> ---
>(In reply to rguenther@suse.de from comment #18)
>> On October 16, 2020 4:17:43 PM GMT+02:00, amacleod at redhat dot com
>> 
>> >
>> >Yeah, I haven't tripped over it in ADA. This was a 512 byte quad on
>the
>> >powerpc
>> >target.. so far the only place I have seen this issue.
>> >
>> >      tree xi_uns_type = make_unsigned_type (512);
>> >      vector_quad_type_node = build_distinct_type_copy
>(xi_uns_type);
>> >      SET_TYPE_MODE (vector_quad_type_node, PXImode);
>         ^^^^^^^^^^^^^^
>
>This is why types_compatible_p() fails.  before checking sign and
>precision
>matching, it does:
>
>  inner_type = TYPE_MAIN_VARIANT (inner_type);
>  outer_type = TYPE_MAIN_VARIANT (outer_type);
>
>  if (inner_type == outer_type)
>    return true;
>
>/* Changes in machine mode are never useless conversions because the
>RTL
>     middle-end expects explicit conversions between modes.  */
>  if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type))
>    return false;
>
>and since the modes are now different, it never gets to check the sign
>and
>precision.... (which do match).
>
>
>
>> >      layout_type (vector_quad_type_node);
>> 
>> Why not put the fix here instead of in build distinct type copy?? 
>> 
>>  lang_hooks.types.register_builtin_type (vector_quad_type_node,
>> >                                              "__vector_quad")
>> >
>> >The vector_quad ends up with MAX and MIN set to the uint512_t type,
>> >which is
>> >not types_compatible_p...  
>> >Perhaps the type should be created some other way rather than
>> >distinct_type?
>> 
>> Quite sure. 
>
>
>I could fix it right there... but it wont prevent something similar
>from
>happening elsewhere.  Anyone changing the mode of the new distinct type
>will
>continue to allow types_incompatible_p() types for MIN and MAX.
>
>Maybe its not a big deal, but it just seems like a glaring
>inconsistency to me.
>If you ask for  distinct type, you should get one, complete with
>distinct
>elements so you don't need to worry about things like this.
>
>It doesn't seem like powerpc is doing anything wrong, but they are
>getting an
>inconsistent type back due to the way types_compatible_p checks things.
>
>Now, looking further into it, this would appear to be the only 2 places
>in all
>the architectures where build_distinct_type_copy() is called, and then
>the mode
>is changed.   All the other places build a type then set the mode
>rather than
>getting a copy. 
>
>I'd still prefer to fix it at its source, but given the lack of
>prevalence, I
>could just set the MIN MAX properly here where these 2 types are having
>their
>mode changed.   
>
>Is this your preferred solution?

The backen should use more lowlevel functions to build this type rather than
copying from a type that isn't appropriate. Off my head it wants
fixup_unsigned_type after setting the mode and eventually a different function
to build the original type. See tree.c where we build for example
boolean_type_node

>diff --git a/gcc/config/rs6000/rs6000-call.c
>b/gcc/config/rs6000/rs6000-call.c
>index 9fdf97bc803..1fcb438ef95 100644
>--- a/gcc/config/rs6000/rs6000-call.c
>+++ b/gcc/config/rs6000/rs6000-call.c
>@@ -12917,6 +12917,13 @@ rs6000_init_builtins (void)
>       tree oi_uns_type = make_unsigned_type (256);
>       vector_pair_type_node = build_distinct_type_copy (oi_uns_type);
>       SET_TYPE_MODE (vector_pair_type_node, POImode);
>+      // Changing modes makes the types incompatable.
>+      TYPE_MIN_VALUE (vector_pair_type_node)
>+       = wide_int_to_tree (vector_pair_type_node,
>+                           wi::to_wide (TYPE_MIN_VALUE
>(oi_uns_type)));
>+      TYPE_MAX_VALUE (vector_pair_type_node)
>+       = wide_int_to_tree (vector_pair_type_node,
>+                           wi::to_wide (TYPE_MAX_VALUE
>(oi_uns_type)));
>       layout_type (vector_pair_type_node);
>       lang_hooks.types.register_builtin_type (vector_pair_type_node,
>                                              "__vector_pair");
>@@ -12924,6 +12931,13 @@ rs6000_init_builtins (void)
>       tree xi_uns_type = make_unsigned_type (512);
>       vector_quad_type_node = build_distinct_type_copy (xi_uns_type);
>       SET_TYPE_MODE (vector_quad_type_node, PXImode);
>+      // Changing modes makes the types incompatable.
>+      TYPE_MIN_VALUE (vector_quad_type_node)
>+       = wide_int_to_tree (vector_quad_type_node,
>+                           wi::to_wide (TYPE_MIN_VALUE
>(xi_uns_type)));
>+      TYPE_MAX_VALUE (vector_quad_type_node)
>+       = wide_int_to_tree (vector_quad_type_node,
>+                           wi::to_wide (TYPE_MAX_VALUE
>(xi_uns_type)));
>       layout_type (vector_quad_type_node);
>       lang_hooks.types.register_builtin_type (vector_quad_type_node,
>                                              "__vector_quad");
>
>.
>
>
>> 
>> >
>> >Im starting to wonder if I should stop trying to assert type
>> >correctness when
>> >processing ranges since our type "system" has too many tweaky
>> >inconsistencies
>> >that we continue to trip over.  
>> >
>> >Instead, just make sure precision and sign are the same and "trust"
>> >gimple to
>> >be right otherwise.
>> 
>> If precision and sign are the same then types_compatible_p will
>return true.
>
>except in cases like this :-P