Bug 36917 - [4.3 regression] miscompilation with -O2 and r136501
Summary: [4.3 regression] miscompilation with -O2 and r136501
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.2
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2008-07-24 09:04 UTC by Matthias Klose
Modified: 2008-07-25 13:54 UTC (History)
2 users (show)

See Also:
Host:
Target: i486-linux-gnu
Build:
Known to work: 4.3.1
Known to fail: 4.3.2
Last reconfirmed: 2008-07-24 09:31:51


Attachments
preprocessed source (12.62 KB, application/x-gzip)
2008-07-24 09:05 UTC, Matthias Klose
Details
tree dump (r136501 reverted) (27.93 KB, application/x-gzip)
2008-07-24 09:06 UTC, Matthias Klose
Details
tree dump (r136501) (27.87 KB, application/x-gzip)
2008-07-24 09:06 UTC, Matthias Klose
Details
diff of tree dump (453 bytes, text/x-diff)
2008-07-24 09:07 UTC, Matthias Klose
Details
preprocessed source (no pch used) (403.54 KB, application/x-gzip)
2008-07-24 09:31 UTC, Matthias Klose
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2008-07-24 09:04:24 UTC
OpenJDK (using the IcedTea6 build and patches) fails to build with 4.3 from the 4.3 branch, when the jvm built in stage1 is used for the first time in the stage2 build:

-def-pcompile:
    [javac] Compiling 2 source files to /scratch/packages/openjdk/x/openjdk-6-6b11/openjdk/control/build/linux-i586/langtools/build/toolclasses
WARNING: Default charset US-ASCII not supported, using ISO-8859-1 instead
    [javac] /scratch/packages/openjdk/x/openjdk-6-6b11/openjdk/langtools/make/tools/CompileProperties/CompileProperties.java:26: cannot access unnamed package
    [javac] ANSI_X3.4-1968
    [javac] import java.io.BufferedWriter;
    [javac] ^

BUILD FAILED

the build failure is not seen when reverting r136501; seen as well when just reverting the two hunks for record_numbers_of_iterations.

seen with -O3 and -O2, not -O1.

not seen on amd64 and sparc (the other two archs using OpenJDK hotspot).

the miscompiled file is ciTypeFlow.cpp, compiled using
g++-4.3 -fpic -fno-rtti -fno-exceptions -D_REENTRANT -fcheck-new -g -m32 -march=i586 -mtune=generic -O2 -fno-strict-aliasing -DVM_LITTLE_ENDIAN  -Wpointer-arith -Wconversion -Wsign-compare -c ciTypeFlow.cpp
Comment 1 Matthias Klose 2008-07-24 09:05:15 UTC
Created attachment 15948 [details]
preprocessed source
Comment 2 Matthias Klose 2008-07-24 09:06:12 UTC
Created attachment 15949 [details]
tree dump (r136501 reverted)
Comment 3 Matthias Klose 2008-07-24 09:06:38 UTC
Created attachment 15950 [details]
tree dump (r136501)
Comment 4 Matthias Klose 2008-07-24 09:07:07 UTC
Created attachment 15951 [details]
diff of tree dump
Comment 5 Matthias Klose 2008-07-24 09:15:30 UTC
-O3 and -fwrapv, r136501 not reverted works
Comment 6 Andrew Pinski 2008-07-24 09:27:25 UTC
if -fwrapv works then is really a bug?  Yes overflow is defined in java but c++ is not java. 
Comment 7 Matthias Klose 2008-07-24 09:31:21 UTC
Created attachment 15952 [details]
preprocessed source (no pch used)
Comment 8 Richard Biener 2008-07-24 09:31:51 UTC
Looking at the source I don't see any integer overflows - the fact that
-fno-ivopts makes it work and the effect is on a variable introduced by
ivopts hints at a GCC bug more than a application bug.

So - now I cannot put the state back to UNCONFIRMED. :P
Comment 9 Richard Biener 2008-07-24 09:52:13 UTC
The difference comes from the second VRP pass which concludes that c_76 is
[1, +INF] from which it changes

# c_173 = PHI <0(7), c_76(12)>

to

# c_173 = PHI <0(7), 1(12)>

which it concludes from

c_76 = (Cell) D.156138_4;

where Cell is

  enum Cell {
    Cell_0
  };

so the only valid values for c_76 are 0 and 1.  The problem is probably here
(c is of type Cell):

  D.156112_62 = c_173 * 4;

(D.156112 is unsigned int), where we obviously miss a cast in the IL.

This is from inlining ciTypeFlow::StateVector::type_at which looks like

ciType* ciTypeFlow::StateVector::type_at(ciTypeFlow::Cell) const (this, c)
{
  struct ciType * D.144073;
  struct ciType * * D.144074;
  unsigned int D.144075;
  struct ciType * * D.144076;

  D.144074 = this->_types;
  D.144075 = c * 4;
  D.144076 = D.144074 + D.144075;
  D.144073 = *D.144076;
  return D.144073;
}

gimplified from

return <retval> = *((struct ciType * *) ((const struct StateVector *) this)->_types + (unsigned int) ((unsigned int) c * 4));

which hints at that c probably has the same precision and signedness as
unsigned int.
Comment 10 Richard Biener 2008-07-25 12:14:05 UTC
I belive this is just INVALID.  The code seems to do lots of things with
this enum Cell, but the C++ compiler is allowed to just allocate 1 bit of
storage for it.

Maybe changing the Cell declaration to

enum Cell { Cell_0, Cell_max = UINT_MAX }

fixes the issue.

See 7.2/6 for the standard wording.
Comment 11 Matthias Klose 2008-07-25 13:54:18 UTC
the suggested change fixes the OpenJDK build.