[Bug target/59343] New: miscompiled for loop in sh4 target (-Os)

[Bug target/59343] New: miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

            Bug ID: 59343
           Summary: miscompiled for loop in sh4 target (-Os)
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gcc-bugzilla-f5d8 at theblacksun dot eu

Created attachment 31327
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31327&action=edit
miscompilation testcase

The attached testcase miscompiles on sh4 target if build with -Os

$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -o for_loop_miscompile_O2 -O2
for_loop_miscompile.cpp
$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -o for_loop_miscompile_Os -Os
for_loop_miscompile.cpp

target $ ./for_loop_miscompile_O2
teststring = 0x411068
teststring = 0x411068
target $ ./for_loop_miscompile_Os
teststring = 0x411068
teststring = 0x1

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

--- Comment #1 from gcc-bugzilla-f5d8 at theblacksun dot eu ---
Created attachment 31329
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31329&action=edit
assembler output from -Os

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

--- Comment #2 from gcc-bugzilla-f5d8 at theblacksun dot eu ---
Created attachment 31330
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31330&action=edit
assembler output from -O2

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

--- Comment #3 from gcc-bugzilla-f5d8 at theblacksun dot eu ---
Created attachment 31349
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31349&action=edit
Second failing testcase. Triggers in -O1 and -O2 too

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

gcc-bugzilla-f5d8 at theblacksun dot eu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |sh4-unknown-linux-gnu

--- Comment #4 from gcc-bugzilla-f5d8 at theblacksun dot eu ---
Using the second testcase the compiler also misbehaves:

$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -static -o testcase2_O2 -O2
testcase2.cpp
$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -static -o testcase2_O1 -O1
testcase2.cpp
$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -static -o testcase2_O0 -O0
testcase2.cpp
$ sh4-unknown-linux-gnu-g++ -Wall -Wextra -static -o testcase2_Os -Os
testcase2.cpp

$ qemu-sh4 ./testcase2_00
flag1 is true... flag2?: 0

$ qemu-sh4 ./testcase2_01
flag1 is true... flag2?: 0
flag2 is true

$ qemu-sh4 ./testcase2_02
flag1 is true... flag2?: 0
flag2 is true

$ qemu-sh4 ./testcase2_0s
flag1 is true... flag2?: 0
flag2 is true

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2013-12-3
                 CC|                            |olegendo at gcc dot gnu.org

--- Comment #5 from chrbr at gcc dot gnu.org ---
seems a wrong T-bit optimization.

we have 

.L3:
tst    r1,r1
    bt

replaced by
.L3:
    bf    .L4

although the r1 value can have defs reached from several path.

This has been fixed by Oleg in the 4.9 with a new cflow pass. In 4.8 I think
it's best to just disable it as unsafe, The following workaround is enough to
to fix the problem:

Index: sh.md
===================================================================
--- sh.md       (revision 205585)
+++ sh.md       (working copy)
@@ -8380,7 +8380,7 @@ label:
 {
   return output_branch (sh_eval_treg_value (operands[1]), insn, operands);
 }
-  "&& 1"
+  "&& 0"
   [(set (pc) (if_then_else (eq (reg:SI T_REG) (match_dup 2))
                           (label_ref (match_dup 0))
                           (pc)))]

do you agree Oleg ?

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[gcc-bugzilla-f5d8 at theblacksun dot eu]

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

gcc-bugzilla-f5d8 at theblacksun dot eu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.7.3

--- Comment #6 from gcc-bugzilla-f5d8 at theblacksun dot eu ---
I tested your patch and it fixes the problem for me. Thanks for the quick
feedback.

Meanwhile I tested gcc 4.7.3. I cannot reproduce the miscompilation there.

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to chrbr from comment #5)
> This has been fixed by Oleg in the 4.9 with a new cflow pass. In 4.8 I think
> it's best to just disable it as unsafe, The following workaround is enough
> to to fix the problem:
> 
> Index: sh.md
> ===================================================================
> --- sh.md       (revision 205585)
> +++ sh.md       (working copy)
> @@ -8380,7 +8380,7 @@ label:
>  {
>    return output_branch (sh_eval_treg_value (operands[1]), insn, operands);
>  }
> -  "&& 1"
> +  "&& 0"
>    [(set (pc) (if_then_else (eq (reg:SI T_REG) (match_dup 2))
>                            (label_ref (match_dup 0))
>                            (pc)))]
> 
> do you agree Oleg ?

This has been mentioned in PR 51244 already, I think.

It should not be necessary to disable the T bit optimization completely, only
the cases where there are multiple non-fallthrough basic blocks (i.e. where
there's a label in between).
Something like the patch in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51244#c64 should do.

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[olegendo at gcc dot gnu.org]

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

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Fri Dec  6 10:46:53 2013
New Revision: 205734

URL: http://gcc.gnu.org/viewcvs?rev=205734&root=gcc&view=rev
Log:
    PR target/51244
    PR target/59343
    * config/sh/sh.md (*cbranch_t): Check that there are no labels between
    the s1 insn and the testing insn.  Remove REG_DEAD note    from s1 insn.

    PR target/51244
    PR target/59343
    * gcc.target/sh/pr51244-19.c: Adjust test case.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/sh.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/sh/pr51244-19.c

[Bug target/59343] miscompiled for loop in sh4 target (-Os)

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

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

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #9)
> Author: olegendo
> Date: Fri Dec  6 10:46:53 2013
> New Revision: 205734
> 
> URL: http://gcc.gnu.org/viewcvs?rev=205734&root=gcc&view=rev
> Log:
> 	PR target/51244
> 	PR target/59343
> 	* config/sh/sh.md (*cbranch_t): Check that there are no labels between
> 	the s1 insn and the testing insn.  Remove REG_DEAD note	from s1 insn.
> 
> 	PR target/51244
> 	PR target/59343
> 	* gcc.target/sh/pr51244-19.c: Adjust test case.
> 
> 
> Modified:
>     branches/gcc-4_8-branch/gcc/ChangeLog
>     branches/gcc-4_8-branch/gcc/config/sh/sh.md
>     branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
>     branches/gcc-4_8-branch/gcc/testsuite/gcc.target/sh/pr51244-19.c

Should be fixed on 4.8 now, too.
If there are any further issues regarding T bit optimizations please drop a
comment in PR 51244.
>From gcc-bugs-return-436841-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Dec 06 13:02:39 2013
Return-Path: <gcc-bugs-return-436841-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 23374 invoked by alias); 6 Dec 2013 13:02:39 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 23347 invoked by uid 48); 6 Dec 2013 13:02:36 -0000
From: "redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libstdc++/59406] functions labelled FNV-1a in tr1/functional_hash.h are not FNV-1a
Date: Fri, 06 Dec 2013 13:02:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libstdc++
X-Bugzilla-Version: 4.7.2
X-Bugzilla-Keywords:
X-Bugzilla-Severity: minor
X-Bugzilla-Who: redi at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-59406-4-moWAEaaMJn@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-59406-4@http.gcc.gnu.org/bugzilla/>
References: <bug-59406-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-12/txt/msg00496.txt.bz2
Content-length: 184

http://gcc.gnu.org/bugzilla/show_bug.cgi?idY406

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We're not really making any non-critical changes to TR1 code now.