[Bug rtl-optimization/64331] New: regcprop propagates registers noted as REG_DEAD

[Bug rtl-optimization/64331] New: regcprop propagates registers noted as REG_DEAD

[senthil_kumar.selvaraj at atmel dot com]

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

            Bug ID: 64331
           Summary: regcprop propagates registers noted as REG_DEAD
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: senthil_kumar.selvaraj at atmel dot com

Created attachment 34290
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34290&action=edit
Source file

For the AVR target, compiling the attached source file with -O1 results in code
like this

<snip>
        or r20,r21
        or r20,r22
        or r20,r23
        breq .L1
        ldd r24,Z+4
        ldd r25,Z+5
        ldd r26,Z+6
        ldd r27,Z+7
        cp r20,r24
        cpc r21,r25
        cpc r22,r26
        cpc r23,r27
</snip>

Register R20 was clobbered first up, but is used to compare against R24.

fdump-rtl-all-raw showed that cprop_hardreg is the culprit. In pass *.ce3

<snip>
(insn 7 4 8 2 (set (reg:SI 16 r16 [orig:43 D.1617 ] [43]) 
        (reg/v:SI 20 r20 [orig:46 x ] [46])) reduced.c:12 94 {*movsi}
     (nil))
...
(insn 13 12 14 3 (parallel [
            (set (cc0)
                (compare (reg/v:SI 20 r20 [orig:46 x ] [46])
                    (const_int 0 [0])))
            (clobber (scratch:QI))
        ]) reduced.c:17 413 {*cmpsi}
     (expr_list:REG_DEAD (reg/v:SI 20 r20 [orig:46 x ] [46])
        (nil)))
...
(insn 17 16 18 4 (parallel [
            (set (cc0)
                (compare (reg:SI 16 r16 [orig:43 D.1617 ] [43])
                    (reg:SI 24 r24 [orig:48 t_3(D)->b ] [48])))
            (clobber (scratch:QI))
        ]) reduced.c:20 413 {*cmpsi}
     (expr_list:REG_DEAD (reg:SI 24 r24 [orig:48 t_3(D)->b ] [48])
        (expr_list:REG_DEAD (reg:SI 16 r16 [orig:43 D.1617 ] [43])
            (nil))))
</snip>

into

<snip>
;; Function foo (foo, funcdef_no=0, decl_uid=1599, cgraph_uid=0,
symbol_order=0)

insn 17: replaced reg 16 with 20
rescanning insn with uid = 17.
..
(insn 17 16 18 4 (parallel [
            (set (cc0)
                (compare (reg:SI 20 r20 [orig:43 D.1617 ] [43])
                    (reg:SI 24 r24 [orig:48 t_3(D)->b ] [48])))
            (clobber (scratch:QI))
        ]) reduced.c:20 413 {*cmpsi}
     (expr_list:REG_DEAD (reg:SI 24 r24 [orig:48 t_3(D)->b ] [48])
        (expr_list:REG_DEAD (reg:SI 16 r16 [orig:43 D.1617 ] [43])
            (nil))))
</snip>

The AVR backend, on seeing that reg:SI r20 is dead in insn 13, emits code that
clobbers r20, and this breaks the read in insn 17.

[Bug rtl-optimization/64331] regcprop propagates registers noted as REG_DEAD

[senthil_kumar.selvaraj at atmel dot com]

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

--- Comment #1 from Senthil Kumar Selvaraj <senthil_kumar.selvaraj at atmel dot com> ---
Created attachment 34291
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34291&action=edit
Assembly

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |avr
          Component|rtl-optimization            |target

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Sounds like a target bug.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-12-16
                 CC|                            |ebotcazou at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Right, RTL passes aren't required to maintain REG_DEAD/REG_UNUSED notes, it's
the job of the DF framework.  And this is not done automatically, the consumers
of the notes must request it by means of df_note_add_problem prior to calling
the DF analyzer.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[senthil_kumar.selvaraj at atmel dot com]

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

--- Comment #4 from Senthil Kumar Selvaraj <senthil_kumar.selvaraj at atmel dot com> ---
Yes, running df_notes_add_problem and df_analyze in the target code does work.

Is it just REG_USED/REG_DEAD notes, or is register liveliness
(df_regs_ever_live_p etc..) also not guaranteed to be up to date?

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Is it just REG_USED/REG_DEAD notes, or is register liveliness
> (df_regs_ever_live_p etc..) also not guaranteed to be up to date?

