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

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 *R*eviewer). 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/interp_masm.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/nativeInst_zero.cpp.udiff.html
> >
> >  #include "precompiled.hpp"
> >  #include "assembler_zero.inline.hpp"
> > +#include "interpreter/cppInterpreter.hpp"
> >  #include "memory/resourceArea.hpp"
> >  #include "nativeInst_zero.hpp"
> > +#include "entry_zero.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 TARGET_ARCH_zero
> >>   35 # include "entry_zero.hpp"
> >>   36 #endif
> >>   37 #endif
> >>
> >> ??
> >>
> >> Thanks,
> >> David
> >>
> >>
> >>> Thank you,
> >>>
> >>> Thomas
> >>>
> >>>
> >





More information about the hotspot-dev mailing list