[8u] Request for enhancement backport approval for 8210352: [hotspot] --with-extra-cxx-flags should not lower optimization

Fri Sep 7 07:44:43 UTC 2018

Hi Rob,

On Thu, 2018-09-06 at 18:55 +0100, Rob McKenna wrote:
> Meta: given the state of the codereview and the ongoing discussion, I'd
> like to see this fleshed out in the review thread before proceeding with
> push approval. While Erik doesn't have any objections, it would appear
> that others have concerns that should be addressed.

Thanks for the heads-up!

One clarification, though: This isn't asking for push approval, but
enhancement approval. As far as I understand the process this comes
*before* the actual webrev/code review as there will be a push approval
for 8u as a third step. Did I get this wrong?

Thanks,
Severin

>     -Rob
> 
> On 06/09/18 11:46, Severin Gehwolf wrote:
> > On Wed, 2018-09-05 at 20:06 +0100, Andrew Hughes wrote:
> > > On Wed, 5 Sep 2018 at 09:55, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > I've been asked to proceed with the enhancement request process[1] for
> > > > this proposed JDK 8u fix.
> > > > 
> > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210352
> > > > 
> > > > Currently JDK 8 and JDK 9+ behave differently in terms of hotspot
> > > > libjvm optimization flags. In JDK 8 they can be overridden by
> > > > --with-extra-c{,xx}-flags configure options. In JDK 9+ this cannot be
> > > > done as libjvm optimization flags come after any extra C flags from
> > > > configure. This enhancement request for JDK 8 asks for the build of the
> > > > JVM to behave the same as JDK 9+.
> > > > 
> > > > The rationale why we'd wanted such behaviour is as follows. In Fedora
> > > > distribution wide config exists which sets C flags. Hooks are in place
> > > > so that all natives get compiled with these C flags. One goal is to
> > > > ensure all distro libraries and programs are being optimized at at
> > > > least a certain threshold, say >= -O2. This includes OpenJDK.
> > > > 
> > > > While there is a way to fix the build system to compile each and every
> > > > library with some optimization level the risk is that there might not
> > > > be a one-size-fits-all downstream distribution consumers. The easier
> > > > and more flexible case is to allow for this via extra C flags which can
> > > > be passed to the build. This is currently possible.
> > > > 
> > > > The exception to this rule should be libjvm. Optimization levels for
> > > > the JVM have been thoroughly tested and to some extent been lowered via
> > > > a by-object overrides. Example case is
> > > > sharedRuntimeTrans.o and sharedRuntimeTrig.o which have -O0
> > > > settings[2]. The point is that for the JVM optimization flags have been
> > > > thoroughly crafted to a point where they are today. Letting the user
> > > > override this to some other value, like -O2, via a simple configure
> > > > option seems dangerous. Currently in JDK 8 it's possible to override
> > > > JVM optimization flags via --with-extra-c{,xx}-flags configure option.
> > > > 
> > > > OK to move ahead with this enhancement for JDK 8u?
> > > > 
> > > > Thanks,
> > > > Severin
> > > > 
> > > > [1] http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023064.html
> > > > [2] http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/80ee2541504e/make/linux/makefiles/amd64.make#l25
> > > > 
> > > 
> > > I tend to agree with David's comment:
> > > 
> > > "If I set an extra cflag I expect it to be applied to all compilations
> > > - cross-compilation would be
> > > broken if that were not the case. I don't see why -On should be
> > > treated any differently." [0]
> > 
> > This seems to be a comment on an old webrev. It does not apply to the
> > updated one here which adds the OPT flags *after* user CFLAGS for the
> > JVM:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210352/webrev.02/
> > 
> > As to "the flags should get applied to all compilations" issue: I'd
> > think that's not an issue for the updated webrev, is it? They get
> > applied, but OPT flags come before any JVM set flags.
> > 
> > > and we've encountered the same issue in Gentoo in the past [1].
> > > 
> > > Your example seems a little disingenuous, especially given you're
> > > proposing to remove
> > > those -O0 examples in another bug [2].
> > 
> > Fair point. I can come up with others. The hotspot build seems to be
> > sprinkled with optimization overrides:
> > 
> > http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/80ee2541504e/make/solaris/makefiles/sparc.make#l34
> > http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/80ee2541504e/make/linux/makefiles/gcc.make#l259
> > http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/80ee2541504e/make/linux/makefiles/fastdebug.make#l35
> > 
> > They could be summarized as work-arounds for GCC bugs. If user
> > supplied -O flags override all these, then they might get broken
> > builds. I question that's a good approach...
> > 
> > > Given that the code is
> > > generally compiled with -O3,
> > > the more common effect of overriding the optimisation is going to be
> > > to lower it.
> > 
> > Since the hotspot build already has slowdebug as a build option I'm not
> > sure. Do we really have data that somebody wants release builds with
> > -O2 (over -O3) with the risk of getting some mis-compiled objects in
> > the JVM?
> > 
> > My experience with dealing with JVM compilation issues and/or bugs in
> > newer GCCs was that slowdebug/release/fastdebug comparisons are usually
> > enough.
> > 
> > > The optimisations selected by these options are not static. They vary
> > > from compiler to
> > > compiler, and between versions of the same compiler. Just check the output of
> > > gcc -Q --help=optimizers. It's perfectly possible that -O3 produces a
> > > working build
> > > on one compiler, but enables optimisations that causes crashes on another. Thus,
> > > disabling the user's ability to tone down the optimisation seems wrong to me.
> > 
> > There are slowdebug builds for the JVM already. That turns off all
> > optimizations. I don't see how a global -O2 for the JVM which might
> > actually turn optimizations for some objects *up* will help.
> > 
> > > I
> > > wasn't aware that this had changed in 9 and, given it's barely a year
> > > old, it may
> > > still come as a surprise to others. I definitely don't think it should
> > > be changed in
> > > a four year old stable version.
> > 
> > It seems we disagree. It looks like the better choice to me. After all,
> > user defined global optimization for the JVM might produce wrong code.
> > Doesn't that make it too easy for somebody to shoot themselves in the
> > foot?
> > 
> > > The right solution here seems to me to be to add optimisations to
> > > those areas that
> > > lack it, in appropriate build modes. It seems likely to me that jsig
> > > and saproc were
> > > overlooked because of the use of separate makefiles in the old HotSpot
> > > build more
> > > than anything else (as seems to have been the case with execstack in 8187045).
> > 
> > Sure. It can be done. In that case we'd have to decide on some
> > appropriate level of optimization for those. Perhaps -O2?
> > 
> > > Are they optimised in 9? Or is there any evidence of breakage caused by
> > > using optimisation there?
> > 
> > They don't seem to be on 12:
> > 
> > $ grep -C3 SetupJdkLibrary make/lib/Lib-jdk.hotspot.agent.gmk
> > 
> > ################################################################################
> > 
> > $(eval $(call SetupJdkLibrary, BUILD_LIBSA, \
> >     NAME := saproc, \
> >     OPTIMIZATION := NONE, \
> >     DISABLED_WARNINGS_microsoft := 4267, \
> > 
> > I haven't checked on 9. I don't think there is evidence of breakage
> > caused by optimization as nobody likely uses optimization for those.
> > That doesn't mean it might not break, though :-)
> > 
> > Thanks,
> > Severin
> > 
> > > [0] http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023060.html
> > > [1] https://bugs.gentoo.org/356991
> > > [2] https://bugs.openjdk.java.net/browse/JDK-8210425