RFR(xs): 8078628: linux-zero does not build without precompiled header (original) (raw)

Severin Gehwolf sgehwolf at redhat.com
Wed Apr 29 07:35:22 UTC 2015

Hi Thomas,

On Mon, 2015-04-27 at 11:25 +0200, Thomas Stüfe wrote:

Hi David, Stefan,

thanks for reviewing! third webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8078628/webrev.02/webrev Ordering includes alphabetically turned up even more not-selfcontained includes. This is a really quite annoying work :-) I wish there were an official non-PCH nightly build. Not only would people notice missing headers, it also would make implementation creep into header files more obvious.

This looks good to me (not a Reviewer). All variants (release/fastdebug/slowdebug) were broken with PCH disabled. Fixed post-patch.

Thanks a lot for fixing it! I'll be sure to add a CI job to the Zero builders which have PCH disabled.

Cheers, Severin

Thanks, Thomas

On Mon, Apr 27, 2015 at 10:01 AM, Stefan Karlsson <_ _stefan.karlsson at oracle.com> wrote: > Hi Thomas, > > On 2015-04-27 02:40, David Holmes wrote: > >> Hi Thomas, >> >> On 25/04/2015 3:11 AM, Thomas Stüfe wrote: >> >>> Hi all, >>> >>> please review this tiny change. Build was broken for zero/slowdebug when >>> building without precompiled header. >>> >>> bug: https://bugs.openjdk.java.net/browse/JDK-8078628 >>> webrev: >>> http://cr.openjdk.java.net/~stuefe/webrevs/8078628/webrev.00/webrev/ >>> >> > The includes are kept sorted in other part of HotSpot, so I think it would > be good to have them sorted for the zero code as well: > > > http://cr.openjdk.java.net/~stuefe/webrevs/8078628/webrev.01/webrev/src/share/vm/interpreter/interpreterGenerator.hpp.udiff.html > > #include "interpreter/cppInterpreter.hpp" > +#include "interpreter/interpmasm.hpp" > #include "interpreter/cppInterpreterGenerator.hpp" > #include "interpreter/templateInterpreter.hpp" > #include "interpreter/templateInterpreterGenerator.hpp" > > > http://cr.openjdk.java.net/~stuefe/webrevs/8078628/webrev.01/webrev/src/cpu/zero/vm/nativeInstzero.cpp.udiff.html > > #include "precompiled.hpp" > #include "assemblerzero.inline.hpp" > +#include "interpreter/cppInterpreter.hpp" > #include "memory/resourceArea.hpp" > #include "nativeInstzero.hpp" > +#include "entryzero.hpp" > #include "oops/oop.inline.hpp" > #include "runtime/handles.hpp" > #include "runtime/sharedRuntime.hpp" > > Thanks, > StefanK > > > >> Why the double condition here: >> >> 33 #ifdef ZERO >> 34 #ifdef TARGETARCHzero >> 35 # include "entryzero.hpp" >> 36 #endif >> 37 #endif >> >> ?? >> >> Thanks, >> David >> >> >>> Thank you, >>> >>> Thomas >>> >>> >

More information about the hotspot-dev mailing list