Bug 41813 - [4.5 Regression] SH: delay slot is filled with a wrong insn
Summary: [4.5 Regression] SH: delay slot is filled with a wrong insn
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.0
: P4 normal
Target Milestone: 4.5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-10-24 00:46 UTC by Kazumoto Kojima
Modified: 2009-10-25 23:14 UTC (History)
1 user (show)

See Also:
Host:
Target: sh4-unknown-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kazumoto Kojima 2009-10-24 00:46:10 UTC
gcc.dg/pr28796-2.c execution test fails on sh for a while.
There is a wrong code like

	fcmp/eq	dr2,dr4
	bf/s	.L93
	fcmp/gt	dr2,dr4
	bra	.L92
	movt	r2

which should be

        fcmp/eq dr2,dr4
        bf/s    .L53
        movt    r2
.L92:
	...

.L53:
        fcmp/gt dr2,dr4
.L93:

i.e. a delay slot is filled with a wrong insn.  It stated
to fail between revision 152230 and 152317 but it seems
there are no related changes.  Perhaps some changes revealed
this latent problem.
Comment 1 Kazumoto Kojima 2009-10-24 00:48:14 UTC
For a slightly reduced case, the insns before dbr pass are
configured like as:

(insn 460 456 461
  (parallel [(set (reg:SI 147 t)
               (eq:SI (reg:DF 68 fr4) (reg:DF 66 fr2)))
             (use (reg/v:PSI 151 ))]) {cmpeqdf_t})

(jump_insn 461 460 575
  (set (pc) (if_then_else (eq (reg:SI 147 t) (const_int 0))
      	      (label_ref:SI 571) (pc))) {branch_false})

(insn 575 461 576
  (parallel [(set (pc) (unspec [(const_int 200) (pc)] 4))
             (set (reg:SI 147 t) (const_int 1 [0x1]))]) {stuff_delay_slot})

(insn 576 575 572
  (set (pc) (unspec [(const_int 200)] 4)))

(jump_insn 572 576 573
  (set (pc) (label_ref 200)))

...
(code_label 200 198 263 25 "" [1 uses])

(note 263 200 202 [bb 20] NOTE_INSN_BASIC_BLOCK)

(insn 202 263 203
  (set (reg:QI 2 r2) (reg:QI 147 t)) {movqi_i})

...
(code_label 571 573 501 85 "" [1 uses])

(note 501 571 502 [bb 32] NOTE_INSN_BASIC_BLOCK)

(jump_insn 502 501 503
  (set (pc) (label_ref 471)) {jump_compact})
...
(code_label 471 497 262 53 "" [2 uses])

(note 262 471 199 [bb 29] NOTE_INSN_BASIC_BLOCK)

(insn 199 262 449
  (parallel [(set (reg:SI 147 t)
               (gt:SI (reg:DF 68 fr4) (reg:DF 66 fr2)))
             (use (reg/v:PSI 151 ))]) {cmpgtdf_t})

and fill_slots_from_thread takes insn 199 as an insn
filling the delay slot of jump_insn 461.  When this is done,
the opposite thread is started with stuff_delay_slot having
(set (reg T_REG) (const_int 1)) sub-pattern which makes
the live register analysis consider T_REG is defined here.
Thus fill_slots_from_thread takes insn 199 which changes
T_REG in spite of insn 202 in the opposite thread which
expects the original T_REG value.  It seems that the pattern
(set (reg T_REG) ... in stuff_delay_slot insn is the cause
of this problem and stuff_delay_slot is simply used as
a unique marker.  I'm testing the patch below.

--- ORIG/trunk/gcc/config/sh/sh.md	2009-08-13 09:46:10.000000000 +0900
+++ trunk/gcc/config/sh/sh.md	2009-10-23 17:07:19.000000000 +0900
@@ -6825,7 +6825,7 @@ label:
 (define_insn "stuff_delay_slot"
   [(set (pc)
 	(unspec [(match_operand:SI 0 "const_int_operand" "") (pc)] UNSPEC_BBR))
-   (set (reg:SI T_REG) (match_operand:SI 1 "const_int_operand" ""))]
+   (match_operand:SI 1 "const_int_operand" "")]
   "TARGET_SH1"
   ""
   [(set_attr "length" "0")
Comment 2 Kazumoto Kojima 2009-10-25 23:10:39 UTC
Subject: Bug 41813

Author: kkojima
Date: Sun Oct 25 23:10:22 2009
New Revision: 153543

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153543
Log:
	PR target/41813
	* config/sh/sh.md (stuff_delay_slot): Don't set T_REG in pattern.


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

Comment 3 Kazumoto Kojima 2009-10-25 23:14:08 UTC
Fixed.