Re: [PATCH][ARM] PR target/71436: Restrict *load_multiple pattern till after LRA

[Kyrill Tkachov]

Ping.

Thanks,
Kyrill

On 08/12/16 11:55, Kyrill Tkachov wrote:

Ping.
https://gcc.gnu.org/ml/gcc-patches/2016-11/msg03078.html

Thanks,
Kyrill

On 30/11/16 16:47, Kyrill Tkachov wrote:

Hi all,

In this awkward ICE we have a *load_multiple pattern that is being transformed 
in reload from:
(insn 55 67 151 3 (parallel [
            (set (reg:SI 0 r0)
                (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
            (set (reg:SI 158 [ c+4 ])
                (mem/u/c:SI (plus:SI (reg/f:SI 147)
                        (const_int 4 [0x4])) [2 c+4 S4 A32]))
        ]) arm-crash.c:25 393 {*load_multiple}
     (expr_list:REG_UNUSED (reg:SI 0 r0)
        (nil)))


into the invalid:
(insn 55 67 70 3 (parallel [
            (set (reg:SI 0 r0)
                (mem/u/c:SI (reg/f:SI 5 r5 [147]) [2 c+0 S4 A32]))
            (set (mem/c:SI (plus:SI (reg/f:SI 102 sfp)
                        (const_int -4 [0xfffffffffffffffc])) [4 %sfp+-12 S4 
A32])
                (mem/u/c:SI (plus:SI (reg/f:SI 5 r5 [147])
                        (const_int 4 [0x4])) [2 c+4 S4 A32]))
        ]) arm-crash.c:25 393 {*load_multiple}
     (nil))

The operands of *load_multiple are not validated through constraints like LRA 
is used to, but rather through
a match_parallel predicate which ends up calling ldm_stm_operation_p to 
validate the multiple sets.
But this means that LRA cannot reason about the constraints properly.
This two-regiseter load should not have used *load_multiple anyway, it should 
have used *ldm2_ from ldmstm.md
and indeed it did until the loop2_invariant pass which copied the ldm2_ pattern:
(insn 27 23 28 4 (parallel [
            (set (reg:SI 0 r0)
                (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
            (set (reg:SI 1 r1)
                (mem/u/c:SI (plus:SI (reg/f:SI 147)
                        (const_int 4 [0x4])) [2 c+4 S4 A32]))
        ]) "ldm.c":25 385 {*ldm2_}
     (nil))

into:
(insn 55 19 67 3 (parallel [
            (set (reg:SI 0 r0)
                (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
            (set (reg:SI 158)
                (mem/u/c:SI (plus:SI (reg/f:SI 147)
                        (const_int 4 [0x4])) [2 c+4 S4 A32]))
        ]) "ldm.c":25 404 {*load_multiple}
     (expr_list:REG_UNUSED (reg:SI 0 r0)
        (nil)))

Note that it now got recognised as load_multiple because the second register is 
not a hard register but the pseudo 158.
In any case, the solution suggested in the PR (and I agree with it) is to 
restrict *load_multiple to after reload.
The similar pattern *load_multiple_with_writeback also has a similar condition 
and the comment above *load_multiple says that
it's used to generate epilogues, which is done after reload anyway. For 
pre-reload load-multiples the patterns in ldmstm.md
should do just fine.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2016-11-30  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

    PR target/71436
    * config/arm/arm.md (*load_multiple): Add reload_completed to
    matching condition.

2016-11-30  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

    PR target/71436
    * gcc.c-torture/compile/pr71436.c: New test.