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
