[Bug tree-optimization/58253] IPA-SRA creates calls with different arguments that the callee accepts

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58253

--- Comment #2 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 28 Nov 2013, jamborm at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58253
> 
> --- Comment #1 from Martin Jambor <jamborm at gcc dot gnu.org> ---
> I do not quite understand why the port needs to look at alignments of
> scalars passed by value at all but I assume there is a reason.
> 
> When it comes to the IPA-SRA created formal parameter and actual
> argument in the pr52402.c testcase, the following happens.  The
> decision in which register pair the parameter is passed is made in
> epiphany_function_arg_boundary.  This is reached in two different
> ways:
> 
> 1. When compiling foo.isra.0, indirectly from call to
>    targetm.calls.function_incoming_arg at function.c:2440.  The type
>    is the type of the PARM_DECL which is aligned to 8 bits.
> 
> 2. When expanding the call in main, indirectly from call to
>    targetm.calls.function_arg.  The type passed is the type of the
>    actual argument, which is an anonymous SSA name which has the
>    natural alignment of the node which is 64.
> 
> Thus, epiphany_function_arg_boundary erroneously comes to two
> different conclusions.  Assuming there is nothing wrong with the
> above, we can fix the problem in IPA-SRA by the patch below which sets
> the type of the PARM_DECL to natural mode alignment (bootstrapped and
> tested and tested on x86_64-linux, the same on ppc64 is underway).
> But again, I am not really sure what the semantics of alignment of
> scalar PARM_DECL is.  Nevertheless, can you please check if the patch
> indeed fixes the bug?  If so, I'll post it to the mailing list for
> review/further discussion.  Thanks.
> 
> 
> 2013-11-28  Martin Jambor  <mjambor@suse.cz>
> 
>     PR ipa/58253
>     * ipa-prop.c (ipa_modify_formal_parameters): Create decls of
>     non-BLKmode in their naturally aligned type.
> 
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 6bdb0df..a26b11a 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -3443,7 +3443,15 @@ ipa_modify_formal_parameters (tree fndecl,
> ipa_parm_adjustment_vec adjustments)
>        if (adj->by_ref)
>          ptype = build_pointer_type (adj->type);
>        else
> -        ptype = adj->type;
> +        {
> +          ptype = adj->type;
> +          if (TYPE_MODE (ptype) != BLKmode)
> +        {
> +          unsigned malign = GET_MODE_ALIGNMENT (TYPE_MODE (ptype));
> +          if (TYPE_ALIGN (ptype) < malign)
> +            ptype = build_aligned_type (ptype, malign);
> +        }
> +        }

Isn't it easier to avoid building a type with different alignment
in the first place?  Or do this adjustment in SRA where the bug
happens?  It seems that when SRA representatives ->type is unaligned
that this means, for by value passing, the value is accessed unaligned
in the caller only.  Thus turn_representatives_into_adjustments
should go back to naturally aligned ->type for !by_ref params.

Richard.