On Fri, 2 Oct 2015, Jan Hubicka wrote: > > There must be a reason why I allowed modes to differ there btw ;) > > Thinking about it, I guess reason is that incomplete types do not have > resonable modes set, so requiring modes to match will prevent complete > and incomplete types to match.
Hmm. I still think that the mode shouldn't be relevant. Isn't this only about the cases where the aggregate ends up not having its address taken and thus we don't allocate a stack slot for it but end up using a register with mode? ISTR expansion has special cases dealing with some of the mismatch cases and a fallback spilling to the stack. I think we should fix that rather than disallowing aggregate assignments (which are memory ops to GIMPLE) based on the mode of the aggregate. > Here is updated patch which uses the earlier mode check and adjust it > to skip modes only for incomplete types. > > Bootstrapped/regtested ppc64-linux, OK? > > * 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,15 +87,11 @@ 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; > - So I'd like to see an incremental patch committed that just removes this early check and retains it for the AGGREGATE_TYPE_P and OFFSET_TYPE cases. Such patch is pre-approved. > /* 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) > - && !AGGREGATE_TYPE_P (inner_type)) > + && (!AGGREGATE_TYPE_P (inner_type) > + || COMPLETE_TYPE_P (outer_type))) > return false; > > /* If both the inner and outer types are integral types, then the > @@ -270,12 +266,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_SIZE should return false, not true, unless both inner and outer type have !TYPE_SIZE. Btw, I wonder whether using TYPE_CANONICAL for AGGREGATE_TYPE_Ps still makes sense because alias-compatible types should have the same size, no? > + 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'd rather sort out the offset type issue differently and stay with TYPE_CANONICAL compare here for that time. Richard. > return false; > } > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
