[patch] hotspot miscompilation of OpenJDK6 with gcc from the gcc-4_3-branch

[Hiroshi Yamauchi]

Though it may not be a big deal, Martin and I have talked about
various options to fix the issue:

1. What Matthias suggested.
2. Pass -fno-tree-vrp to GCC which disables the value range
propagation optimization
3. Make Cell a real class and use it as a value type with almost no
runtime overhead
4. Replace the enum with "typedef int Cell" (but loses type safety)

Hiroshi

On Fri, Jul 25, 2008 at 10:10 AM, Martin Buchholz <martinrb at google.com> wrote:
> I've thought about how best to fix the enum Cell crash.
> Extending the range of the enum to
>  enum Cell { Cell_0, Cell_max = MAX_INT }
> works in practice.  But I was left wondering whether this was actually
> standards-correct.  Is it legal to use values for an enum that were
> not specified in the enum declaration (e.g. 1 for Cell above).
> A check of the draft C++ standard gives this wording:
>
> "An expression of arithmetic or enumeration type can be converted to
> an enumeration type explicitly. The value is
>  unchanged if it is in the range of enumeration values of the
> enumeration type; otherwise the resulting enumeration value
>  is unspecified."
>
> which makes it look like extending the range is Just Right.
>
> I might be tempted to create a proper "class Cell",
> but there is not a lot of state or behavior there to encapsulate.
>
> Martin
>
> On Fri, Jul 25, 2008 at 6:51 AM, Matthias Klose <doko at ubuntu.com> wrote:
>> 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
>>
>>
>> Upstream GCC [1] doesn't agree on a bug in the compiler, but in the application
>> code:
>>
>>  "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."
>>
>> The suggested fix is attached; I don't see any regressions. IcedTea currently
>> has a patch to work around the problem, compiling this file with -fno-ivopts.
>>
>>   Matthias
>>
>> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36917
>>
>> --- openjdk/hotspot/src/share/vm/ci/ciTypeFlow.hpp~     2008-07-10 22:04:30.000000000 +0200
>> +++ openjdk/hotspot/src/share/vm/ci/ciTypeFlow.hpp      2008-07-25 14:32:03.544802121 +0200
>> @@ -130,7 +130,7 @@
>>
>>   // Used as a combined index for locals and temps
>>   enum Cell {
>> -    Cell_0
>> +    Cell_0, Cell_max = UINT_MAX
>>   };
>>
>>   // A StateVector summarizes the type information at some
>>
>>
>