RFR(s): 8218041: Assorted wrong/missing includes

Stefan Karlsson stefan.karlsson at oracle.com
Wed Jan 30 12:05:39 UTC 2019


Hi Robbin,

Thanks for cleaning this up!

https://cr.openjdk.java.net/~rehn/8218041/v02/webrev/src/hotspot/share/runtime/handles.inline.hpp.udiff.html

An alternative would be to somehow call a non-inlined version of 
oopDesc::is_a, given that it's only used in an assert. That way we 
wouldn't have to include oop.inline.hpp in handles.inline.hpp.

https://cr.openjdk.java.net/~rehn/8218041/v02/webrev/src/hotspot/share/ci/ciMethod.cpp.udiff.html
https://cr.openjdk.java.net/~rehn/8218041/v02/webrev/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp.udiff.html

Incorrect sort order.

https://cr.openjdk.java.net/~rehn/8218041/v02/webrev/src/hotspot/share/jvmci/compilerRuntime.cpp.udiff.html

Preexisting incorrect sort order for deoptimization.hpp. Maybe fix it in 
this patch?

Thanks,
StefanK

On 2019-01-30 12:22, Robbin Ehn wrote:
> Sorry wrong url:
> http://cr.openjdk.java.net/~rehn/8218041/v02/
> http://cr.openjdk.java.net/~rehn/8218041/v02/inc/
>
> Thanks Aleksey for bringing to my attention!
>
> /Robbin
>
> On 2019-01-30 12:17, Robbin Ehn wrote:
>> Hi, here is v02.
>>
>> Add includes to handles.inline.hpp in all files we use a method in 
>> there.
>> Compiles on my 7 different configs including gcc 7.3/8.2, clang 7, no 
>> pre-compiled headers. And tier-1 which includes our std builds.
>>
>> http://rehn-ws.se.oracle.com/cr_mirror/8218041/v02/inc
>> http://rehn-ws.se.oracle.com/cr_mirror/8218041/v02/
>>
>> Also this seems to signification reduce gcc inline warnings about 
>> local comdat symbol for Handle(Thread*, oop).
>>
>> Thanks, Robbin
>>
>> On 2019-01-30 09:21, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Code:
>>> http://cr.openjdk.java.net/~rehn/8218041/webrev/
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8218041
>>>
>>> After fixing these includes, there was a circular dependency via 
>>> shenandoah
>>> code. I moved try_cancel_gc to cpp where the only use was. So it 
>>> never should
>>> had been in the inline header in the first place.
>>>
>>> I listed why the include is needed below.
>>>
>>> Tier 1 and no pre-compiled.
>>>
>>> FYI: I was investigating why Handle::Handle(Thread*,oop) was not 
>>> inlined.
>>> gcc complains there being a local comdat symbol, forcing it to be 
>>> inlined or
>>> using clang there is no issue. So it looks like a gcc bug both in 
>>> 7.3 and 8.2.
>>>
>>> Thanks, Robbin
>>>
>>> src/hotspot/share/aot/aotLoader.cpp
>>> runtime/os.inline.hpp      for os::dll_unload
>>>
>>> src/hotspot/share/c1/c1_Runtime1.cpp
>>> runtime/handles.inline.hpp for Handle(Thread*, oop)
>>>
>>> src/hotspot/share/gc/z/zFuture.inline.hpp
>>> runtime/interfaceSupport.inline.hpp not used.
>>>
>>> src/hotspot/share/prims/nativeLookup.cpp
>>> runtime/os.inline.hpp      for os::dll_unload
>>>
>>> src/hotspot/share/runtime/handles.hpp
>>> Forward declaration            Thread
>>>
>>> src/hotspot/share/runtime/handles.inline.hpp
>>> runtime/thread.hpp       for Thread::current
>>> oops/oop.inline.hpp        for oopDesc::is_a
>>> oops/metadata.hpp          for is_valid
>>>
>>> src/hotspot/share/runtime/semaphore.inline.hpp
>>> runtime/thread.hpp         for osthread
>>>
>>> src/hotspot/share/runtime/vframe.cpp
>>> runtime/thread.inline.hpp  for JavaThread::class_to_be_initialized



More information about the hotspot-dev mailing list