RFR(M): Fix non-PCH build on Linux, Windows and MacOS X
volker.simonis at gmail.com
Tue Feb 26 00:19:24 PST 2013
On Mon, Feb 25, 2013 at 7:58 PM, Christian Thalinger
<christian.thalinger at oracle.com> wrote:
> On Feb 25, 2013, at 10:09 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
>> Recent changes have broken the no-PCH build again.
> Is this also broken for you?
Yes, that's exactly the problem (more precisely, a subset of the problem.)
The first issue reported by 8005084 has been fixed already by
subsequent changes (i.e. the inclusion of "asm/codeBuffer.hpp" into
The other issues in 8005084 (and some more) are fixed by my changes.
Should I resend the RFR with the bug id '8005084'?
Can you please review it?
Thank you and best regards,
> -- Chris
>> The fix requires some bigger changes this time but before going into
>> the details - PLEASE, PLEASE add a no-PCH build to your JPRT (or
>> nightly build or whatever) system to catch such errors. On platforms
>> which don't support PCH (i.e. Solaris) the normal build 'naturally'
>> works and I would consider it 'good practice' for a project to be able
>> to build without such an esoteric:) feature like precompiled headers.
>> Actually this only means that all the dependencies of a compilation
>> unit are explicitly recorded in that compilation unit (which I'd
>> consider self-evident).
>> Tested 'product' and 'jvmg' builds on Linux/x86_64, Linux/x86_32,
>> Windows/x86_64, Solaris/Sparc_64, Solaris/Sparc_32, and MacOS
>> Please be so kind to open a BugID for this issue and review the change:
>> PS: following some details of this change:
>> The following places were easy to fix (just include the appropriate
>> ".inline.hpp" files):
>> - uses Atomic inline functions (inc() and dec()) without including
>> - uses Atomic inline functions (load() and store()) without including
>> - uses Arguments::get_java_home() without including "runtime/arguments.hpp"
>> - use the Atomic inline functions (load() and store()) without
>> including "runtime/atomic.inline.hpp"
>> - include "orderAccess_<os>_<cpu>.inline.hpp" without using any
>> OrderAccess method. I removed this includes to avoid a cyclic
>> dependency between "orderAccess_<os>_<cpu>.inline.hpp" and
>> "atomic_<os>_<cpu>.inline.hpp" via "atomic.inline.hpp"
>> - use os::is_MP() without including os.hpp
>> The following fix was a little more cumbersome:
>> - The methods "set_suspended()" and "clear_suspended()" use
>> Atomic::cmpxchg without "atomic.inline.hpp" being included.
>> Unfortunately "os_bsd.hpp" and "os_linux.hpp" are not self-contained
>> files. They are themselves included into "os.hpp" right into the class
>> definition of the "os" class. It is therefore not possible to simply
>> include "atomic.inline.hpp" into "os_bsd.hpp" and "os_linux.hpp".
>> - The natural solution was to transfer the implementations of
>> "set_suspended()" and "clear_suspended()" to the corresponding
>> "os_bsd.inline.hpp" and "os_linux.inline.hpp" files. Unfortunately
>> this is not trivial as well, because both methods use preprocessor
>> defined constants from their enclosing class "SuspendResume" which are
>> defined right before there definition and undefined right thereafter.
>> They are therefore not available in "os_bsd.inline.hpp" and
>> - The solution for the last problem is to change the mentioned
>> preprocessor constants into an C++ enumeration which also results in a
>> nicer overall code. The drawback is that all the users of the
>> preprocessor constants have to be updated to use the new, qualified
>> enum-values (i.e. 'os::Bsd::SuspendResume::SR_CONTINUE' instead of
>> 'SR_CONTINUE'). It's a little more verbose but definitely a lot
>> cleaner this way.
>> I also did the following clean-up because I touched the corresponding
>> files anyway:
>> - remove "#pragma warning(disable: 4035)" because there don't seem to
>> be any missing return statement in this file anymore
>> - include both "atomic.hpp" and "atomic.inline.hpp". This is
>> unnecessary, because "XXX.inline.hpp" always includes "XXX.hpp"
>> Finally, I've also updated the copyright line to 2013 in every file
>> touched by this change.
More information about the hotspot-runtime-dev