RFR (s) 8141570: Fix Zero interpreter build for --disable-precompiled-headers

[Coleen Phillimore]

Thanks for looking at this.  I had a lot of trouble getting an 
environment where I could get zero to build, and it these happened to be 
the things I needed to do to compile it.  I use 
--disable-precompiled-headers by default.

On 11/9/15 7:51 AM, Severin Gehwolf wrote:
> On Fri, 2015-11-06 at 11:47 -0500, Kim Barrett wrote:
>> On Nov 5, 2015, at 8:33 PM, Coleen Phillimore <coleen.phillimore at orac
>> le.com> wrote:
>>> With the RedHat compiler that I'm using g++ (GCC) 4.8.2 20140120
>>> (Red Hat 4.8.2-16)
>>> these additional includes are needed to compile without precompiled
>>> headers.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8141570.01/
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8141570
>>>
>>> I'm not sure why allocation.inline.hpp is needed unless the virtual
>>> destructor in src/share/vm/gc/shared/plab.hpp PLABStats implicitly
>>> calls delete.
>>>
>>> Tested with compiles of Zero plus plain linux
>>> debug/fastdebug/product.
>>>
>>> Thanks,
>>> Coleen
>> src/share/vm/gc/g1/g1EvacStats.hpp
>>    29 #include "memory/allocation.inline.hpp"
>>    30 #include "runtime/atomic.inline.hpp"
>>
>> xxx.inline.hpp files are not supposed to be included by xxx.hpp
>> files.
>>
>> The need for atomic.inline.hpp seems clear, and would seem to
>> constitute a bug in
>> g1EvacStats.hpp - there should be a g1EvacStats.inline.hpp file.

I filed bug

https://bugs.openjdk.java.net/browse/JDK-8142354
>>
>> I really don’t understand why allocation.inline.hpp is needed here at
>> all.
> FWIW, the below hunk seems to be all that's necessary in order to fix
> the --disable-precompiled-headers Zero build for me:
>
> diff --git a/src/share/vm/runtime/java.cpp b/src/share/vm/runtime/java.cpp
> --- a/src/share/vm/runtime/java.cpp
> +++ b/src/share/vm/runtime/java.cpp
> @@ -49,6 +49,7 @@
>   #include "runtime/arguments.hpp"
>   #include "runtime/biasedLocking.hpp"
>   #include "runtime/compilationPolicy.hpp"
> +#include "runtime/deoptimization.hpp"
>   #include "runtime/fprofiler.hpp"
>   #include "runtime/init.hpp"
>   #include "runtime/interfaceSupport.hpp"
>
> It does not seem the other change is required. Tested with fastdebug
> build of Zero with --disable-precompiled-headers that failed to build
> without and built fine with the fix.

I could check in just the deoptimization.hpp change and build only 
fastdebug until the other bug is fixed.

>
> Coleen, do you have details about the failure which gets supposedly
> fixed by the other hunk?

The only thing I have is the g++ compilation failures without these 
changes.  The one that was fixed by including allocation.inline.hpp was:

Building target 'images' in configuration 'linux-x86_64-normal-zero-release'
g1EvacStats.o: In function `G1EvacStats::~G1EvacStats()':
/home/cphillim/hg.local/jdk9.zero/hotspot/src/share/vm/gc/g1/g1EvacStats.hpp:32: 
undefined reference to `CHeapObj<(MemoryType)5>::operator delete(void*)'
collect2: error: ld returned 1 exit status

Since the destructor is complaining about the missing delete operator, I 
assume this compiler has the sort of destructor that implicitly can also 
call delete.  I don't have any other information or ideas really why 
this fails to compile.

Coleen


>
> Also the original build failure this patch is supposed to fix now got
> picked up by our builders too:
> http://builder.classpath.org/jenkins/job/OpenJDK9_hs_rt_Zero_no_pch/37
>
> Cheers,
> Severin