Re: RFA: patch to fix PR55116

[Vladimir Makarov <vmakarov at redhat dot com>]

On 12-10-29 12:21 PM, Richard Sandiford wrote:
> 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 ...))
Yes, that was my thought too.
> 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.
Yes, it works.
I've submitted the following patch.

Attachment: pr55116.patch
Description: Text document