[Bug target/59189] New: [SH] machine_dependent_reorg (sh_reorg) creates broken basic block structures

[Bug target/59189] New: [SH] machine_dependent_reorg (sh_reorg) creates broken basic block structures

[olegendo at gcc dot gnu.org]

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

            Bug ID: 59189
           Summary: [SH] machine_dependent_reorg (sh_reorg) creates broken
                    basic block structures
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: olegendo at gcc dot gnu.org
                CC: kkojima at gcc dot gnu.org
            Target: sh*-*-*

I've run into this when working on an SH specific RTL pass that is supposed to
remove clrt and sett insns (PR 53976).  E.g. a clrt insn can be removed in the
BB that is the branch-false edge, as T_REG will be known to be zero there.
This kind of optimization can only be done after the basic block and branch
conditions are finalized to avoid wrong code, so I initially inserted this pass
before the delayed-branch stuffing pass (dbr), which happens after
machine_dependent_reorg.

SH's machine_dependent_reorg splits conditional branches that are out of range
as follows.

Original:
     bf    <original out of range label>

     < fall thru BB>

Split:
     bt   .Lxxx

     bra  <original out of range label>

.Lxxx:

However, it seems it doesn't create the appropriate basic blocks for the
additional branch, and the modified basic block does not end with a conditional
branch.  Some example RTL (from CSiBE OpenTCP):

Before machine_dependent_reorg:

(note 1502 1452 1503 55 [bb 55] NOTE_INSN_BASIC_BLOCK)
(note 1503 1502 1505 55 NOTE_INSN_DELETED)
(note 1505 1503 2474 55 NOTE_INSN_DELETED)
(insn:TI 2474 1505 1506 55 (set (reg:SI 0 r0)
        (reg:SI 3 r3 [orig:374 D.4241 ] [374])) tcp.c:1822 257 {movsi_ie}
     (nil))
(insn 1506 2474 1507 55 (set (reg:SI 147 t)
        (and:SI (not:SI (reg:SI 0 r0))
            (const_int 1 [0x1]))) tcp.c:1822 9 {tstsi_t_and_not}
     (expr_list:REG_DEAD (reg:SI 0 r0)
        (nil)))
(jump_insn:TI 1507 1506 1538 55 (set (pc)
        (if_then_else (eq (reg:SI 147 t)
                (const_int 0 [0]))
            (label_ref:SI 2692)
            (pc))) tcp.c:1822 302 {*cbranch_t}
     (expr_list:REG_DEAD (reg:SI 147 t)
        (int_list:REG_BR_PROB 2071 (nil)))
 -> 2692)


(note 1538 1507 1539 56 [bb 56] NOTE_INSN_BASIC_BLOCK)
....



After machine_dependent_reorg:

(note 1502 2998 1503 [bb 55] NOTE_INSN_BASIC_BLOCK)
(note 1503 1502 1505 NOTE_INSN_DELETED)
(note 1505 1503 2474 NOTE_INSN_DELETED)
(insn 2474 1505 1506 (set (reg:SI 0 r0)
        (reg:SI 3 r3 [orig:374 D.4241 ] [374])) tcp.c:1822 257 {movsi_ie}
     (nil))
(insn 1506 2474 1507 (set (reg:SI 147 t)
        (and:SI (not:SI (reg:SI 0 r0))
            (const_int 1 [0x1]))) tcp.c:1822 9 {tstsi_t_and_not}
     (expr_list:REG_DEAD (reg:SI 0 r0)
        (nil)))
(jump_insn 1507 1506 2996 (set (pc)
        (if_then_else (ne (reg:SI 147 t)
                (const_int 0 [0]))
            (label_ref:SI 2992)
            (pc))) tcp.c:1822 302 {*cbranch_t}
     (expr_list:REG_DEAD (reg:SI 147 t)
        (int_list:REG_BR_PROB 7929 (nil)))
 -> 2992)
(insn 2996 1507 2997 (set (pc)
        (unspec [
                (const_int 43 [0x2b])
                (pc)
                (const_int 0 [0])
            ] 4)) tcp.c:1822 305 {stuff_delay_slot}
     (nil))
(insn 2997 2996 2993 (set (pc)
        (unspec [
                (const_int 44 [0x2c])
            ] 4)) tcp.c:1822 -1
     (nil))
(jump_insn 2993 2997 2994 (set (pc)
        (label_ref 2692)) tcp.c:1822 -1
     (nil)
 -> 2692)


(barrier 2994 2993 2992)
(code_label 2992 2994 1538 617 "" [1 uses])
(note 1538 2992 1539 [bb 56] NOTE_INSN_BASIC_BLOCK)
...


This makes it impossible to traverse basic blocks and track T_REG values.  It
doesn't seem to cause any other problems, but it's confusing to see broken RTL
at some point in time.  Maybe this should be fixed by putting the introduced
unconditional branch instruction into a separate basic block.
Kaz, do you happen to know any historical things regarding this behavior?

[Bug target/59189] [SH] machine_dependent_reorg (sh_reorg) creates broken basic block structures

[olegendo at gcc dot gnu.org]

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

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Hm, I found this one:
http://gcc.gnu.org/ml/gcc/2008-12/msg00368.html

"The CFG is not valid at the point of the machine reorg pass, mainly
for historical reasons.  You can see all the insns reliably by doing
  for (insn = get_insns (); insn != NULL_RTX; insn = NEXT_INSN (insn))"

In sh.md (currently line ~1255) there's a comment:

;; If we applied this split when not optimizing, it would only be
;; applied during the machine-dependent reorg, when no new basic blocks
;; may be created.

But that's all.
The branch splitting happens in sh.c (split_branches), and I could find any
hints there either ...

[Bug target/59189] [SH] machine_dependent_reorg (sh_reorg) creates broken basic block structures

[kkojima at gcc dot gnu.org]

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

--- Comment #2 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
It seems that simply there was no user of the valid CFG after m_d_r
pass.

> Maybe this should be fixed by putting the introduced unconditional
> branch instruction into a separate basic block.

Sounds plausible.

[Bug target/59189] [SH] machine_dependent_reorg (sh_reorg) creates broken basic block structures

[olegendo at gcc dot gnu.org]

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

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Maybe adding a "conditional far branch" insn, as mentioned in PR 54762, would
fix this problem, or at least parts of it.