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

Robbin Ehn robbin.ehn at oracle.com
Wed Jan 30 12:41:04 UTC 2019


Hi Stefan,

On 2019-01-30 13:05, Stefan Karlsson wrote:
> 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.

My bad, we actually never use that method since it's a macro argument, and macro 
only generates the special ones: is_instance_noinline, is_array_noinline, 
is_objArray_noinline, is_typeArray_noinline.

So I just changed to include oop.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.

Fixed.

> 
> 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?

Fixed.

Compile testing, sending the v3 to rfr mail if all passes.

Thanks!

/Robbin

> 
> 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