[RFA,middle-end/54041] Convert modes as needed from expand_expr

On 02/07/14 02:17, Richard Biener wrote:
>> +2014-02-05  Jeff Law  <law@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  <wschmidt@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?

As I mentioned last week, we want to pass VOIDmode objects (constants) 
down to convert_memory_address_addr_space unchange.  c_m_a_a_s will 
handle those correctly.

This patch fixes that oversight and the function name in the ChangeLog 
entry.

I've verified this version still fixes the original bug report and 
included it in an x86_64-unknown-linux-gnu bootstrap & test for sanity's 
sake.

OK for the trunk?

Thanks,
Jeff

On Mon, Feb 10, 2014 at 5:35 PM, Jeff Law <law@redhat.com> wrote:
> On 02/07/14 02:17, Richard Biener wrote:
>>>
>>> +2014-02-05  Jeff Law  <law@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  <wschmidt@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?
>
>
> As I mentioned last week, we want to pass VOIDmode objects (constants) down
> to convert_memory_address_addr_space unchange.  c_m_a_a_s will handle those
> correctly.
>
> This patch fixes that oversight and the function name in the ChangeLog
> entry.
>
> I've verified this version still fixes the original bug report and included
> it in an x86_64-unknown-linux-gnu bootstrap & test for sanity's sake.
>
>
> OK for the trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Jeff
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 2dbab72..eca3e2f 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2014-02-05  Jeff Law  <law@redhat.com>
> +
> +       PR middle-end/54041
> +       * expr.c (expand_expr_addr_expr_1): Handle expand_expr returning an
> +       object with an undesirable mode.
> +
>  2014-02-05  Bill Schmidt  <wschmidt@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..42a451d 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 (GET_MODE (tmp) != VOIDmode && tmode != GET_MODE (tmp))
> +       tmp = convert_modes (tmode, GET_MODE (tmp),
> +                            tmp, TYPE_UNSIGNED (TREE_TYPE (offset)));
>        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  <law@redhat.com>
> +
> +       PR middle-end/54041
> +       * gcc.target/m68k/pr54041.c: New test.
> +
>  2014-02-05  Bill Schmidt  <wschmidt@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];
> +}
> +
>