On Wed, 30 Sep 2015, Jan Hubicka wrote: > Hi, > this implements the idea we discussed at Cauldron to not use TYPE_CANONICAL > for > useless_type_conversion_p. The basic idea is that TYPE_CANONICAL is language > specific and should not be part of definition of the Gimple type system that > should > be quite agnostic of language. > > useless_type_conversion_p clearly is about operations on the type and those > do not > depends on TYPE_CANONICAL or alias info. For LTO and C/Fortran > interpoerability > rules we are forced to make TYPE_CANONICAL more coarse than it needs to be > that results in troubles with useless_type_conversion_p use. > > After dropping the check I needed to solve two issues. First is that we need a > definition of useless conversions for aggregates. As discussed earlier I made > it to depend only on size. The basic idea is that only operations you can do > on > gimple with those are moves and field accesses. Field accesses have > corresponding type into in COMPONENT_REF or MEM_REF, so we do not care about > conversions of those. This caused three Ada failures on PPC64, because we can > not move between structures of same size but different mode. > > Other failure introduced was 2 cases in C++ testsuite because we currently > do not handle OFFSET_TYPE at all. I added the obvious check for TREE_TYPE > and BASE_TYPE to be compatible. > I think we can allow more of conversions between OFFSET_TYPEs and integer > types, but I would like to leave this for incremental changes. (It is probalby > not too important anyway). > > Bootstrapped/regtested x86_64-linux except Ada and ppc64-linux for all > languages > including Ada. OK?
Comments below > I have reviewed the uses of useless_type_conversion_p on non-register types > and there are some oddities, will send separate patches for those. > > Honza > > * gimple-expr.c (useless_type_conversion_p): Do not use TYPE_CANONICAL > for defining useless conversions; make structure compatible if size > and mode are. > Index: gimple-expr.c > =================================================================== > --- gimple-expr.c (revision 228267) > +++ gimple-expr.c (working copy) > @@ -87,11 +87,6 @@ useless_type_conversion_p (tree outer_ty > if (inner_type == outer_type) > return true; > > - /* If we know the canonical types, compare them. */ > - if (TYPE_CANONICAL (inner_type) > - && TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) > - return true; > - > /* Changes in machine mode are never useless conversions unless we > deal with aggregate types in which case we defer to later checks. */ > if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) > @@ -270,12 +265,23 @@ useless_type_conversion_p (tree outer_ty > return true; > } > > - /* For aggregates we rely on TYPE_CANONICAL exclusively and require > - explicit conversions for types involving to be structurally > - compared types. */ > + /* For aggregates compare only the size and mode. Accesses to fields do > have > + a type information by themselves and thus we only care if we can i.e. > + use the types in move operations. */ > else if (AGGREGATE_TYPE_P (inner_type) > && TREE_CODE (inner_type) == TREE_CODE (outer_type)) > - return false; > + return (!TYPE_SIZE (outer_type) > + || (TYPE_SIZE (inner_type) > + && operand_equal_p (TYPE_SIZE (inner_type), > + TYPE_SIZE (outer_type), 0))) > + && TYPE_MODE (outer_type) == TYPE_MODE (inner_type); If we require the TYPE_MODE check then just remove the !AGGREGATE_TYPE_P check on the mode equality check at the beginning of the function. There must be a reason why I allowed modes to differ there btw ;) The size compare might be too conservative for X = WITH_SIZE_EXPR <Y, size>; where we take the size to copy from the WITH_SIZE_EXPR. Of course you can't tell this from the types. Well. Do we actually need the !TYPE_SIZE (aka !COMPLETE_TYPE_P) case? Or does this happen exactly when WITH_SIZE_EXPR is used? vertical space missing > + else if (TREE_CODE (inner_type) == OFFSET_TYPE > + && TREE_CODE (inner_type) == TREE_CODE (outer_type)) > + return useless_type_conversion_p (TREE_TYPE (outer_type), > + TREE_TYPE (inner_type)) > + && useless_type_conversion_p > + (TYPE_OFFSET_BASETYPE (outer_type), > + TYPE_OFFSET_BASETYPE (inner_type)); I believe OFFSET_TYPEs in GIMPLE are a red herring - the only cases I saw them are on INTEGER_CSTs. Nothing in the middle-end looks at their type or TYPE_OFFSET_BASETYPE. IMHO the C++ frontend should GIMPLIFY those away. I don't remember trying that btw. so I believe it might be quite easy (and OFFSET_TYPE should become a C++ frontend tree code). > > return false; > } > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)