[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[olegendo at gcc dot gnu.org]

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

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to John Paul Adrian Glaubitz from comment #0)
> As previously discussed in private mail, I am now filing a bug report for
> the regression in gcc-5 that was introduced somewhere between r222550 and
> r225710 which leads to the miscompilation of gcc/gengtype.c when building a
> native compiler on SH [1]:

Looking at the SVN log for gcc-5-branch:
   svn log -r222550:r225710

shows the following SH specific commits...

------------------------------------------------------------------------
r223722 | kkojima | 2015-05-27 07:37:06 +0900 (Wed, 27 May 2015) | 5 lines

PR target/65979
* config/sh/sh.md (tstsi_t peephole2): Use gen_rtx_SET and take into
  account the case that operands[1] and operands[2] are the same register.

------------------------------------------------------------------------
r225094 | olegendo | 2015-06-27 09:46:58 +0900 (Sat, 27 Jun 2015) | 9 lines

gcc/
        Backport from mainline
        2015-06-25  Oleg Endo  <olegendo@gcc.gnu.org>

        PR target/65979
        PR target/66611
        * config/sh/sh.md (tstsi_t peephole2): Use insn_invalid_p to check if
        the replacement insn will work.

------------------------------------------------------------------------
r225660 | kkojima | 2015-07-10 18:50:18 +0900 (Fri, 10 Jul 2015) | 5 lines

PR target/66780
* config/sh/sh.md (symGOT_load): Revert a part of 2015-03-03
change for target/65249.

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #2 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to Oleg Endo from comment #1)
> (In reply to John Paul Adrian Glaubitz from comment #0)
> > As previously discussed in private mail, I am now filing a bug report for
> > the regression in gcc-5 that was introduced somewhere between r222550 and
> > r225710 which leads to the miscompilation of gcc/gengtype.c when building a
> > native compiler on SH [1]:
> 
> Looking at the SVN log for gcc-5-branch:
>    svn log -r222550:r225710

r223346 isn't affected either, 5.1.1-6 is still building and already got past
gengtype.c in stage 2.

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #3 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to John Paul Adrian Glaubitz from comment #2)
> r223346 isn't affected either, 5.1.1-6 is still building and already got
> past gengtype.c in stage 2.

Going through the other logs while r223346 is still building, r224724 *is*
affected. So unless the bug is somewhere else, it has to be an issue with 
PR/65979 or PR/66611.

Will test that once the current build of r223346 has finished.

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #4 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to John Paul Adrian Glaubitz from comment #3)
> Will test that once the current build of r223346 has finished.

Another hint is a changelog entry for the Debian package 5.1.1-11:

 * Build with -O1 on sh4 (try to work around PR target/66358).

So it *might* be a combination of PR/65979 and using -O1 instead of -O2.

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #5 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
As expected, r223346 went as far as r222550 and ran into the same bug which is
PR/65979.

I am now building r225710, however with -O2 and not -O1 unlike what's currently
set in the Debian package source for sh4. I expect it to run into PR/66358.

Currently, my suspicion is that this particular bug is actually PR/66358 but it
just shows differently with -O1 by miscompiling the code instead of resulting
into an ICE. If my theory is correct, we should see an ICE instead of
gengtype.c being miscompiled.

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[kkojima at gcc dot gnu.org]

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

--- Comment #6 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 36040
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36040&action=edit
.i file for gengtype.c

I've confirmed a miscompile for gengtype.c with -O1 on my 5/6
compilers.  With them,

     if (union_p)
       {
         oprintf (d->of, "%*sbreak;\n", d->indent, "");
         d->indent -= 2;
       }

lines in gengtype.c:walk_type function are compiled like as:

        bf      .L2253
        mov.l   .L2593,r7
        mov.l   @(36,r12),r6
        mov.l   .L2580,r5
        mov.l   .L2595,r1
        jsr     @r1
        ...

i.e. the instruction testing union_p variable is removed.
The resulted gengtype produces files without "break" in many
cases.  Although it's the other way around with the reported
full of "break" symptom, I think the both are the same issue.
The deletion has happened in sh_split_movrt_negc_to_movt_xor
which is called by movrt_negc insn_and_split.  It seems that
that splitting is applied for the case

        tst     reg,reg
        mov     #-1,reg1
        negc    reg1,reg1
        ...
        call ...
        ...
        tst     reg,reg

and the last tst was removed.  Oleg, could you take a look into
this?

BTW, I guess that

> * Build with -O1 on sh4 (try to work around PR target/66358).

will open a can of worms :-)

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #7 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to Kazumoto Kojima from comment #6)
> Created attachment 36040 [details]
> .i file for gengtype.c
> 
> I've confirmed a miscompile for gengtype.c with -O1 on my 5/6
> compilers.

Just for confirmation. Removing -O1 and defaulting to -O2 again lets the
current gcc-5 snapshot build past this particular issue.

Interesting that this is triggered with -O1 and not -O2. I'd expected that to
be the other way around.

> BTW, I guess that
> 
> > * Build with -O1 on sh4 (try to work around PR target/66358).
> 
> will open a can of worms :-)

