On Wed, 7 Oct 2015, Jan Hubicka wrote:
> > >
> > > The patch works for me but I'm not sure about the store_expr_with_bounds
> > > change. Where does the actual copying take place? adjust_address_nv
> > > just adjusts the mode ...
> >
> > Yep, the mode was supposed to happen in the later path where we emit block
> > moves,
> > but i missed an else there. I will update the patch. Thanks for catching
> > this.
>
> Hi,
> here is updated patch with temp not being ignored - it will fall into the
> emit_block_move as expected. I tested the pach on x86_64-linux, ppc64-linux
> testing in progress, OK?
>
> Honza
>
> * expr.c (store_expr_with_bounds): Handle aggregate moves from
> BLKmode.
> * gimple-expr.c (useless_type_conversion_p): Do not use TYPE_CANONICAL
> to define gimple type system; compare aggregates only by size.
>
> Index: ../gcc/expr.c
> ===================================================================
> --- ../gcc/expr.c (revision 228267)
> +++ ../gcc/expr.c (working copy)
> @@ -5425,6 +5425,15 @@ store_expr_with_bounds (tree exp, rtx ta
> temp = convert_modes (GET_MODE (target), TYPE_MODE (TREE_TYPE (exp)),
> temp, TYPE_UNSIGNED (TREE_TYPE (exp)));
>
> + /* We allow move between structures of same size but different mode.
> + If source is in memory and the mode differs, simply change the memory.
> */
> + if (GET_MODE (temp) == BLKmode && GET_MODE (target) != BLKmode)
> + {
> + gcc_assert (AGGREGATE_TYPE_P (TREE_TYPE (exp)));
I'd remove this assert as we can have, for example, BLKmode vector types.
> + gcc_assert (MEM_P (temp));
> + temp = adjust_address_nv (temp, GET_MODE (target), 0);
> + }
> +
> /* If value was not generated in the target, store it there.
> Convert the value to TARGET's type first if necessary and emit the
> pending incrementations that have been queued when expanding EXP.
> Index: ../gcc/gimple-expr.c
> ===================================================================
> --- ../gcc/gimple-expr.c (revision 228267)
> +++ ../gcc/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. 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)));
> +
> + else if (TREE_CODE (inner_type) == OFFSET_TYPE
> + && TREE_CODE (inner_type) == TREE_CODE (outer_type))
&& TREE_CODE (outer_type) == OFFSET_TYPE
Ok with those changes.
Thanks,
Richard.
> + 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));
>
> return false;
> }
