RFR: 8072911: Remove includes of oop.inline.hpp from .hpp files
Volker Simonis
volker.simonis at gmail.com
Wed Feb 11 18:34:38 UTC 2015
I found the problem. Can you please include the following additional
line in interp_masm_ppc_64.cpp:
diff -r e9b3fedb3bca src/cpu/ppc/vm/interp_masm_ppc_64.cpp
--- a/src/cpu/ppc/vm/interp_masm_ppc_64.cpp Wed Feb 11 13:20:41 2015 +0100
+++ b/src/cpu/ppc/vm/interp_masm_ppc_64.cpp Wed Feb 11 19:00:23 2015 +0100
@@ -29,6 +29,7 @@
#include "interp_masm_ppc_64.hpp"
#include "interpreter/interpreterRuntime.hpp"
#include "prims/jvmtiThreadState.hpp"
+#include "runtime/sharedRuntime.hpp"
#ifdef PRODUCT
#define BLOCK_COMMENT(str) // nothing
It seems this was somehow implicitly included before your change.
Thanks,
Volker
On Wed, Feb 11, 2015 at 6:56 PM, Volker Simonis
<volker.simonis at gmail.com> wrote:
> Hi Stefan,
>
> thanks for doing this change and thanks for changing the ppc-files as well.
>
> Unfortunately there still seems to be an error if building on
> Linux/ppc64 without precompiled headers.
> Can you please wait a little until I'll send you the additional
> changes which are required before pushing this one.
>
> Thanks,
> Volker
>
> PS: I've just checked that AIX/ppc64 builds without a problem.
>
> On Wed, Feb 11, 2015 at 5:57 PM, Jesper Wilhelmsson
> <jesper.wilhelmsson at oracle.com> wrote:
>> Looks good!
>> /Jesper
>>
>> Stefan Karlsson skrev den 11/2/15 16:55:
>>
>>> Hi Bengt,
>>>
>>> On 2015-02-11 16:51, Bengt Rutisson wrote:
>>>>
>>>>
>>>> Hi Stefan,
>>>>
>>>> Pretty difficult to review this type of change, but I think it looks
>>>> good. I
>>>> verified that no hpp files inlcude oop.inline.hpp after your change. Nice
>>>> cleanup!
>>>>
>>>> One minor thing (not your code):
>>>>
>>>> In src/share/vm/oops/oop.hpp there is this comment:
>>>>
>>>> // type test operations (inlined in oop.inline.h)
>>>>
>>>> I think the comment should say oop.inline.hpp not just .h. Do you mind
>>>> fixing
>>>> this as part of this change too?
>>>
>>>
>>> Thanks for reviewing. I'll fix the comment.
>>>
>>> Thanks,
>>> StefanK
>>>
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>>
>>>>
>>>> On 11/02/15 13:38, Stefan Karlsson wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Please, review this patch to fix our includes of oop.inline.hpp.
>>>>>
>>>>> http://cr.openjdk.java.net/~stefank/8072911/webrev.01
>>>>> https://bugs.openjdk.java.net/browse/JDK-8072911
>>>>>
>>>>>
>>>>> From the enhancement description:
>>>>> ---
>>>>> Our inline.hpp files should preferably only be included from .cpp files
>>>>> or
>>>>> other .inline.hpp files. When they are included in .hpp files, the
>>>>> include
>>>>> dependency of the implementation spreads to unrelated parts of the code
>>>>> base,
>>>>> and it's easy to get circular dependencies.
>>>>>
>>>>> This guide line is documented on:
>>>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#Files
>>>>>
>>>>> oop.inline.hpp is one of the more problematic files, and I propose a
>>>>> patch to
>>>>> get rid of all inclusions of oop.inline.hpp from other .hpp files.
>>>>> ---
>>>>>
>>>>>
>>>>> Summary of the code changes:
>>>>>
>>>>> oop.inline.hpp
>>>>> - Added to needed .cpp and .inline.hpp files.
>>>>>
>>>>> oop.inline2.hpp
>>>>> - This file isn't needed anymore and the contents have been moved to
>>>>> oop.inline.hpp.
>>>>>
>>>>> oop.hpp
>>>>> - Inlined klass_gap_offset_in_bytes in the hpp file, since it's
>>>>> currently
>>>>> used from a few hpp files.
>>>>>
>>>>> - Create non-inlined versions of is_instance checks, to be used in
>>>>> asserts
>>>>> in handles.hpp.
>>>>>
>>>>> - Added has_klass_gap to get rid of dependency against globals.hpp
>>>>>
>>>>> javaClasses.hpp
>>>>> - There were a couple of usages of klass.hpp that were moved out to
>>>>> javaClasses.cpp and a few that were moved to javaClasses.inline.hpp when
>>>>> it
>>>>> wasn't obvious if we needed to inline the functions or not.
>>>>>
>>>>> vmGCOperations.hpp
>>>>> - Moved the VM_GC_Operation destructor to the cpp file.
>>>>>
>>>>> collectedHeap.hpp
>>>>> - Moved print_on_error
>>>>>
>>>>> barrierSet.hpp
>>>>> - Moved the inline keyword from the hpp file to the inline.hpp file.
>>>>>
>>>>> cardTableModRefBS.hpp
>>>>> - Moved inline_write_ref_field to the inline.hpp file.
>>>>>
>>>>> objArrayOop.hpp
>>>>> - Moved obj_at to the inline.hpp file.
>>>>>
>>>>> graphKit.hpp
>>>>> - Moved byte_map_base_node to .cpp file, where it's used.
>>>>>
>>>>> <other added includes>
>>>>> - Added missing includes that were removed when oop.inline.hpp was
>>>>> removed
>>>>> from the hpp files.
>>>>>
>>>>>
>>>>> The patch has been tested with JPRT.
>>>>>
>>>>> Thanks,
>>>>> StefanK
>>>>>
>>>>
>>>
>>
More information about the hotspot-dev
mailing list