These REG_UNUSED/REG_DEAD notes are special because they are not maintained
incrementally by the DF framework, i.e. the insn manipulation routines don't
cause them to be updated, so they are really valid only just after they are
recomputed.  The rest of the liveness info is updated incrementally (if DF is
asked to).

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P4
                 CC|                            |gjl at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |gjl at gcc dot gnu.org
   Target Milestone|---                         |5.0

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Tue Mar  3 11:09:30 2015
New Revision: 221142

URL: https://gcc.gnu.org/viewcvs?rev=221142&root=gcc&view=rev
Log:
gcc/
    PR target/64331
    * config/avr/avr.c (context.h, tree-pass.h): Include them.
    (avr_pass_data_recompute_notes): New static variable.
    (avr_pass_recompute_notes): New class.
    (avr_register_passes): New static function.
    (avr_option_override): Call it.
gcc/testsuite/
    PR target/64331
    * gcc.target/avr/torture/pr64331.c: New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/avr/torture/pr64331.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/avr/avr.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Tue Mar  3 11:25:04 2015
New Revision: 221143

URL: https://gcc.gnu.org/viewcvs?rev=221143&root=gcc&view=rev
Log:
gcc/
    PR target/64331
    * config/avr/avr.c (context.h, tree-pass.h): Include them.
    (avr_pass_data_recompute_notes): New static variable.
    (avr_pass_recompute_notes): New class.
    (avr_register_passes): New static function.
    (avr_option_override): Call it.
gcc/testsuite/
    PR target/64331
    * gcc.target/avr/torture/pr64331.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/avr/torture/pr64331.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|avr                         |avr sh*-*-*
                 CC|                            |olegendo at gcc dot gnu.org

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I've got a similar case on SH.  In the file spacetab.c, when compiled with -O2
-m4 -ml there's a sequence:

        cmp/eq  r3,r4
        movt    r3
        movt    r12
        cmp/gt  r0,r2

which should be rather (after scheduling):

        cmp/eq  r3,r4
        movt    r3
        cmp/gt  r0,r2
        mov     r3,r12

The sequence can be caught with a simple peephole:

(define_peephole2
  [(set (match_operand:SI 0 "arith_reg_dest")
        (match_operand:SI 1 "t_reg_operand"))
   (set (match_operand:SI 2 "arith_reg_dest")
        (match_operand:SI 3 "t_reg_operand"))]
  "TARGET_SH1"
  [(set (match_dup 0) (reg:SI T_REG))
   (set (match_dup 2) (match_dup 0))])

which doesn't work for some reason.

So I thought maybe something like this:

(define_peephole2
  [(set (match_operand:SI 0 "arith_reg_dest")
        (match_operand:SI 1 "t_reg_operand"))
   (set (match_operand:SI 2 "arith_reg_dest")
        (match_operand:SI 3 "t_reg_operand"))]
  "TARGET_SH1"
  [(const_int 0)]
{
  emit_insn (gen_movt (operands[0], get_t_reg_rtx ()));
  emit_move_insn (operands[2], operands[0]);
  DONE;
})

which emits the insns in the peephole2 pass as intended, but regcprop changes
them again into the undesired movt-movt sequence.  So I've tried adding a
REG_DEAD note to the first insn:

(define_peephole2
  [(set (match_operand:SI 0 "arith_reg_dest")
        (match_operand:SI 1 "t_reg_operand"))
   (set (match_operand:SI 2 "arith_reg_dest")
        (match_operand:SI 3 "t_reg_operand"))]
  "TARGET_SH1"
  [(const_int 0)]
{
  add_reg_note (emit_insn (gen_movt (operands[0], get_t_reg_rtx ())),
                REG_DEAD, get_t_reg_rtx ());

  emit_move_insn (operands[2], operands[0]);
  DONE;
})

and it is still ignored.  regcprop also doesn't look at any costs, so the only
solution to "fix" this is searching and replacing insns manually in split4.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.5                         |---

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|gjl at gcc dot gnu.org             |unassigned at gcc dot gnu.org

--- Comment #13 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Resetting assignee to default.  The AVR backend solved the problem by a
target-specific mini-pass that (re)computes notes as late as possible.

[Bug target/64331] regcprop propagates registers noted as REG_DEAD

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW