[Bug target/64761] New: [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[Bug target/64761] New: [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

            Bug ID: 64761
           Summary: [4.9/5 Regression] -freorder-blocks-and-partition
                    causes some failures on SH
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kkojima at gcc dot gnu.org
            Target: sh*-*-*

For SH, we have two failures on testsuite

FAIL: gcc.dg/tree-prof/cold_partition_label.c compilation,  -fprofile-use
-D_PROFILE_USE (internal compiler error)
FAIL: gcc.dg/tree-prof/va-arg-pack-1.c compilation,  -fprofile-use
-D_PROFILE_USE (internal compiler error)

with -freorder-blocks-and-partition.  Although both test cases
are added at 4.9, they don't fail with 4.8.4 compiler.  So
this is a 4.9/5 regression from 4.8.
In both cases, (set (pc) (label_ref ...)) can't be recognized
because CROSSING_JUMP_P (insn) is set.  I've added a new insn

(define_insn "*jump_compact_crossing"
  [(set (pc)
    (label_ref (match_operand 0 "" "")))]
  "TARGET_SH1
   && flag_reorder_blocks_and_partition
   && CROSSING_JUMP_P (insn)"
{
  /* The length is 16 if the delay slot is unfilled.  */
  return output_far_jump (insn, operands[0]);
}
  [(set_attr "type" "jump")
   (set_attr "length" "16")])

to sh.md.  I guess that it's enough ATM, though it isn't optimal.
Then we get another failures for both tests like:

  Error: displacement to defined symbol .L10 overflows 8-bit field

from assembler.
It looks almost wrong branches came from follow jump optimization.
Giving SH specific TARGET_CAN_FOLLOW_JUMP target macro as

static bool
sh_can_follow_jump (const rtx_insn *, const rtx_insn *)
{
  if (TARGET_SH1
      && flag_reorder_blocks_and_partition
      && simplejump_p (branch2)
      && CROSSING_JUMP_P (branch2))
    return false;

  return true;
}

could fix them.  Joern had already proposed
https://gcc.gnu.org/ml/gcc-patches/2013-04/msg01332.html
which replaces MD_REDIRECT_BRANCH with TARGET_CAN_FOLLOW_JUMP
and these changes could be merged.  I'll attach the patches
for it.

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #1 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 34560
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34560&action=edit
Update of Joern's patch

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #2 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 34561
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34561&action=edit
patch for crossing jump

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Even after these changes,

  Error: displacement to defined symbol .L59 overflows 12-bit field

remains for va-arg-pack-1.c and a new failure

  Error: displacement to defined symbol .L31 overflows 8-bit field

pops up for gcc.dg/tree-prof/20041218-1.c.
These wrong branches crossing between partitions were created by
dbr_schedule.
For va-arg-pack-1.c, it seems that relax_delay_slots in dbr_schedule
pass does a variant of follow jump optimization without checking
targetm.can_follow_jump.  The patch below fixes it.

diff --git a/reorg.c b/reorg.c
index 326fa53..2ac6dcf 100644
--- a/reorg.c
+++ b/reorg.c
@@ -3262,12 +3263,13 @@ relax_delay_slots (rtx_insn *first)

       /* See if this jump conditionally branches around an unconditional
          jump.  If so, invert this jump and point it to the target of the
-         second jump.  */
+         second jump.  Check if it's possible on the target.  */
       if (next && simplejump_or_return_p (next)
           && any_condjump_p (insn)
           && target_label
           && next_active_insn (target_label) == next_active_insn (next)
-          && no_labels_between_p (insn, next))
+          && no_labels_between_p (insn, next)
+          && targetm.can_follow_jump (insn, next))
         {
           rtx label = JUMP_LABEL (next);

For 20041218-1.c, relax_delay_slots deletes the jump_insn 74 bellow
as a trivial jump to the next insn ignoring that this jump is
a crossing jump between hot/cold partitions.  Notice that there
is a NOTE_INSN_SWITCH_TEXT_SECTIONS note between the jump and its
target label.

...
(jump_insn/j 74 58 59 (set (pc)
        (label_ref:SI 29)) 312 {*jump_compact_crossing}
     (nil)
 -> 29)
(barrier 59 74 105)
(note 105 59 29 NOTE_INSN_SWITCH_TEXT_SECTIONS)
(code_label 29 105 30 31 "" [5 uses])
(note 30 29 31 [bb 13] NOTE_INSN_BASIC_BLOCK)
(insn 31 30 32 (set (reg/f:SI 5 r5 [178])
        (mem/u/c:SI (label_ref 108) [0  S4 A32]))
...

It seems that we should take account of crossing jumps here.  The patch

diff --git a/reorg.c b/reorg.c
index 326fa53..2ac6dcf 100644
--- a/reorg.c
+++ b/reorg.c
@@ -3247,6 +3247,7 @@ relax_delay_slots (rtx_insn *first)
         target_label = find_end_label (target_label);

       if (target_label && next_active_insn (target_label) == next
+          && ! (CROSSING_JUMP_P (insn) || crossing)
           && ! condjump_in_parallel_p (insn))
         {
           delete_jump (insn);

fixes the issue.

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 34562
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34562&action=edit
patch for dbr_schedule

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.3

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
                 CC|                            |jakub at gcc dot gnu.org

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Author: kkojima
Date: Mon Feb  9 23:47:11 2015
New Revision: 220552

URL: https://gcc.gnu.org/viewcvs?rev=220552&root=gcc&view=rev
Log:
PR target/64761
Replace MD_REDIRECT_BRANCH with TARGET_CAN_FOLLOW_JUMP.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.h
    trunk/gcc/doc/tm.texi
    trunk/gcc/doc/tm.texi.in
    trunk/gcc/reorg.c

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

--- Comment #6 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Author: kkojima
Date: Tue Feb 10 00:00:54 2015
New Revision: 220553

URL: https://gcc.gnu.org/viewcvs?rev=220553&root=gcc&view=rev
Log:
PR target/64761
[SH] Add jump insn for -freorder-blocks-and-partition.  Don't degrade
-freorder-blocks-and-partition to -freorder-blocks even when unwinding
is enabled.

* [SH] Add jump insn for -freorder-blocks-and-partition.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md

[Bug target/64761] [4.9/5 Regression] -freorder-blocks-and-partition causes some failures on SH

[kkojima at gcc dot gnu.org]

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

Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Fixed on trunk.