Re: [PATCH 2/2 v3][IRA,LRA] Fix PR86939, IRA incorrectly creates an interference between a pseudo register and a hard register

[Peter Bergner <bergner at linux dot ibm dot com>]

On 11/8/18 4:57 AM, Renlin Li wrote:
> I think I found the problem!
> 
> As described in the PR, a hard register is used in
> an pre/post modify expression. The hard register is live, but updated.
> In this case, we should make it conflicting with all pseudos live at
> that point.  Does it make sense?
[snip]
> It fixes the ICE of mis-compiled arm-linux-gnueabihf toolchain described in the
> PR.
> 
> I attached the patch for discussion.  I haven't give a complete test on arm or
> any other targets, yet. (Probably need more adjusting)

Yes, this is the problem.  We see from the dump, that r2040 does not conflict with
hard reg r1:

;; a2040(r1597,l0) conflicts: <list of pseudo regs>
;;     total conflict hard regs:
;;     conflict hard regs:

...and we have the following RTL:

(insn 10905 179 199 2 (set (reg:SI 1 r1)
        (reg/f:SI 2040)) "../../gcc/gcc/vec.h":1654 647 {*thumb2_movsi_vfp}
     (nil))

...

(insn 208 202 182 2 (set (mem/f/c:SI (pre_modify:SI (reg:SI 1 r1)
                (plus:SI (reg:SI 1 r1)
                    (const_int 12 [0xc]))) [129 loop_nest.m_vec+0 S4 A32])
        (reg:SI 1048)) "../../gcc/gcc/vec.h":1654 647 {*thumb2_movsi_vfp}
     (expr_list:REG_INC (reg:SI 1 r1)
        (nil)))

...

(insn 191 189 192 2 (set (mem/f/c:SI (plus:SI (reg/f:SI 2040)
                (const_int 8 [0x8])) [367 ddrs_table+0 S4 A32])
        (reg/f:SI 1047)) "../../gcc/gcc/tree-loop-distribution.c":2741 647 {*thumb2_movsi_vfp}
     (nil))

So my patch caused us to (correctly) skip adding a conflict between r1 and
r2040 due to the register copy in insn 10905.  However, they really should
conflict as you found due to the definition of r1 in insn 208 and the fact
we don't add one is a latent bug in LRA.  I think your patch is on the right
track, but not totally there yet.  Imagine instead that the references to r1
and r2040 were swapped, so instead we have:

(insn 10905 179 199 2 (set (reg:SI 2040)
        (reg/f:SI 1 r1)) "../../gcc/gcc/vec.h":1654 647 {*thumb2_movsi_vfp}
     (nil))

...

(insn 208 202 182 2 (set (mem/f/c:SI (pre_modify:SI (reg:SI 2040)
                (plus:SI (reg:SI 2040)
                    (const_int 12 [0xc]))) [129 loop_nest.m_vec+0 S4 A32])
        (reg:SI 1048)) "../../gcc/gcc/vec.h":1654 647 {*thumb2_movsi_vfp}
     (expr_list:REG_INC (reg:SI 2040)
        (nil)))

...

(insn 191 189 192 2 (set (mem/f/c:SI (plus:SI (reg/f:SI 1 r1)
                (const_int 8 [0x8])) [367 ddrs_table+0 S4 A32])
        (reg/f:SI 1047)) "../../gcc/gcc/tree-loop-distribution.c":2741 647 {*thumb2_movsi_vfp}
     (nil))

Even with your patch, we'd miss adding the conflict between r1 and r2040.
Let me think about how we should solve this one.

And a *BIG* thank you for tracking down the problem!!!

Peter