Re: [PATCH] combine: Do not combine moves from hard registers

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Fri, Nov 02, 2018 at 10:19:01PM +0000, Renlin Li wrote:
> I find a problem with your change to add make_more_copies.
> I am investigating those regressions, a big amount of them are wrong code 
> generation.
> 
> One problem is that, make_more_copies will split the assignment of fp to 
> sfp.
> 
> From:
> (insn 48 26 28 5 (set (reg/f:SI 102 sfp)
>         (reg/f:SI 11 fp)) -1
> To:
> (insn 51 32 26 5 (set (reg:SI 117)
>         (reg/f:SI 11 fp)) 646 {*arm_movsi_vfp}
>      (expr_list:REG_EQUIV (reg/f:SI 11 fp)
>         (nil)))
> (insn 48 26 28 5 (set (reg/f:SI 102 sfp)
>         (reg:SI 117)) 646 {*arm_movsi_vfp}
>      (expr_list:REG_DEAD (reg:SI 117)
>         (nil)))

I was looking at this just now :-)  (PR87871)

fp is a hard reg, but not a fixed reg, so make_more_moves thinks it is
fine to copy it to some pseudo, before copying it to the final dest.  And
that is just fine as far as I can see.

That final dest is sfp, and that final move is moved over the clobber of
fp, and yes eventually deleted as you say below.

> The original rtx is generated by expand_builtin_setjmp_receiver to adjust 
> the frame pointer.
> 
> And later in LRA, it will try to eliminate frame_pointer with hard frame 
> pointer which is
> defined the ELIMINABLE_REGS.
> 
> Your change split the insn into two.
> This makes it doesn't match the "from" and "to" regs defined in 
> ELIMINABLE_REGS.
> The if statement to generate the adjustment insn is been skipt.
> And the original instruction is just been deleted!

I don't follow why, or what should have prevented it from being deleted.

> Probably, we don't want to split the move rtx if they are related to 
> entries defined in ELIMINABLE_REGS?

One thing I can easily do is not making an intermediate pseudo when copying
*to* a fixed reg, which sfp is.  Let me try if that helps the testcase I'm
looking at (setjmp-4.c).


Segher