https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648

--- Comment #1 from pinskia at gmail dot com <pinskia at gmail dot com> ---
On Wed, Apr 1, 2015 at 5:44 PM, terry.guo at arm dot com
<gcc-bugzi...@gcc.gnu.org> wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65648
>
>             Bug ID: 65648
>            Summary: [5 Regression] Bad code due to IRA fails to recognize
>                     the clobber in parallel
>            Product: gcc
>            Version: 5.0
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: P3
>          Component: target
>           Assignee: unassigned at gcc dot gnu.org
>           Reporter: terry.guo at arm dot com
>
> Created attachment 35200
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35200&action=edit
> test case
>
> When compile attached case with command:
> arm-none-eabi-gcc -march=armv6-m -mthumb -Os x.c -S
>
> The gcc generates bad code as below:
> main:
>         push    {r0, r1, r2, r4, r5, r6, r7, lr}
>         ldr     r4, .L5
>         movs    r5, #0
>         ldr     r0, [r4, #8]
>         add     r1, sp, #4
>         rsbs    r2, r0, #0
>         adcs    r2, r2, r0
>         subs    r3, r2, r3  //r3 is used but not initialized
>
> The instruction to initialize r3 is removed due to IRA failed to recognize the
> clobber in reload pass. Before the reload pass, we have three instructions 
> like
> below:
> (insn 12 11 14 2 (parallel [
>             (set (reg:SI 128)
>                 (eq:SI (reg:SI 110 [ D.4914 ])
>                     (const_int 0 [0])))
>             (clobber (reg:SI 129))
>         ]) x.c:23 760 {*cstoresi_eq0_thumb1_insn}
>      (expr_list:REG_UNUSED (reg:SI 129)
>         (nil)))
> (insn 20 19 22 2 (set (reg:SI 135)
>         (plus:SI (plus:SI (reg:SI 136)
>                 (reg:SI 137))
>             (geu:SI (reg:SI 131 [ D.4914 ])
>                 (reg:SI 134 [ c ])))) x.c:23 764 {thumb1_addsi3_addgeu}
>      (expr_list:REG_DEAD (reg:SI 137)
>         (expr_list:REG_DEAD (reg:SI 136)
>             (expr_list:REG_DEAD (reg:SI 134 [ c ])
>                 (expr_list:REG_DEAD (reg:SI 131 [ D.4914 ])
>                     (nil))))))
> (insn 22 20 23 2 (set (reg:SI 138)
>         (minus:SI (reg:SI 128)
>             (reg:SI 135))) x.c:23 720 {thumb1_subsi3_insn}
>      (expr_list:REG_DEAD (reg:SI 135)
>         (expr_list:REG_DEAD (reg:SI 128)
>             (nil))))
>
> After the reload pass, those instructions are wrongly turned into:
> (insn 20 53 58 2 (set (reg:SI 3 r3 [135])
>         (plus:SI (plus:SI (reg:SI 3 r3 [137])
>                 (reg:SI 2 r2 [136]))
>             (geu:SI (reg:SI 7 r7 [orig:131 D.4914 ] [131])
>                 (reg:SI 6 r6 [153])))) x.c:23 764 {thumb1_addsi3_addgeu}
>      (nil))
> (insn 58 20 55 2 (parallel [
>             (set (reg:SI 2 r2 [128])
>                 (eq:SI (reg:SI 0 r0 [orig:110 D.4914 ] [110])
>                     (const_int 0 [0])))
>             (clobber (reg:SI 3 r3 [129]))
>         ]) x.c:23 760 {*cstoresi_eq0_thumb1_insn}
>      (nil))
> (note 55 58 22 2 NOTE_INSN_DELETED)
> (insn 22 55 23 2 (set (reg:SI 3 r3 [138])
>         (minus:SI (reg:SI 2 r2 [128])
>             (reg:SI 3 r3 [135]))) x.c:23 720 {thumb1_subsi3_insn}
>      (nil))
>
> However the later pass can recognize the clobber in insn 58 and will remove 
> the
> insn 20 because the r3 defined in insn 20 is clobbered by insn 58.


This is a bug in the arm's thumb1.md file, it uses match_operand when
it should really be using match_scratch.
Looks like most of the arm back-end has this bug in it.

See
https://gcc.gnu.org/onlinedocs/gccint/Regs-and-Memory.html#index-scratch-2870
and the corresponding match_scratch documentation.

That is this:

1515 (define_insn "*cstoresi_eq0_thumb1_insn"
1516   [(set (match_operand:SI 0 "s_register_operand" "=&l,l")
1517         (eq:SI (match_operand:SI 1 "s_register_operand" "l,0")
1518                (const_int 0)))
1519    (clobber (match_operand:SI 2 "s_register_operand" "=X,l"))]
1520   "TARGET_THUMB1"
1521   "@
1522    rsbs\\t%0, %1, #0\;adcs\\t%0, %0, %1
1523    rsbs\\t%2, %1, #0\;adcs\\t%0, %1, %2"
1524   [(set_attr "length" "4")
1525    (set_attr "type" "multiple")]
1526 )

Really should be using (clobber (match_scratch:... ))

So it generates a scratch RTL right away and then the register
allocater knows that it is only used in that instruction.

Thanks,
Andrew

Reply via email to