[Bug target/27182] New: [4.1 regression] SH: wrong-code generation

[Bug target/27182] New: [4.1 regression] SH: wrong-code generation

[sugioka at itonet dot co dot jp]

Hi,
gcc-4.1.0 miscompiles on sh4-linux target.
Attached testcase does not trigger this bug on mainline,
and also correctly compiled with gcc-3.4.5.

For attached testcase, source line 514 is incorrectly compiled with
following command line.

g++ -O2 -g -fno-strict-aliasing -fwrapv \
   -fno-implicit-templates -fno-exceptions -fno-rtti -S -o bad.s sql_select4.ii

incorrect result:
        .loc 1 515 0
        mov.l   @(52,r14),r2
        .loc 1 514 0
        add     #12,r1
        .loc 1 515 0
        mov.l   r2,@-r1
        .loc 1 514 0
        mov     #0,r2
        mov.l   r2,@r1          <===== #0 is stored incorrect address
                                        ("add #4,r1" is needed bofore this
line)

Adding -fno-schedule-insns fixes this.
g++ -O2 -g -fno-schedule-insns -fno-strict-aliasing -fwrapv \
   -fno-implicit-templates -fno-exceptions -fno-rtti -S -o good.s
sql_select4.ii

correct result:
        mov     #0,r2
        .....
        fmov.s  fr6,@r1
        fmov.s  fr7,@-r1
        .loc 1 514 0
        add     #12,r1
        mov.l   r2,@r1
        .loc 1 515 0
        mov.l   @(52,r14),r7
        mov.l   r7,@-r1

On mainline, this problem disapeares after following patch.
2005-11-30  Paolo Bonzini  <bonzini@gnu.org>

        * simplify-rtx.c (simplify_plus_minus): Remove final parameter.
        Always produce an output if we can remove NEGs or canonicalize
        (minus (minus ...)) expressions.  Provide a fast path for the
        two-operand case.
        (simplify_gen_binary): Do not call simplify_plus_minus.
        (simplify_binary_operation_1): Reassociate at the end of the
        function.


-- 
           Summary: [4.1 regression] SH: wrong-code generation
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: sugioka at itonet dot co dot jp
 GCC build triplet: sh4-unknown-linux
  GCC host triplet: sh4-unknown-linux
GCC target triplet: sh4-unknown-linux


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[sugioka at itonet dot co dot jp]

------- Comment #1 from sugioka at itonet dot co dot jp  2006-04-17 13:19 -------
Created an attachment (id=11282)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=11282&action=view)
testcase


-- 


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[kkojima at gcc dot gnu dot org]

------- Comment #2 from kkojima at gcc dot gnu dot org  2006-04-19 12:42 -------
I'd like to add Joern to the CC list.

I've looked the rtl dumps for the testcase.  It seems that
the wrong code is generated during the peephole2 optimization.
The corresponding part of .41.csa dump is:

(insn:HI 1275 1768 2254 181 sql_select4.cc:515 (set (reg/f:SI 2 r2 [670])
        (reg:SI 1 r1 [656])) 172 {movsi_ie} (insn_list:REG_DEP_TRUE 1267 (nil))
    (nil))

  (insn 2254 1275 1276 181 sql_select4.cc:515 (set (reg:SI 6 r6)
        (mem/f/c:SI (plus:SI (reg/f:SI 14 r14)
                (const_int 52 [0x34])) [292 s+0 S4 A32])) 172 {movsi_ie} (nil)
    (nil))

(insn:HI 1276 2254 1769 181 sql_select4.cc:515 (set (mem/s/f:SI (pre_dec:SI
(reg/f:SI 2 r2 [670])) [6 <variable>.table+0 S4 A32])
        (reg:SI 6 r6)) 172 {movsi_ie} (insn_list:REG_DEP_TRUE 1275 (nil))
    (expr_list:REG_DEAD (reg:SI 6 r6)
        (expr_list:REG_UNUSED (reg/f:SI 2 r2 [670])
            (expr_list:REG_INC (reg/f:SI 2 r2 [670])
                (nil)))))

