Re: [PATCH][RFA][LRA] Don't try to break down subreg expressions if insn already matches

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

On 13/02/15 10:10, pinskia@gmail.com wrote:
> > On Feb 13, 2015, at 1:48 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> >
> > Hi all,
> >
> > In my tree added a pattern to the arm backend that's supposed to match:
> > (set (reg:SI r0)
> >      (subreg:SI
> >        (plus:DI
> >          (mult:DI (sign_extend:DI (reg:SI r1))
> >                   (sign_extend:DI (reg:SI r2)))
> >        (const_int 2147483648 [0x80000000])) 4))
> >
> > That is, take two SImode regs, sign-extend to DImode, multiply in DImode,
> > add a const_int and take the most significant SImode subreg.
> Seems better to use shifts for the most significant simode and low part subreg after that. Isn't that what other targets do?

I thought about that, but combine tries to match:
(set (reg/i:SI 0 r0)
    (subreg:SI (plus:DI (mult:DI (sign_extend:DI (reg:SI 0 r0 [ a ]))
                (sign_extend:DI (reg:SI 1 r1 [ b ])))
            (const_int 2147483648 [0x80000000])) 4))


Looking at the RTL dumps all shifts are gone by the time combine is 
reached (in this case cse1 removes the shifts)

Kyrill

> Thanks,
> Andrew
>
>
> > Combine matches it fine but I get an ICE in lra:
> > 0xa665a5 crash_signal
> >         $SRC/gcc/toplev.c:383
> > 0x91ec1c lra_emit_add(rtx_def*, rtx_def*, rtx_def*)
> >         $SRC/gcc/lra.c:418
> > 0x91ef8a lra_emit_move(rtx_def*, rtx_def*)
> >         $SRC/gcc/lra.c:528
> > 0x9279b2 insert_move_for_subreg
> >         $SRC/gcc/lra-constraints.c:1371
> > 0x931bdb simplify_operand_subreg
> >         $SRC/gcc/lra-constraints.c:1507
> > 0x931bdb curr_insn_transform
> >         $SRC/gcc/lra-constraints.c:3437
> > 0x934793 lra_constraints(bool)
> >         $SRC/gcc/lra-constraints.c:4451
> > 0x9217d0 lra(_IO_FILE*)
> >         $SRC/gcc/lra.c:2292
> > 0x8e0dba do_reload
> >         $SRC/gcc/ira.c:5418
> > 0x8e0dba execute
> >         $SRC/gcc/ira.c:5589
> >
> >
> > I *think* the problem is that simplify_operand_subreg tries to break down
> > this complex RTX involving the subreg into separate MULT/PLUS operations
> > and breaking down the line.
> >
> > If I apply the attached patch that stops trying to simplify the subreg
> > expression if the whole thing matches something already my code compiles
> > succesfully, and passes arm testing and x86_64 bootstrap.
> > However, I'm concerned that calling recog_memoized is too heavy-handed,
> > is there a better approach?
> > Or is this even the right way to go about this?
> >
> > I'm also attaching the patch to the arm backend that can be used to
> > reproduce this ICE with the following C-code at -O2:
> >
> > int
> > smmulr (int a, int b)
> > {
> >   return ((long long)a * b + 0x80000000) >> 32;
> > }
> >
> >
> > Thanks,
> > Kyrill
> >
> > 2015-02-10  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >     * lra-constraints.c (simplify_operand_subreg): Do not try to simplify
> >     subreg if the pattern already matches.
> > <lra-subreg.patch>
> > <arm-smmulr.patch>