On Thu, 1 Oct 2015, Jan Hubicka wrote: > > > - /* 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 ;) > > I will test the variant moving TYPE_MODE check early. I don't know if > aggregates are special. Only I know that move from agreggate with one mode to > aggregate with another does not work even if they have same sizes. I dunno if > we can actually move non-aggregates this way for fun and profit. > > In general I would like to see less uses of TYPE_MODE than more and sort of > seeing it go away from gimple types, because it is RTL thingy. > > > > > 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? > > It does happen for "bogus" uses of type_compatible_p. For exmaple > ipa-icf calls the predicate on almost everything and gets to incomplete > types here, so we can't segfault. > > I have patches for this, but would like to handle this incrementally.
But then I'd like you to handle !TYPE_SIZE conservatively (return false). > > > > 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). > > Agreed. I also do not see reason to have them and their existence always > surprise me. I can try to look into this incrementally (keeping the compare > as it is right now) Jason, do you have any suggestions how this can > be done? > > I guess we can pull those out of onstant initializers when folding, so we may > need to do some canonicalization here. Yeah, I don't remember exactly how we end up with these in the IL but we could "drop" them during gimplification and make INTEGER_CSTs with OFFSET_TYPE not gimple in is_gimple_constant. Richard. > Honza > > > > > > > > return false; > > > } > > > > > > > > > > -- > > Richard Biener <rguent...@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB > > 21284 (AG Nuernberg) > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)