[Bug ipa/65765] New: Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[Bug ipa/65765] New: Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[mh+gcc at glandium dot org]

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

            Bug ID: 65765
           Summary: Compiling Firefox with GCC 5 leads to broken
                    javascript engine on x86-64
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mh+gcc at glandium dot org

Created attachment 35314
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35314&action=edit
reduced.tgz

When building Firefox with GCC 5 [1], some parts of the test suite fail because
of what might be a compiler bug. It seems to be involving
__attribute__((cold)). Dan Gohman got a reduced testcase, attached here.

Unpack and build with g++ -O2 *.cpp, then run a.out.

This doesn't happen at -O1, or with -O2 -Dcold=.

1. GCC 5.1.0-RC-20150412, compiled with "--enable-languages=c,c++ 
--disable-nls --disable-gnu-unique-object --enable-__cxa_atexit
--with-arch-32=pentiumpro".
I've also reproduced with the reduced testcase with a snapshot of 5-20150111
(building Firefox with that snapshot fails for other reasons, unfortunately).

[Bug ipa/65765] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[mh+gcc at glandium dot org]

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

--- Comment #1 from Mike Hommey <mh+gcc at glandium dot org> ---
git bisect identified this commit as the first one where the reduced testcase
fails:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=52200d03c231f0bddbd4bbc5cd3608c6a1dd4598

svn+ssh://gcc.gnu.org/svn/gcc/trunk@216305

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[trippels at gcc dot gnu.org]

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

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-15
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |trippels at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|Compiling Firefox with GCC  |[5/6 Regression] Compiling
                   |5 leads to broken           |Firefox with GCC 5 leads to
                   |javascript engine on x86-64 |broken javascript engine on
                   |                            |x86-64

--- Comment #2 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Confirmed. -fno-ipa-icf "fixes" the issue.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[trippels at gcc dot gnu.org]

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

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-15
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |trippels at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|Compiling Firefox with GCC  |[5/6 Regression] Compiling
                   |5 leads to broken           |Firefox with GCC 5 leads to
                   |javascript engine on x86-64 |broken javascript engine on
                   |                            |x86-64

--- Comment #2 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Confirmed. -fno-ipa-icf "fixes" the issue.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[trippels at gcc dot gnu.org]

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

--- Comment #3 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
markus@x4 tc % cat main.ii
int a, b, c, d, e;
unsigned char h[] = { 1, 1 };

__attribute__ ((cold)) int ModRM_Mode () { return a; }

int
ModRM_RM (int p1)
{
  return p1;
}

__attribute__ ((cold)) static bool ModRM_hasSIB (unsigned char p1)
{
  return ModRM_Mode () != 1 && ModRM_RM (p1);
}

__attribute__ ((cold)) static bool ModRM_hasRIP (unsigned char p1)
{
  return ModRM_Mode () && ModRM_RM (p1);
}

unsigned char *
DisassembleHeapAccess (unsigned char *p1)
{
  b = *p1++;
  if (ModRM_hasSIB (b))
    c = *p1++;
  int f = c, g = 0;
  d = ModRM_hasRIP (g);
  e = f == 0;
  if (e)
    p1 += sizeof 0;
  return p1;
}

int
main ()
{
  DisassembleHeapAccess (h) == h + 2 ? static_cast<void> (0)
                                     : __builtin_abort ();
}

markus@x4 tc % g++ -O2 main.ii && ./a.out
[1]    19854 abort      ./a.out
markus@x4 tc % g++ -O2 -fno-ipa-icf main.ii && ./a.out
markus@x4 tc %

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
             Status|NEW                         |ASSIGNED
                 CC|                            |jakub at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Target Milestone|---                         |5.0

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Testing the obvious fix.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[mh+gcc at glandium dot org]

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

--- Comment #5 from Mike Hommey <mh+gcc at glandium dot org> ---
I can confirm that building Firefox with -fno-ipa-icf "fixes" the issue as well
(that is, that the testcase is correctly related to the Firefox breakage)

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 35316
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35316&action=edit
gcc5-pr65765.patch

Untested fix.  The main bug has been a return true; for
GIMPLE_NOP/GIMPLE_PREDICT, that means compare_bb ignored anything in the rest
of the basic block, which is obviously completely wrong.
While looking at it, I think we should compare for GIMPLE_PREDICT the
predictor/outcome, if somebody puts an effort to mark something as hot or cold,
unifying it if they differ sounds wrong to me.  And similarly, IMHO for
GIMPLE_EH_DISPATCH we should compare the region number.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Looks good to me, though the "obvious" part alone would be fine for 5.1 as well
(even obvious - heh).

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #8 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Seems fine to me, thanks for looking into it! (I jsut arrived to china).
It is safe to ignore GIMPLE_PREDICT at ipa-ICF time. Noone is using it
afterwards.  We are stripping it at some point, I am surprised it is not done
before ICF's analysis.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[hubicka at gcc dot gnu.org]

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

--- Comment #9 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
GIMPLE_PREDICT at this time is already converted to the profile (just to
explain).  If we start to do tail merging in early opts, matching gimple
predict before profile is built is probably good idea.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I can certainly change that hunk to just do break; for GIMPLE_PREDICT (except
for the already bootstrapped and in progress tested patch, which I'd have to
redo).
Richard suggested that perhaps for 5.1 it would be safer to just do the break
for GIMPLE_PREDICT/GIMPLE_NOP and leave the rest for 6/5.2.
I'm worried about the GIMPLE_EH_DISPATCH missing comparison in that case
though.
compare_bb compares the EH regions the stmts are in, but not the EH_DISPATCH
argument.

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed Apr 15 11:47:44 2015
New Revision: 222123

URL: https://gcc.gnu.org/viewcvs?rev=222123&root=gcc&view=rev
Log:
    PR ipa/65765
    * ipa-icf-gimple.c (func_checker::compare_bb): For GIMPLE_NOP
    and GIMPLE_PREDICT use break instead of return true. For
    GIMPLE_EH_DISPATCH, compare dispatch region.

    * g++.dg/ipa/pr65765.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/pr65765.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-icf-gimple.c
    trunk/gcc/testsuite/ChangeLog

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed Apr 15 12:09:56 2015
New Revision: 222124

URL: https://gcc.gnu.org/viewcvs?rev=222124&root=gcc&view=rev
Log:
    PR ipa/65765
    * ipa-icf-gimple.c (func_checker::compare_bb): For GIMPLE_NOP
    and GIMPLE_PREDICT use break instead of return true. For
    GIMPLE_EH_DISPATCH, compare dispatch region.

    * g++.dg/ipa/pr65765.C: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/ipa/pr65765.C
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/ipa-icf-gimple.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog

[Bug ipa/65765] [5/6 Regression] Compiling Firefox with GCC 5 leads to broken javascript engine on x86-64

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed for 5.1+.