(note 1769 1276 1772 181 ("sql_select4.cc") 514)

(insn 1772 1769 1269 181 sql_select4.cc:514 (set (reg:SI 2 r2 [666])
        (const_int 0 [0x0])) 172 {movsi_ie} (nil)
    (expr_list:REG_EQUIV (const_int 0 [0x0])
        (nil)))

(insn:HI 1269 1772 1770 181 sql_select4.cc:514 (set (mem/s/f:SI (reg:SI 1 r1
[656]) [16 <variable>.key+0 S4 A32])
        (reg:SI 2 r2 [666])) 172 {movsi_ie} (insn_list:REG_DEP_TRUE 1268
(insn_list:REG_DEP_TRUE 1267 (nil)))
    (expr_list:REG_DEAD (reg:SI 2 r2 [666])
        (expr_list:REG_DEAD (reg:SI 1 r1 [656])
            (expr_list:REG_EQUAL (const_int 0 [0x0])
                (nil)))))

and peephole2 changes these in .42.peephole2 to:

(insn 2640 1768 2641 181 sql_select4.cc:515 (set (reg/f:SI 2 r2 [670])
        (mem/f/c:SI (plus:SI (reg/f:SI 14 r14)
                (const_int 52 [0x34])) [292 s+0 S4 A32])) 172 {movsi_ie} (nil)
    (nil))

(insn 2641 2640 1769 181 sql_select4.cc:515 (set (mem/s/f:SI (pre_dec:SI
(reg:SI 1 r1 [656])) [6 <variable>.table+0 S4 A32])
        (reg/f:SI 2 r2 [670])) 172 {movsi_ie} (nil)
    (expr_list:REG_DEAD (reg/f:SI 2 r2 [670])
        (nil)))

(note 1769 2641 1772 181 ("sql_select4.cc") 514)

(insn 1772 1769 1269 181 sql_select4.cc:514 (set (reg:SI 2 r2 [666])
        (const_int 0 [0x0])) 172 {movsi_ie} (nil)
    (expr_list:REG_EQUIV (const_int 0 [0x0])
        (nil)))

(insn:HI 1269 1772 1770 181 sql_select4.cc:514 (set (mem/s/f:SI (reg:SI 1 r1
[656]) [16 <variable>.key+0 S4 A32])
        (reg:SI 2 r2 [666])) 172 {movsi_ie} (insn_list:REG_DEP_TRUE 1268
(insn_list:REG_DEP_TRUE 1267 (nil)))
    (expr_list:REG_DEAD (reg:SI 2 r2 [666])
        (expr_list:REG_DEAD (reg:SI 1 r1 [656])
            (expr_list:REG_EQUAL (const_int 0 [0x0])
                (nil)))))

It looks the define_peephole2 movsicc_true+3

(define_peephole2
  [(set (match_operand 0 "any_register_operand" "")
        (match_operand 1 "any_register_operand" ""))
   (set (match_operand 2 "any_register_operand" "") (match_operand 3 "" ""))
   (set (match_operand 4 "" "") (match_operand 5 "" ""))]
  "(HARD_REGNO_NREGS (REGNO (operands[0]), GET_MODE (operands[2]))
    <= HARD_REGNO_NREGS (REGNO (operands[0]), GET_MODE (operands[0])))
   && peep2_reg_dead_p (3, operands[0]) && peep2_reg_dead_p (3, operands[2])
   && ! reg_overlap_mentioned_p (operands[0], operands[3])
   && ! reg_overlap_mentioned_p (operands[2], operands[0])
   && ! reg_overlap_mentioned_p (operands[0], operands[1])
   && (REGNO_REG_CLASS (REGNO (operands[0]))
       == REGNO_REG_CLASS (REGNO (operands[2])))
   && (REGNO_REG_CLASS (REGNO (operands[1]))
       == REGNO_REG_CLASS (REGNO (operands[0])))"
  [(set (match_dup 0) (match_dup 3))
   (set (match_dup 4) (match_dup 5))]

