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-bugzi...@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 rguent...@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