What do you mean? Using -O1 will trigger all kinds of bugs? Or is it rather
about PR target/66358?

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[kkojima at gcc dot gnu.org]

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

--- Comment #8 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to John Paul Adrian Glaubitz from comment #7)
> What do you mean? Using -O1 will trigger all kinds of bugs? Or is it rather
> about PR target/66358?

Just my 2 cents.  Even on the primary targets, bootstrap with other
than '-g -O2' are not so well tested.  Most packages are built with
-O2, aren't they?  After all, -O1 should work but not safer than -O2,
IMO.  O2 does many optimizations, and some of them might cause bug.
OTOH, some of them simplify the intermediate code.  With -O1, we could
see highly complex intermediate code in some cases and they might cause
problem.  We have just seen an example in this PR.

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[glaubitz at physik dot fu-berlin.de]

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

--- Comment #9 from John Paul Adrian Glaubitz <glaubitz at physik dot fu-berlin.de> ---
(In reply to Kazumoto Kojima from comment #8)
> (In reply to John Paul Adrian Glaubitz from comment #7)
> > What do you mean? Using -O1 will trigger all kinds of bugs? Or is it rather
> > about PR target/66358?
> 
> Just my 2 cents.  Even on the primary targets, bootstrap with other
> than '-g -O2' are not so well tested.

Ah, I didn't know that. Really interesting fact!

> Most packages are built with -O2, aren't they?

They are, true.

> After all, -O1 should work but not safer than -O2,
> IMO.  O2 does many optimizations, and some of them might cause bug.
> OTOH, some of them simplify the intermediate code.  With -O1, we could
> see highly complex intermediate code in some cases and they might cause
> problem.  We have just seen an example in this PR.

I agree and, as I said, I already asked Matthias to revert to -O2.

Let's see how far we'll get this time. My board is still building.

Adrian

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[olegendo at gcc dot gnu.org]

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

--- Comment #11 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> 
> I've added this code as part of PR 63986.  I've checked with make -k
> ...

Sorry, by "this code" I didn't mean the patch in c#10 of this PR, but the
original sh_split_movrt_negc_to_movt_xor related code.

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[kkojima at gcc dot gnu.org]

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

--- Comment #12 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> Index: gcc/config/sh/sh.c
> ===================================================================
> --- gcc/config/sh/sh.c	(revision 225987)
> +++ gcc/config/sh/sh.c	(working copy)
> @@ -13888,6 +13888,7 @@
>        && !sh_insn_operands_modified_between_p (t_before_negc.insn,
>  					       t_before_negc.insn,
>  					       t_after_negc.insn)
> +      && !modified_between_p (get_t_reg_rtx (), curr_insn,
> t_after_negc.insn)
>        && !sh_unspec_insn_p (t_after_negc.insn)
>        && !volatile_insn_p (PATTERN (t_after_negc.insn))
>        && !side_effects_p (PATTERN (t_after_negc.insn))
> 
> I've added this code as part of PR 63986.  I've checked with make -k
> check-gcc RUNTESTFLAGS="sh.exp
> --target_board=sh-sim\{-m2a/-mb,-m4/-ml,-m4/-mb}" that there are no new SH
> specific fails, so I assume the patch above should be OK (although all the
> checks should probably be combined to avoid walking the insns repeatedly). 
> Could you please add it to your test run?

The toplevel "make -k check" on sh4-unknown-linux-gnu is running.
I'll report back when it completes.

BTW, during debugging the issue, I thought that this call-clobbered case
is excluded with sh_find_set_of_reg and found again that NONJUMP_INSN_P
is simply defined as (GET_CODE (X) == INSN).  Each time I've found it,
I've recalled that it isn't (GET_CODE (X) != JUMP_INSN), then the next
time...  Anyway I've tried another one liner

--- a/config/sh/sh-protos.h
+++ b/config/sh/sh-protos.h
@@ -198,7 +198,7 @@ sh_find_set_of_reg (rtx reg, rtx_insn* insn, F stepfunc,
     {
       if (BARRIER_P (result.insn))
        break;
-      if (!NONJUMP_INSN_P (result.insn))
+      if (!NONJUMP_INSN_P (result.insn) && !CALL_P (result.insn))
        continue;
       if (reg_set_p (reg, result.insn))
        {

and it works like as my expectation, though I'm not sure whether it's
a "right thing" or not.
>From gcc-bugs-return-493158-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jul 24 00:24:32 2015
Return-Path: <gcc-bugs-return-493158-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 110005 invoked by alias); 24 Jul 2015 00:24:32 -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 109952 invoked by uid 48); 24 Jul 2015 00:24:27 -0000
From: "eric.niebler at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/66962] [concepts] overloaded function causing memory blow-up and ICE
Date: Fri, 24 Jul 2015 00:24:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c++
X-Bugzilla-Version: c++-concepts
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: eric.niebler at gmail dot com
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution:
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-66962-4-H79CNXSLPu@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66962-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66962-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: 2015-07/txt/msg02048.txt.bz2
Content-length: 241

https://gcc.gnu.org/bugzilla/show_bug.cgi?idf962

--- Comment #9 from Eric Niebler <eric.niebler at gmail dot com> ---
Jason, is there anything I can do in my code to avoid the quadratic explosion
while we wait for Andrew to fix the bug?

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[olegendo at gcc dot gnu.org]

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

--- Comment #13 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #12)
> 
> --- a/config/sh/sh-protos.h
> +++ b/config/sh/sh-protos.h
> @@ -198,7 +198,7 @@ sh_find_set_of_reg (rtx reg, rtx_insn* insn, F stepfunc,
>      {
>        if (BARRIER_P (result.insn))
>  	break;
> -      if (!NONJUMP_INSN_P (result.insn))
> +      if (!NONJUMP_INSN_P (result.insn) && !CALL_P (result.insn))
>  	continue;
>        if (reg_set_p (reg, result.insn))
>  	{
> 
> and it works like as my expectation, though I'm not sure whether it's
> a "right thing" or not.

This would be OK for hardregs (which are clobbered by calls).  When working on
pseudos, it's actually OK to ignore calls.  Maybe it'd be a good idea to extend
sh_find_set_of_reg to check for call clobbered hardregs only.  This will make
the function easier to use.
>From gcc-bugs-return-493160-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jul 24 04:48:30 2015
Return-Path: <gcc-bugs-return-493160-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 105747 invoked by alias); 24 Jul 2015 04:48:30 -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 105705 invoked by uid 55); 24 Jul 2015 04:48:21 -0000
From: "naveenh at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug middle-end/25529] (unsigned * 2)/2 is not changed into unsigned &0x7FFFFFFF
Date: Fri, 24 Jul 2015 04:48:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: middle-end
X-Bugzilla-Version: 4.2.0
X-Bugzilla-Keywords: missed-optimization, TREE
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: naveenh at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution:
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-25529-4-ulTm1TLafl@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-25529-4@http.gcc.gnu.org/bugzilla/>
References: <bug-25529-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: 2015-07/txt/msg02050.txt.bz2
Content-length: 624

https://gcc.gnu.org/bugzilla/show_bug.cgi?id%529

--- Comment #2 from naveenh at gcc dot gnu.org ---
Author: naveenh
Date: Fri Jul 24 04:47:48 2015
New Revision: 226136

URL: https://gcc.gnu.org/viewcvs?rev"6136&root=gcc&view=rev
Log:
PR middle-end/25529
2015-07-24  Naveen H.S  <Naveen.Hurugalawadi@caviumnetworks.com>

gcc/testsuite/ChangeLog:
        * gcc.dg/pr25529.c: New test.

gcc/ChangeLog:
        * match.pd (trunc_div (mult @0 integer_pow2p@1) @1) : New simplifier.


Added:
    trunk/gcc/testsuite/gcc.dg/pr25529.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/match.pd
    trunk/gcc/testsuite/ChangeLog

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[kkojima at gcc dot gnu.org]

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

--- Comment #14 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #12)
> The toplevel "make -k check" on sh4-unknown-linux-gnu is running.
> I'll report back when it completes.

I've confirmed that there are no new failures with the patch in c#10.

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[kkojima at gcc dot gnu.org]

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

--- Comment #15 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #13)
> This would be OK for hardregs (which are clobbered by calls).  When working
> on pseudos, it's actually OK to ignore calls.  Maybe it'd be a good idea to
> extend sh_find_set_of_reg to check for call clobbered hardregs only.  This
> will make the function easier to use.

I see.  Thanks for your explanation.

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[olegendo at gcc dot gnu.org]

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

--- Comment #16 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #14)
> (In reply to Kazumoto Kojima from comment #12)
> > The toplevel "make -k check" on sh4-unknown-linux-gnu is running.
> > I'll report back when it completes.
> 
> I've confirmed that there are no new failures with the patch in c#10.

Thanks!  I will commit it to trunk and GCC 5 branch.


(In reply to Kazumoto Kojima from comment #6)
> 
> BTW, I guess that
> 
> > * Build with -O1 on sh4 (try to work around PR target/66358).
> 
> will open a can of worms :-)

Yeah, but it revealed a bug, so it was good in this case :)

[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

[olegendo at gcc dot gnu.org]

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

--- Comment #18 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sat Jul 25 14:13:48 2015
New Revision: 226219

URL: https://gcc.gnu.org/viewcvs?rev=226219&root=gcc&view=rev
Log:
gcc/
        Backport from mainline
        2015-07-25  Oleg Endo  <olegendo@gcc.gnu.org>

        PR target/66930
        * config/sh/sh.c (sh_split_movrt_negc_to_movt_xor): Add missing
        T bit register modified_between_p check.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/sh/sh.c