causes this change.  In the problematic case, (reg/f:SI 2 r2 [670])
in (mem/s/f:SI (pre_dec:SI (reg/f:SI 2 r2 [670])) is replaced with
(reg:SI 1 r1 [656]) and results a wrong r1 value.  Perhaps this
issue is simply latent in the mainline, though I have no testcases
for the mainline.  I'm testing the patch below to prevent this
peephole2 optimization for such situation.

diff -uprN ORIG/gcc-4_1-branch/gcc/config/sh/sh.md
LOCAL/gcc-4_1-branch/gcc/config/sh/sh.md
--- ORIG/gcc-4_1-branch/gcc/config/sh/sh.md     2006-02-06 21:07:13.000000000
+0900
+++ LOCAL/gcc-4_1-branch/gcc/config/sh/sh.md    2006-04-18 09:06:47.000000000
+0900
@@ -1094,6 +1094,8 @@
   "(HARD_REGNO_NREGS (REGNO (operands[0]), GET_MODE (operands[2]))
     <= HARD_REGNO_NREGS (REGNO (operands[0]), GET_MODE (operands[0])))
    && peep2_reg_dead_p (3, operands[0]) && peep2_reg_dead_p (3, operands[2])
+   && ! FIND_REG_INC_NOTE (peep2_next_insn (2), operands[0])
+   && ! FIND_REG_INC_NOTE (peep2_next_insn (2), operands[2])
    && ! reg_overlap_mentioned_p (operands[0], operands[3])
    && ! reg_overlap_mentioned_p (operands[2], operands[0])
    && ! reg_overlap_mentioned_p (operands[0], operands[1])


-- 

kkojima at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu dot
                   |                            |org, amylaar at gcc dot gnu
                   |                            |dot org


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[amylaar at gcc dot gnu dot org]

------- Comment #3 from amylaar at gcc dot gnu dot org  2006-04-19 19:35 -------
(In reply to comment #2)
> I'd like to add Joern to the CC list.
> 
> I've looked the rtl dumps for the testcase.  It seems that
> the wrong code is generated during the peephole2 optimization.
...
> (reg:SI 1 r1 [656]) and results a wrong r1 value.  Perhaps this
> issue is simply latent in the mainline, though I have no testcases
> for the mainline.

Well, a latent bug can exist even if no test case can currently trigger it.
If could be triggered again after some other optimizer change.

Your patch is OK for 4.1 branch and mainline. (With the addition of a
ChangeLog entry.)


-- 


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[kkojima at gcc dot gnu dot org]

------- Comment #4 from kkojima at gcc dot gnu dot org  2006-04-19 22:41 -------
Thanks.  I'll commit it with the ChangeLog entry below if the usual
bootstrap/regtest on sh4-unknown-linux-gnu is done successfully.

        PR target/27182
        * config/sh/sh.md (movsicc_true+3): Tweak condition for the case
        that the last insn has the REG_INT note.


-- 


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[kkojima at gcc dot gnu dot org]

------- Comment #5 from kkojima at gcc dot gnu dot org  2006-04-20 01:54 -------
Subject: Bug 27182

Author: kkojima
Date: Thu Apr 20 01:54:20 2006
New Revision: 113109

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113109
Log:
        PR target/27182
        * config/sh/sh.md (movsicc_true+3): Tweak condition for the case
        that the last insn has the REG_INC note.


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


-- 


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[kkojima at gcc dot gnu dot org]

------- Comment #6 from kkojima at gcc dot gnu dot org  2006-04-20 02:03 -------
Subject: Bug 27182

Author: kkojima
Date: Thu Apr 20 02:03:47 2006
New Revision: 113110

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113110
Log:
        PR target/27182
        * config/sh/sh.md (movsicc_true+3): Tweak condition for the case
        that the last insn has the REG_INC note.


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


-- 


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

[Bug target/27182] [4.1 regression] SH: wrong-code generation

[jsm28 at gcc dot gnu dot org]

------- Comment #7 from jsm28 at gcc dot gnu dot org  2006-06-04 19:45 -------
Patch committed, so I presume fixed.


-- 

jsm28 at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.1.1


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