Do not use TYPE_CANONICAL in useless_type_conversion

[Richard Biener]

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 <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)