Bug 35190 - Wrong branch instruction with -freorder-blocks-and-partition on SH
Summary: Wrong branch instruction with -freorder-blocks-and-partition on SH
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2008-02-14 03:54 UTC by Kazumoto Kojima
Modified: 2008-03-09 23:44 UTC (History)
1 user (show)

See Also:
Host:
Target: sh4-unknown-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-02-16 00:24:45


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kazumoto Kojima 2008-02-14 03:54:20 UTC
On SH, gcc.dg/tree-prof/bb-reorg.c test fails with assembler
messages like:

/tmp/cc3RtxMk.s: 115: Error: displacement to defined symbol .L27 overflows 12-bit field

It happens with a branch from .text to .text.unlikely section
where the label .L27 is placed on.
I've found that bb-reorder makes crossing unconditional branches
into indirect jumps when the architecture doesn't have unconditional
branches that can span all of memory.  This is the case for SH and
the rtls for the above branch in .bbpart rtl dump look like:

(insn 172 156 173 10 (set (reg:SI 211)
        (label_ref:SI 155)) -1 (insn_list:REG_LABEL_OPERAND 155 (nil)))

(jump_insn 173 172 159 10 (set (pc)
        (reg:SI 211)) -1 (expr_list:REG_CROSSING_JUMP (nil)
        (nil)))

which will be assembled to the correct jmup instruction across
sections on this target.
It seems that local-alloc.c:update_equiv_regs replaces this
indirect jump back into a usual jump.  The corresponding part
of .lreg dump is

(jump_insn:HI 173 156 159 13 (set (pc)
        (label_ref:SI 155)) 219 {jump_compact} (expr_list:REG_CROSSING_JUMP (nil)
        (nil)))

which is assembled to a bad short branch instruction on SH.
I'm testing the patch below, though SH has a separate issue with
constant pools for bb-reorg.c test.

--- ORIG/trunk/gcc/local-alloc.c	2008-01-17 09:41:36.000000000 +0900
+++ TMP/trunk/gcc/local-alloc.c	2008-02-14 11:14:04.000000000 +0900
@@ -1107,7 +1107,8 @@ update_equiv_regs (void)
 
 	  /* Don't substitute into a non-local goto, this confuses CFG.  */
 	  if (JUMP_P (insn)
-	      && find_reg_note (insn, REG_NON_LOCAL_GOTO, NULL_RTX))
+	      && (find_reg_note (insn, REG_NON_LOCAL_GOTO, NULL_RTX)
+		  || find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX)))
 	    continue;
 
 	  for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
Comment 1 Steven Bosscher 2008-02-14 10:10:58 UTC
At least add a comment please why REG_CROSSING would need special treatment.
Comment 2 Kazumoto Kojima 2008-02-14 11:09:26 UTC
Sure. 
I'll add the following comment for that.

@@ -1105,9 +1105,14 @@ update_equiv_regs (void)
 	  if (! INSN_P (insn))
 	    continue;
 
-	  /* Don't substitute into a non-local goto, this confuses CFG.  */
+	  /* Don't substitute into a non-local goto, this confuses CFG.
+	     Since bb-reorder uses indirect jumps for crossing branches
+	     when the architecture doesn't have unconditional branches
+	     that can span all of memory, don't touch a jump associated
+	     with a REG_CROSSING_JUMP note.  */
 	  if (JUMP_P (insn)
-	      && find_reg_note (insn, REG_NON_LOCAL_GOTO, NULL_RTX))
+	      && (find_reg_note (insn, REG_NON_LOCAL_GOTO, NULL_RTX)
+		  || find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX)))
 	    continue;
 
 	  for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
Comment 3 Kazumoto Kojima 2008-02-16 00:24:44 UTC
Steven pointed out that SH back end should just not accept this
substitution.  I'll come back with such a target patch.
Comment 4 Kazumoto Kojima 2008-02-20 23:36:20 UTC
Subject: Bug 35190

Author: kkojima
Date: Wed Feb 20 23:35:41 2008
New Revision: 132502

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132502
Log:
	PR target/35190
	* config/sh/sh.md (jump_compact): Disable for crossing jumps.

	* config/sh/sh.c (find_barrier): Don't go past
	NOTE_INSN_SWITCH_TEXT_SECTIONS note.


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

Comment 5 Kazumoto Kojima 2008-03-09 23:30:27 UTC
Subject: Bug 35190

Author: kkojima
Date: Sun Mar  9 23:29:49 2008
New Revision: 133064

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133064
Log:
	Backport from mainline:
	PR target/35190
	* config/sh/sh.md (jump_compact): Disable for crossing jumps.

	* config/sh/sh.c (find_barrier): Don't go past
	NOTE_INSN_SWITCH_TEXT_SECTIONS note.


Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/sh/sh.c
    branches/gcc-4_3-branch/gcc/config/sh/sh.md

Comment 6 Kazumoto Kojima 2008-03-09 23:44:21 UTC
fixed.