RFA: patch to fix PR55116

H.J. in

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

   reported an interesting address

(and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ])
                 (const_int 2 [0x2]))
             (symbol_ref:SI ("glob_vol_int_arr") <var_decl 
0x7ffff03c2720 glob_vol_int_arr>)) 0)
     (const_int 4294967295 [0xffffffff]))

   which can not be correctly extracted.  Here `and' with `subreg' 
behaves as an address mutation.

   The following patch fixes the problem.

Ok to commit, Richard?

2012-10-29  Vladimir Makarov  <vmakarov@redhat.com>

         PR middle-end/55116
         * rtlanal.c (strip_address_mutation): Add SUBREG case.

Vladimir Makarov <vmakarov@redhat.com> writes:
>    H.J. in
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116
>
>    reported an interesting address
>
> (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ])
>                  (const_int 2 [0x2]))
>              (symbol_ref:SI ("glob_vol_int_arr") <var_decl 
> 0x7ffff03c2720 glob_vol_int_arr>)) 0)
>      (const_int 4294967295 [0xffffffff]))
>
>    which can not be correctly extracted.  Here `and' with `subreg' 
> behaves as an address mutation.
>
>    The following patch fixes the problem.
>
> Ok to commit, Richard?

Heh, I wondered if subregs might still be used like that, and was almost
tempted to add them just in case.

I think this particular case is really a failed canonicalisation and that:

   (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0xffffffff))

ought to be:

   (zero_extend:DI (foo:SI ...))

But I know I've approved MIPS patches to accept
(and:DI ... (const_int 0xffffffff)) as an alternative.

> Index: rtlanal.c
> ===================================================================
> --- rtlanal.c   (revision 192942)
> +++ rtlanal.c   (working copy)
> @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum
>         else if (code == AND && CONST_INT_P (XEXP (*loc, 1)))
>          /* (and ... (const_int -X)) is used to align to X bytes.  */
>          loc = &XEXP (*loc, 0);
> +      else if (code == SUBREG
> +              && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0)))
> +       /* (subreg (operator ...) ...) usually inside and is used for
> +          mode conversion too.  */
> +       loc = &XEXP (*loc, 0);

I think the condition should be:

      else if (code == SUBREG
               && !OBJECT_P (SUBREG_REG (*loc))
               && subreg_lowpart (*loc))

OK with that change, if it works.

Thanks for fixing this.

Richard

Sorry, forgot a line:

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Vladimir Makarov <vmakarov@redhat.com> writes:
>> Index: rtlanal.c
>> ===================================================================
>> --- rtlanal.c   (revision 192942)
>> +++ rtlanal.c   (working copy)
>> @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum
>>         else if (code == AND && CONST_INT_P (XEXP (*loc, 1)))
>>          /* (and ... (const_int -X)) is used to align to X bytes.  */
>>          loc = &XEXP (*loc, 0);
>> +      else if (code == SUBREG
>> +              && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0)))
>> +       /* (subreg (operator ...) ...) usually inside and is used for
>> +          mode conversion too.  */
>> +       loc = &XEXP (*loc, 0);
>
> I think the condition should be:
>
>       else if (code == SUBREG
>                && !OBJECT_P (SUBREG_REG (*loc))
>                && subreg_lowpart (*loc))
          loc = &SUBREG_REG (*loc);

i.e. please use SUBREG_REG rather than XEXP.

On 12-10-29 12:30 PM, Richard Sandiford wrote:
> Sorry, forgot a line:
>
>
> i.e. please use SUBREG_REG rather than XEXP.
I've just fixed it.