On Thu, Feb 6, 2014 at 8:33 PM, Jeff Law <l...@redhat.com> wrote:
>
> expand_expr has, for as long as I can remember, had the ability to ignore
> the desired mode provided by its callers and instead returning something in
> a completely different mode. It's always been the caller's responsibility
> to deal with that.
>
> For the testcase in 54041, we call expand_expr with a desired mode of
> SImode, but it actually returns a HImode object. This causes the assertion
> in convert_memory_address_addr_space to trip because the passed mode must be
> the same as the mode of the memory address.
>
> The fix is simple. If expand_expr returns something in the wrong mode,
> convert it to the desired mode.
>
> I've reviewed the resulting code for the m68k target and it looks correct to
> me. I've also bootstrapped and regression tested on
> x86_64-unknown-linux-gnu, though the new code most certainly does not
> trigger there.
>
> I guess if someone really wanted to be thorough, they'd test on a target
> where pointers and integers are different sizes.
>
> OK for the trunk?
>
> Thanks,
> Jeff
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 2dbab72..4c7da83 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2014-02-05 Jeff Law <l...@redhat.com>
> +
> + PR middle-end/54041
> + * expr.c (expand_expr_addr_1): Handle expand_expr returning an
> + object with an undesirable mode.
> +
> 2014-02-05 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> * config/rs6000/rs6000.c (altivec_expand_vec_perm_const): Change
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 878a51b..9609c45 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -7708,6 +7708,11 @@ expand_expr_addr_expr_1 (tree exp, rtx target, enum
> machine_mode tmode,
> modifier == EXPAND_INITIALIZER
> ? EXPAND_INITIALIZER : EXPAND_NORMAL);
>
> + /* expand_expr is allowed to return an object in a mode other
> + than TMODE. If it did, we need to convert. */
> + if (tmode != GET_MODE (tmp))
> + tmp = convert_modes (tmode, GET_MODE (tmp),
> + tmp, TYPE_UNSIGNED (TREE_TYPE (offset)));
What about CONSTANT_P tmp? Don't you need to use
TYPE_MODE (TREE_TYPE (offset)) in that case?
Richard.
> result = convert_memory_address_addr_space (tmode, result, as);
> tmp = convert_memory_address_addr_space (tmode, tmp, as);
>
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index c81a00d..283912d 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-02-05 Jeff Law <l...@redhat.com>
> +
> + PR middle-end/54041
> + * gcc.target/m68k/pr54041.c: New test.
> +
> 2014-02-05 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> * gcc.dg/vmx/sum2s.c: New.
> diff --git a/gcc/testsuite/gcc.target/m68k/pr54041.c
> b/gcc/testsuite/gcc.target/m68k/pr54041.c
> new file mode 100644
> index 0000000..645cb6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/m68k/pr54041.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -mshort" } */
> +
> +extern int r[];
> +
> +int *fn(int i)
> +{
> + return &r[i];
> +}
> +
>
