[Bug target/52804] New: IRA/RELOAD allocate wrong register on ARM for cortex-m0

[Bug target/52804] New: IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

             Bug #: 52804
           Summary: IRA/RELOAD allocate wrong register on ARM for
                    cortex-m0
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: amker.cheng@gmail.com


For following code code:
void foo(unsigned char ** i, char *** o,
         unsigned int row, int num);
extern signed long tab[];
extern unsigned int w;
void foo(unsigned char ** i, char *** o,
         unsigned int row, int num)
{
  register int r, g, b;
  register signed long * t = tab;
  register char * pi;
  register char * o0;
  register char * o1;
  register unsigned int c;
  unsigned int n = w;

  while (--num >= 0) {
    pi = *i++;
    o0 = o[0][row];
    o1 = o[1][row];
    row++;
    for (c = 0; c < n; c++) {
      r = ((int) (pi[0]));
      g = ((int) (pi[1]));
      b = ((int) (pi[2]));
      pi += 3;

      o0[c] = (unsigned char)
        ((t[r] + t[g] + t[b]));
      o1[c] = (unsigned char)
        ((t[r] + t[g] + t[b]));
    }
  }
}
Compile it with following command:
$ arm-none-eabi-gcc -S -mthumb -mcpu=cortex-m0 -O2 -o foo.S foo.c

comparing ira/reload dump as following:
/*
dump of ira:

(insn 82 81 83 3 (set (reg/f:SI 281 [ *o_15(D) ])
        (mem/f:SI (reg/v/f:SI 315 [orig:275 o ] [275]) [2 *o_15(D)+0 S4 A32]))
./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186 {*thumb1_movsi_insn}
     (expr_list:REG_EQUIV (mem/f:SI (reg/v/f:SI 315 [orig:275 o ] [275]) [2
*o_15(D)+0 S4 A32])
        (nil)))

(insn 83 82 84 3 (set (reg/v/f:SI 198 [ o0 ])
        (mem/f:SI (plus:SI (reg/f:SI 281 [ *o_15(D) ])
                (reg:SI 273 [ D.4183 ])) [2 *D.4088_18+0 S4 A32]))
./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186 {*thumb1_movsi_insn}
     (expr_list:REG_DEAD (reg/f:SI 281 [ *o_15(D) ])
        (nil)))

(insn 84 83 85 3 (set (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
        (mem/f:SI (plus:SI (reg/v/f:SI 315 [orig:275 o ] [275])
                (const_int 4 [0x4])) [2 MEM[(char * * *)o_15(D) + 4B]+0 S4
A32])) ./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:19 186 {*thumb1_movsi_insn}
     (expr_list:REG_EQUIV (mem/f:SI (plus:SI (reg/v/f:SI 315 [orig:275 o ]
[275])
                (const_int 4 [0x4])) [2 MEM[(char * * *)o_15(D) + 4B]+0 S4
A32])
        (nil)))

(insn 85 84 171 3 (set (reg/v/f:SI 201 [ o1 ])
        (mem/f:SI (plus:SI (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
                (reg:SI 273 [ D.4183 ])) [2 *D.4091_23+0 S4 A32]))
./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:19 186 {*thumb1_movsi_insn}
     (expr_list:REG_DEAD (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
        (expr_list:REG_DEAD (reg:SI 273 [ D.4183 ])
            (nil))))


dump of reload:

(note 82 81 207 3 NOTE_INSN_DELETED)

(insn 207 82 208 3 (set (reg:SI 6 r6)
        (reg/v/f:SI 9 r9 [orig:275 o ] [275]))
./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186 {*thumb1_movsi_insn}
     (nil))

(insn 208 207 209 3 (set (reg:SI 6 r6)
        (mem/f:SI (reg:SI 6 r6) [2 *o_15(D)+0 S4 A32]))
./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186 {*thumb1_movsi_insn}
     (nil))

(insn 209 208 210 3 (set (reg:SI 7 r7)
        (mem/f:SI (plus:SI (reg:SI 6 r6)
                (reg:SI 3 r3 [orig:273 D.4183 ] [273])) [2 *D.4088_18+0 S4
A32])) ./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186 {*thumb1_movsi_insn}
     (nil))

(insn 210 209 84 3 (set (reg/v/f:SI 12 ip [orig:198 o0 ] [198])
        (reg:SI 7 r7)) ./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:18 186
{*thumb1_movsi_insn}
     (nil))

(note 84 210 211 3 NOTE_INSN_DELETED)

(insn 211 84 85 3 (set (reg:SI 0 r0)
        (mem/f:SI (plus:SI (reg:SI 6 r6)
                (const_int 4 [0x4])) [2 MEM[(char * * *)o_15(D) + 4B]+0 S4
A32])) ./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:19 186 {*thumb1_movsi_insn}
     (nil))

(insn 85 211 171 3 (set (reg/v/f:SI 7 r7 [orig:201 o1 ] [201])
        (mem/f:SI (plus:SI (reg:SI 0 r0)
                (reg:SI 3 r3 [orig:273 D.4183 ] [273])) [2 *D.4091_23+0 S4
A32])) ./gccmpsm0/obj_lite/cjpeg/jccolor-case.E:19 186 {*thumb1_movsi_insn}
     (nil))

*/
Obviously, r6 is corrupted in insn 208, while it is used in insn 211.
piece of generated assembly codes as following:

foo:
    push    {r4, r5, r6, r7, lr}
    mov    r5, r9
    mov    r7, fp
    mov    r6, sl
    mov    r4, r8
    push    {r4, r5, r6, r7}
    mov    sl, r3
    lsl    r2, r2, #2
    ldr    r3, .L11
    sub    r2, r2, r0
    sub    sp, sp, #20
    mov    r9, r1  *****step1
    sub    r2, r2, #4
    ldr    r1, [r3]
    ldr    r5, .L11+4
    mov    fp, r0
    str    r2, [sp, #12]
.L8:
    mov    r6, sl
    sub    r6, r6, #1
    mov    sl, r6
    bmi    .L10
.L7:
    mov    r0, fp
    ldr    r4, [sp, #12]
    add    r0, r0, #4
    mov    r6, r9  *****step2
    mov    fp, r0
    ldr    r6, [r6]  *****step3, r6 corrupted
    mov    r3, r4
    add    r3, r3, fp
    sub    r0, r0, #4
    ldr    r7, [r6, r3]
    ldmia    r0!, {r2}
    ldr    r0, [r6, #4]  *****step4, r6 is wrong here
    mov    ip, r7
    ldr    r7, [r0, r3]
    mov    r3, #0
    cmp    r1, #0
    beq    .L8
    mov    r4, ip
    str    r1, [sp, #4]
    str    r4, [sp, #8]
    mov    ip, r7
.L6:

[Bug target/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

--- Comment #1 from amker.cheng <amker.cheng at gmail dot com> 2012-04-03 16:43:30 UTC ---
For insns before ira:


(insn 82 81 83 3 (set (reg/f:SI 281 [ *o_15(D) ])
        (mem/f:SI (reg/v/f:SI 315 [orig:275 o ] [275]) [2 *o_15(D)+0 S4 A32]))
pr52804.c:18 186 {*thumb1_movsi_insn}
     (expr_list:REG_EQUIV (mem/f:SI (reg/v/f:SI 315 [orig:275 o ] [275]) [2
*o_15(D)+0 S4 A32])
        (nil)))

(insn 83 82 84 3 (set (reg/v/f:SI 198 [ o0 ])
        (mem/f:SI (plus:SI (reg/f:SI 281 [ *o_15(D) ])
                (reg:SI 273 [ D.4183 ])) [2 *D.4088_18+0 S4 A32])) pr52804.c:18
186 {*thumb1_movsi_insn}
     (expr_list:REG_DEAD (reg/f:SI 281 [ *o_15(D) ])
        (nil)))

(insn 84 83 85 3 (set (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
        (mem/f:SI (plus:SI (reg/v/f:SI 315 [orig:275 o ] [275])
                (const_int 4 [0x4])) [2 MEM[(char * * *)o_15(D) + 4B]+0 S4
A32])) pr52804.c:19 186 {*thumb1_movsi_insn}
     (expr_list:REG_EQUIV (mem/f:SI (plus:SI (reg/v/f:SI 315 [orig:275 o ]
[275])
                (const_int 4 [0x4])) [2 MEM[(char * * *)o_15(D) + 4B]+0 S4
A32])
        (nil)))

(insn 85 84 171 3 (set (reg/v/f:SI 201 [ o1 ])
        (mem/f:SI (plus:SI (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
                (reg:SI 273 [ D.4183 ])) [2 *D.4091_23+0 S4 A32])) pr52804.c:19
186 {*thumb1_movsi_insn}
     (expr_list:REG_DEAD (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
        (expr_list:REG_DEAD (reg:SI 273 [ D.4183 ])
            (nil))))

The registers allocated are:
r315 -> r9
r281 -> mem
r273 -> r3
r198 -> r12
r201 -> r7

The insns need reload are like:
insn 82 (deleted)
insn 84 (deleted)
insn 83
insn 85

The corresponding dump info of reload pass is like:

Reloads for insn # 83
Reload 0: reload_in (SI) = (reg/v/f:SI 9 r9 [orig:275 o ] [275])
    BASE_REGS, RELOAD_FOR_INPADDR_ADDRESS (opnum = 1)
    reload_in_reg: (reg/v/f:SI 9 r9 [orig:275 o ] [275])
    reload_reg_rtx: (reg:SI 6 r6)
Reload 1: reload_in (SI) = (mem/f:SI (reg/v/f:SI 9 r9 [orig:275 o ] [275]) [2
*o_15(D)+0 S4 A32])
    LO_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine
    reload_in_reg: (reg/f:SI 281 [ *o_15(D) ])
    reload_reg_rtx: (reg:SI 6 r6)
Reload 2: LO_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine,
secondary_reload_p
    reload_reg_rtx: (reg:SI 7 r7)
Reload 3: reload_in (SI) = (mem/f:SI (plus:SI (reg/f:SI 281 [ *o_15(D) ])
                                                        (reg:SI 3 r3 [orig:273
D.4183 ] [273])) [2 *D.4088_18+0 S4 A32])
    CORE_REGS, RELOAD_FOR_INPUT (opnum = 1)
    reload_in_reg: (mem/f:SI (plus:SI (reg/f:SI 281 [ *o_15(D) ])
                                                        (reg:SI 3 r3 [orig:273
D.4183 ] [273])) [2 *D.4088_18+0 S4 A32])
    reload_reg_rtx: (reg/v/f:SI 12 ip [orig:198 o0 ] [198])
    secondary_in_reload = 2

Reloads for insn # 85
Reload 0: reload_in (SI) = (reg/v/f:SI 9 r9 [orig:275 o ] [275])
    BASE_REGS, RELOAD_FOR_OPADDR_ADDR (opnum = 1)
    reload_in_reg: (reg/v/f:SI 9 r9 [orig:275 o ] [275])
    reload_reg_rtx: (reg:SI 6 r6)
Reload 1: reload_in (SI) = (mem/f:SI (plus:SI (reg/v/f:SI 9 r9 [orig:275 o ]
[275])
                                                        (const_int 4 [0x4])) [2
MEM[(char * * *)o_15(D) + 4B]+0 S4 A32])
    LO_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 1), can't combine
    reload_in_reg: (reg/f:SI 282 [ MEM[(char * * *)o_15(D) + 4B] ])
    reload_reg_rtx: (reg:SI 0 r0)

We can see, after reload, insn sequence for insn 83/85 shoud be like:
insn 83:
  r6 = r9
  r6 = [r6]
  r7 = [r6 + r3]
  r12 = r7
insn 85:
  r6 = r9
  r0 = [r6 + 4]
  r7 = [r0 + r3]

***BUT***
The problem is:
RELOAD forms wrong inherited information when reloading insn 83, i.e., reload
assumes that r9 is reloaded in r6 and is valid for inheriting when reloading
insn 85. Resulting in using r6, which has already been corrupted.

After looking into reload. I think function reload_reg_reaches_end_p has missed
following case:
rld[0]
  in : r9
  reg_rtx : r6
  when_needed : RELOAD_FOR_INPADDR_ADDRESS
rld[1]
  in : [r9]
  reg_rtx : r6
  when_neede : RELOAD_FOR_INPUT_ADDRESS

In this case, the call of "reload_reg_reaches_end_p(regno(=6), reloadnum(=0))"
should return 0, rather than 1 as now. because r6 used in rld[0] is corrupted
by rld[1].

[Bug target/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

--- Comment #2 from amker.cheng <amker.cheng at gmail dot com> 2012-04-16 09:00:08 UTC ---
Any comments?
Or could anyone help me confirm this issue?
Thanks very much.

[Bug rtl-optimization/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

--- Comment #3 from amker at gcc dot gnu.org 2012-05-04 02:52:32 UTC ---
Author: amker
Date: Fri May  4 02:52:27 2012
New Revision: 187139

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187139
Log:

    PR rtl-optimization/52804
    * reload1.c (reload_reg_reaches_end_p): Check whether successor
    reload with type RELOAD_FOR_INPUT_ADDRESS kills reload register
    of current one with type RELOAD_FOR_INPADDR_ADDRESS.
    Same stands for reloads with type RELOAD_FOR_OUTPUT_ADDRESS and
    RELOAD_FOR_OUTADDR_ADDRESS.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload1.c

[Bug rtl-optimization/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-05-14
                 CC|                            |ramana at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #4 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2012-05-14 11:42:53 UTC ---
Fixed on trunk so far. Any plans of backporting this to 4.7 branch ?

ramana

[Bug rtl-optimization/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

--- Comment #5 from amker at gcc dot gnu.org 2012-05-15 02:14:11 UTC ---
Author: amker
Date: Tue May 15 02:14:05 2012
New Revision: 187496

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187496
Log:
    Backport r187139 from mainline.
    2012-05-04  Bin Cheng  <bin.cheng@arm.com>

    PR rtl-optimization/52804
    * reload1.c (reload_reg_reaches_end_p): Check whether successor
    reload with type RELOAD_FOR_INPUT_ADDRESS kills reload register
    of current one with type RELOAD_FOR_INPADDR_ADDRESS.
    Same stands for reloads with type RELOAD_FOR_OUTPUT_ADDRESS and
    RELOAD_FOR_OUTADDR_ADDRESS.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/reload1.c

[Bug rtl-optimization/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

--- Comment #6 from amker.cheng <amker.cheng at gmail dot com> 2012-05-15 02:15:59 UTC ---
No regression reported in trunk so far, I back ported it into 4.7 branch.

[Bug rtl-optimization/52804] IRA/RELOAD allocate wrong register on ARM for cortex-m0

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52804

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.7.1

--- Comment #7 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2012-05-15 08:43:27 UTC ---
Fixed then .