Re: [8u] RFR: 8227397: Add --with-extra-asflags configure option
On 27/09/2019 16:48, Severin Gehwolf wrote: > Hi, > > Could I please get a review of this 8u build change backport which adds > --with-extra-asflags to OpenJDK 8u. At Red Hat, we need to pass certain > assembler only flags for some builds. For example "-Wa,--generate- > missing-build-notes=yes", to assembly files only. As the build system > is different in 8u over 11u I've re-done the patch. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/jdk8/01/ > > Testing: Built with --with-extra-asflags=-Wa,--generate-missing-build- > notes=yes on Linux x86_64, confirmed linux_x86_64.s gets assembled with > the flag and only that file. > > I've omitted the windows portion of passing as flags to the hotspot > build as I've no idea how. Contributions welcome if that should get > included. > > Thoughts? > > Thanks, > Severin > Looks ok to me. As to the Windows side, it seems there are no ASFLAGS rules at all nor any assembly files: $ grep -r 'ASFLAGS' hotspot/make/ hotspot/make/aix/makefiles/xlc.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/aix/makefiles/rules.make:AS.S= $(AS) $(ASFLAGS) hotspot/make/solaris/makefiles/sparcWorks.make:ASFLAGS += $(AS_ARCHFLAG) hotspot/make/solaris/makefiles/sparcv9.make:ASFLAGS += $(AS_ARCHFLAG) hotspot/make/solaris/makefiles/sparc.make:ASFLAGS += $(AS_ARCHFLAG) hotspot/make/solaris/makefiles/gcc.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/solaris/makefiles/gcc.make:ASFLAGS += -march=k8 -march=amd64 hotspot/make/solaris/makefiles/rules.make:AS.S= $(AS) $(ASFLAGS) hotspot/make/bsd/makefiles/sparcWorks.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/bsd/makefiles/gcc.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/bsd/makefiles/gcc.make:ASFLAGS += -x assembler-with-cpp hotspot/make/bsd/makefiles/rules.make:AS.S= $(AS) $(ASFLAGS) hotspot/make/linux/makefiles/sparcWorks.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/linux/makefiles/gcc.make:ASFLAGS+= $(ARCHFLAG) hotspot/make/linux/makefiles/gcc.make: ASFLAGS+= $(ASFLAGS_DEBUG_SYMBOLS) hotspot/make/linux/makefiles/rules.make:AS.S= $(AS) $(ASFLAGS) $ find hotspot/src/ -name '*.s' hotspot/src/os_cpu/bsd_x86/vm/bsd_x86_64.s hotspot/src/os_cpu/bsd_x86/vm/bsd_x86_32.s hotspot/src/os_cpu/solaris_sparc/vm/solaris_sparc.s hotspot/src/os_cpu/solaris_x86/vm/solaris_x86_32.s hotspot/src/os_cpu/solaris_x86/vm/solaris_x86_64.s hotspot/src/os_cpu/linux_x86/vm/linux_x86_64.s hotspot/src/os_cpu/linux_x86/vm/linux_x86_32.s hotspot/src/os_cpu/linux_sparc/vm/linux_sparc.s so it looks like assembly support would need to be there first before making this change on top. Thanks, -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222 https://keybase.io/gnu_andrew
Re: [8u] RFR: 8227397: Add --with-extra-asflags configure option
LGTM On Fri, Jan 17, 2020 at 2:59 AM Severin Gehwolf wrote: > Hi, > > Could I get a second review from an JDK 8u Reviewer, please? > > Thanks, > Severin > > On Mon, 2019-09-30 at 11:36 +0200, Magnus Ihse Bursie wrote: > > On 2019-09-27 17:48, Severin Gehwolf wrote: > > > Hi, > > > > > > Could I please get a review of this 8u build change backport which > > > adds > > > --with-extra-asflags to OpenJDK 8u. At Red Hat, we need to pass > > > certain > > > assembler only flags for some builds. For example "-Wa,--generate- > > > missing-build-notes=yes", to assembly files only. As the build > > > system > > > is different in 8u over 11u I've re-done the patch. > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > > > webrev: > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/jdk8/01/ > > Looks good to me. > > > > /Magnus > > > Testing: Built with --with-extra-asflags=-Wa,--generate-missing- > > > build- > > > notes=yes on Linux x86_64, confirmed linux_x86_64.s gets assembled > > > with > > > the flag and only that file. > > > > > > I've omitted the windows portion of passing as flags to the hotspot > > > build as I've no idea how. Contributions welcome if that should get > > > included. > > > > > > Thoughts? > > > > > > Thanks, > > > Severin > > > > > > >
Re: [8u] RFR: 8227397: Add --with-extra-asflags configure option
Hi, Could I get a second review from an JDK 8u Reviewer, please? Thanks, Severin On Mon, 2019-09-30 at 11:36 +0200, Magnus Ihse Bursie wrote: > On 2019-09-27 17:48, Severin Gehwolf wrote: > > Hi, > > > > Could I please get a review of this 8u build change backport which > > adds > > --with-extra-asflags to OpenJDK 8u. At Red Hat, we need to pass > > certain > > assembler only flags for some builds. For example "-Wa,--generate- > > missing-build-notes=yes", to assembly files only. As the build > > system > > is different in 8u over 11u I've re-done the patch. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > > webrev: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/jdk8/01/ > Looks good to me. > > /Magnus > > Testing: Built with --with-extra-asflags=-Wa,--generate-missing- > > build- > > notes=yes on Linux x86_64, confirmed linux_x86_64.s gets assembled > > with > > the flag and only that file. > > > > I've omitted the windows portion of passing as flags to the hotspot > > build as I've no idea how. Contributions welcome if that should get > > included. > > > > Thoughts? > > > > Thanks, > > Severin > > >
Re: [8u] RFR: 8227397: Add --with-extra-asflags configure option
On Mon, 2019-09-30 at 11:36 +0200, Magnus Ihse Bursie wrote: > On 2019-09-27 17:48, Severin Gehwolf wrote: > > Hi, > > > > Could I please get a review of this 8u build change backport which adds > > --with-extra-asflags to OpenJDK 8u. At Red Hat, we need to pass certain > > assembler only flags for some builds. For example "-Wa,--generate- > > missing-build-notes=yes", to assembly files only. As the build system > > is different in 8u over 11u I've re-done the patch. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/jdk8/01/ > Looks good to me. > > /Magnus Thanks for the review, Magnus! Cheers, Severin > > Testing: Built with --with-extra-asflags=-Wa,--generate-missing-build- > > notes=yes on Linux x86_64, confirmed linux_x86_64.s gets assembled with > > the flag and only that file. > > > > I've omitted the windows portion of passing as flags to the hotspot > > build as I've no idea how. Contributions welcome if that should get > > included. > > > > Thoughts? > > > > Thanks, > > Severin > > >
[8u] RFR: 8227397: Add --with-extra-asflags configure option
Hi, Could I please get a review of this 8u build change backport which adds --with-extra-asflags to OpenJDK 8u. At Red Hat, we need to pass certain assembler only flags for some builds. For example "-Wa,--generate- missing-build-notes=yes", to assembly files only. As the build system is different in 8u over 11u I've re-done the patch. Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/jdk8/01/ Testing: Built with --with-extra-asflags=-Wa,--generate-missing-build- notes=yes on Linux x86_64, confirmed linux_x86_64.s gets assembled with the flag and only that file. I've omitted the windows portion of passing as flags to the hotspot build as I've no idea how. Contributions welcome if that should get included. Thoughts? Thanks, Severin
Re: [Ping?] RFR: 8227397: Add --with-extra-asflags configure option
Looks good.
Paul
On 7/16/19, 2:17 AM, "Severin Gehwolf" wrote:
Hi Paul,
Thanks for the review!
On Mon, 2019-07-15 at 18:02 +, Hohensee, Paul wrote:
> In CompileJvm.gmk, I'd replace
>
> -ASFLAGS := $(JVM_ASFLAGS), \
> +ASFLAGS := $(JVM_ASFLAGS) $(EXTRA_ASFLAGS), \
>
> with adding EXTRA_ASFLAGS to JVM_ASFLAGS right after where JVM_LDFLAGS is
finalized, vis
>
> JVM_LDFLAGS += \
> $(SHARED_LIBRARY_FLAGS) \
> $(JVM_LDFLAGS_FEATURES) \
> $(EXTRA_LDFLAGS) \
> #
> +
> + JVM_ASFLAGS += ${EXTRA_ASFLAGS)
>
> That way, uses of EXTRA_xxFLAGS are all in one place and possible future
file-specific assembler flags don't have to duplicate adding EXTRA_ASFLAGS.
Fixed as suggested:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/02/webrev/
Thanks,
Severin
> Otherwise, good.
>
> Paul
>
> On 7/15/19, 7:27 AM, "build-dev on behalf of Severin Gehwolf"
wrote:
>
> Anyone?
>
> On Mon, 2019-07-08 at 17:56 +0200, Severin Gehwolf wrote:
> > Hi,
> >
> > Could I please get a review for this patch which adds a new
configure
> > option --with-extra-asflags? The issue at hand is that we, Red Hat,
> > need to pass certain extra flags to the assembler when OpenJDK is
being
> > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's
> > currently not possible and extra C/C++ flags would need to be used,
> > which seems not nice.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397
> > webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/
> >
> > After this patch extra assembler flags get added to *.s/.S files for
> > libjvm.so:
> >
> > $ grep -n generate-missing-build-notes=yes
build/linux-x86_64-server-release/build.log
> > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes
> > 15005:( /usr/bin/rm -f
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
&& /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o
/disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> >(/usr/bin/tee -a
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log)
2> >(/usr/bin/tee -a
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
>&2) || ( exitcode=$? && /usr/bin/cp
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log
&& /usr/bin/cp
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline
&& exit $exitcode ) )
> >
> > I'll run this through jdk/submit before I push.
> >
> > Thoughts?
> >
> > Thanks,
> > Severin
>
>
>
Re: [Ping?] RFR: 8227397: Add --with-extra-asflags configure option
Hi Paul,
Thanks for the review!
On Mon, 2019-07-15 at 18:02 +, Hohensee, Paul wrote:
> In CompileJvm.gmk, I'd replace
>
> -ASFLAGS := $(JVM_ASFLAGS), \
> +ASFLAGS := $(JVM_ASFLAGS) $(EXTRA_ASFLAGS), \
>
> with adding EXTRA_ASFLAGS to JVM_ASFLAGS right after where JVM_LDFLAGS is
> finalized, vis
>
> JVM_LDFLAGS += \
> $(SHARED_LIBRARY_FLAGS) \
> $(JVM_LDFLAGS_FEATURES) \
> $(EXTRA_LDFLAGS) \
> #
> +
> + JVM_ASFLAGS += ${EXTRA_ASFLAGS)
>
> That way, uses of EXTRA_xxFLAGS are all in one place and possible future
> file-specific assembler flags don't have to duplicate adding EXTRA_ASFLAGS.
Fixed as suggested:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/02/webrev/
Thanks,
Severin
> Otherwise, good.
>
> Paul
>
> On 7/15/19, 7:27 AM, "build-dev on behalf of Severin Gehwolf"
> wrote:
>
> Anyone?
>
> On Mon, 2019-07-08 at 17:56 +0200, Severin Gehwolf wrote:
> > Hi,
> >
> > Could I please get a review for this patch which adds a new configure
> > option --with-extra-asflags? The issue at hand is that we, Red Hat,
> > need to pass certain extra flags to the assembler when OpenJDK is being
> > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's
> > currently not possible and extra C/C++ flags would need to be used,
> > which seems not nice.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397
> > webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/
> >
> > After this patch extra assembler flags get added to *.s/.S files for
> > libjvm.so:
> >
> > $ grep -n generate-missing-build-notes=yes
> build/linux-x86_64-server-release/build.log
> > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes
> > 15005:( /usr/bin/rm -f
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
> && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o
>
> /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> > >(/usr/bin/tee -a
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log)
> 2> >(/usr/bin/tee -a
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
> >&2) || ( exitcode=$? && /usr/bin/cp
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
>
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log
> && /usr/bin/cp
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline
>
> /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline
> && exit $exitcode ) )
> >
> > I'll run this through jdk/submit before I push.
> >
> > Thoughts?
> >
> > Thanks,
> > Severin
>
>
>
Re: [Ping?] RFR: 8227397: Add --with-extra-asflags configure option
In CompileJvm.gmk, I'd replace
-ASFLAGS := $(JVM_ASFLAGS), \
+ASFLAGS := $(JVM_ASFLAGS) $(EXTRA_ASFLAGS), \
with adding EXTRA_ASFLAGS to JVM_ASFLAGS right after where JVM_LDFLAGS is
finalized, vis
JVM_LDFLAGS += \
$(SHARED_LIBRARY_FLAGS) \
$(JVM_LDFLAGS_FEATURES) \
$(EXTRA_LDFLAGS) \
#
+
+ JVM_ASFLAGS += ${EXTRA_ASFLAGS)
That way, uses of EXTRA_xxFLAGS are all in one place and possible future
file-specific assembler flags don't have to duplicate adding EXTRA_ASFLAGS.
Otherwise, good.
Paul
On 7/15/19, 7:27 AM, "build-dev on behalf of Severin Gehwolf"
wrote:
Anyone?
On Mon, 2019-07-08 at 17:56 +0200, Severin Gehwolf wrote:
> Hi,
>
> Could I please get a review for this patch which adds a new configure
> option --with-extra-asflags? The issue at hand is that we, Red Hat,
> need to pass certain extra flags to the assembler when OpenJDK is being
> compiled. -Wa,--generate-missing-build-notes=yes in our case. That's
> currently not possible and extra C/C++ flags would need to be used,
> which seems not nice.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8227397
> webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/
>
> After this patch extra assembler flags get added to *.s/.S files for
> libjvm.so:
>
> $ grep -n generate-missing-build-notes=yes
build/linux-x86_64-server-release/build.log
> 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes
> 15005:( /usr/bin/rm -f
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
&& /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o
/disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s
> >(/usr/bin/tee -a
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log)
2> >(/usr/bin/tee -a
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
>&2) || ( exitcode=$? && /usr/bin/cp
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log
&& /usr/bin/cp
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline
/disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline
&& exit $exitcode ) )
>
> I'll run this through jdk/submit before I push.
>
> Thoughts?
>
> Thanks,
> Severin
Re: [Ping?] RFR: 8227397: Add --with-extra-asflags configure option
Anyone? On Mon, 2019-07-08 at 17:56 +0200, Severin Gehwolf wrote: > Hi, > > Could I please get a review for this patch which adds a new configure > option --with-extra-asflags? The issue at hand is that we, Red Hat, > need to pass certain extra flags to the assembler when OpenJDK is being > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's > currently not possible and extra C/C++ flags would need to be used, > which seems not nice. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/ > > After this patch extra assembler flags get added to *.s/.S files for > libjvm.so: > > $ grep -n generate-missing-build-notes=yes > build/linux-x86_64-server-release/build.log > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes > 15005:( /usr/bin/rm -f > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o > > /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s > > >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log) > 2> >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > >&2) || ( exitcode=$? && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log > && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline > && exit $exitcode ) ) > > I'll run this through jdk/submit before I push. > > Thoughts? > > Thanks, > Severin
Re: RFR: 8227397: Add --with-extra-asflags configure option
* Severin Gehwolf: > Hi Martin, > > On Mon, 2019-07-08 at 10:42 -0700, Martin Buchholz wrote: >> (not really a review ...) >> >> I'm confused because the assembler is invoked when compiling any sort of >> source file - C, C++, or .s/.S. >> Wouldn't we want assembler flags passed uniformly, no matter when the >> assembler is invoked? > > Good point. I guess that's debatable. The intention is to only affect > assembling of, well, assembly files (.s/.S). Right now, --with-extra- > cflags would work, but that's more an accident I'd think: Using -Wa,--generate-missing-build-notes=yes for compiling C/C++ files (as opposed to assembling hand-written assembler input) is problematic because it more or less invalidates why we have these notes. They are supposed to come from the compiler plugin for C/C++ files. If the assembler generates them, we won't see in the resulting binary that we actually have a coverage gap for C/C++ sources because the compiler plugin wasn't active for some reason. Thanks, Florian
Re: RFR: 8227397: Add --with-extra-asflags configure option
To my surprise I checked the default meaning of ASFLAGS in make and found 'ASFLAGS' Extra flags to give to the assembler (when explicitly invoked on a '.s' or '.S' file). $ make -p |& grep -B1 AS.*FLAG # default LINK.S = $(CC) $(ASFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_MACH) -- # default COMPILE.S = $(CC) $(ASFLAGS) $(CPPFLAGS) $(TARGET_MACH) -c -- # default COMPILE.s = $(AS) $(ASFLAGS) $(TARGET_MACH) -- # default LINK.s = $(CC) $(ASFLAGS) $(LDFLAGS) $(TARGET_MACH) ... which agrees with your patch ! On Mon, Jul 8, 2019 at 11:14 AM Severin Gehwolf wrote: > Hi Martin, > > On Mon, 2019-07-08 at 10:42 -0700, Martin Buchholz wrote: > > (not really a review ...) > > > > I'm confused because the assembler is invoked when compiling any sort of > source file - C, C++, or .s/.S. > > Wouldn't we want assembler flags passed uniformly, no matter when the > assembler is invoked? > > Good point. I guess that's debatable. The intention is to only affect > assembling of, well, assembly files (.s/.S). Right now, --with-extra- > cflags would work, but that's more an accident I'd think: > > > http://hg.openjdk.java.net/jdk/jdk/file/377e49b3014c/make/autoconf/flags-other.m4#l119 > > In JDK 8 this is a real issue. See: > https://bugs.openjdk.java.net/browse/JDK-8219772 > > During review this suggestion (with-extra-asflags) came up: > http://mail.openjdk.java.net/pipermail/jdk8u-dev/2019-March/008861.html > > > It looks like this patch only affects compilation of .s/.S files in > hotspot? > > Yes. I've only found assemly files in hotspot. Happy to add it for core > libs, too, but not sure where. > > Thanks, > Severin > > > On Mon, Jul 8, 2019 at 8:57 AM Severin Gehwolf > wrote: > > > Hi, > > > > > > Could I please get a review for this patch which adds a new configure > > > option --with-extra-asflags? The issue at hand is that we, Red Hat, > > > need to pass certain extra flags to the assembler when OpenJDK is being > > > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's > > > currently not possible and extra C/C++ flags would need to be used, > > > which seems not nice. > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > > > webrev: > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/ > > > > > > After this patch extra assembler flags get added to *.s/.S files for > > > libjvm.so: > > > > > > $ grep -n generate-missing-build-notes=yes > build/linux-x86_64-server-release/build.log > > > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes > > > 15005:( /usr/bin/rm -f > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o > /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s > > >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log) > 2> >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > >&2) || ( exitcode=$? && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log > && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline > && exit $exitcode ) ) > > > > > > I'll run this through jdk/submit before I push. > > > > > > Thoughts? > > > > > > Thanks, > > > Severin > > > > >
Re: RFR: 8227397: Add --with-extra-asflags configure option
Hi Martin, On Mon, 2019-07-08 at 10:42 -0700, Martin Buchholz wrote: > (not really a review ...) > > I'm confused because the assembler is invoked when compiling any sort of > source file - C, C++, or .s/.S. > Wouldn't we want assembler flags passed uniformly, no matter when the > assembler is invoked? Good point. I guess that's debatable. The intention is to only affect assembling of, well, assembly files (.s/.S). Right now, --with-extra- cflags would work, but that's more an accident I'd think: http://hg.openjdk.java.net/jdk/jdk/file/377e49b3014c/make/autoconf/flags-other.m4#l119 In JDK 8 this is a real issue. See: https://bugs.openjdk.java.net/browse/JDK-8219772 During review this suggestion (with-extra-asflags) came up: http://mail.openjdk.java.net/pipermail/jdk8u-dev/2019-March/008861.html > It looks like this patch only affects compilation of .s/.S files in hotspot? Yes. I've only found assemly files in hotspot. Happy to add it for core libs, too, but not sure where. Thanks, Severin > On Mon, Jul 8, 2019 at 8:57 AM Severin Gehwolf wrote: > > Hi, > > > > Could I please get a review for this patch which adds a new configure > > option --with-extra-asflags? The issue at hand is that we, Red Hat, > > need to pass certain extra flags to the assembler when OpenJDK is being > > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's > > currently not possible and extra C/C++ flags would need to be used, > > which seems not nice. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/ > > > > After this patch extra assembler flags get added to *.s/.S files for > > libjvm.so: > > > > $ grep -n generate-missing-build-notes=yes > > build/linux-x86_64-server-release/build.log > > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes > > 15005:( /usr/bin/rm -f > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > > && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o > > > > /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s > > > >(/usr/bin/tee -a > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log) > > 2> >(/usr/bin/tee -a > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > > >&2) || ( exitcode=$? && /usr/bin/cp > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > > > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log > > && /usr/bin/cp > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline > > > > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline > > && exit $exitcode ) ) > > > > I'll run this through jdk/submit before I push. > > > > Thoughts? > > > > Thanks, > > Severin > >
Re: RFR: 8227397: Add --with-extra-asflags configure option
(not really a review ...) I'm confused because the assembler is invoked when compiling any sort of source file - C, C++, or .s/.S. Wouldn't we want assembler flags passed uniformly, no matter when the assembler is invoked? It looks like this patch only affects compilation of .s/.S files in hotspot? On Mon, Jul 8, 2019 at 8:57 AM Severin Gehwolf wrote: > Hi, > > Could I please get a review for this patch which adds a new configure > option --with-extra-asflags? The issue at hand is that we, Red Hat, > need to pass certain extra flags to the assembler when OpenJDK is being > compiled. -Wa,--generate-missing-build-notes=yes in our case. That's > currently not possible and extra C/C++ flags would need to be used, > which seems not nice. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 > webrev: > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/ > > After this patch extra assembler flags get added to *.s/.S files for > libjvm.so: > > $ grep -n generate-missing-build-notes=yes > build/linux-x86_64-server-release/build.log > 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes > 15005:( /usr/bin/rm -f > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o > /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s > > >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log) > 2> >(/usr/bin/tee -a > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > >&2) || ( exitcode=$? && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log > && /usr/bin/cp > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline > /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline > && exit $exitcode ) ) > > I'll run this through jdk/submit before I push. > > Thoughts? > > Thanks, > Severin > >
RFR: 8227397: Add --with-extra-asflags configure option
Hi, Could I please get a review for this patch which adds a new configure option --with-extra-asflags? The issue at hand is that we, Red Hat, need to pass certain extra flags to the assembler when OpenJDK is being compiled. -Wa,--generate-missing-build-notes=yes in our case. That's currently not possible and extra C/C++ flags would need to be used, which seems not nice. Bug: https://bugs.openjdk.java.net/browse/JDK-8227397 webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227397/01/webrev/ After this patch extra assembler flags get added to *.s/.S files for libjvm.so: $ grep -n generate-missing-build-notes=yes build/linux-x86_64-server-release/build.log 1049: [18] ASFLAGS := -m64 -Wa,--generate-missing-build-notes=yes 15005:( /usr/bin/rm -f /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log && /usr/bin/gcc -c -m64 -Wa,--generate-missing-build-notes=yes -g -o /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o /disk/openjdk/upstream-sources/openjdk-head/src/hotspot/os_cpu/linux_x86/linux_x86_64.s > >(/usr/bin/tee -a /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log) 2> >(/usr/bin/tee -a /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log >&2) || ( exitcode=$? && /usr/bin/cp /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.log /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.log && /usr/bin/cp /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/hotspot/variant-server/libjvm/objs/linux_x86_64.o.cmdline /disk/openjdk/upstream-sources/openjdk-head/build/linux-x86_64-server-release/make-support/failure-logs/hotspot_variant-server_libjvm_objs_linux_x86_64.o.cmdline && exit $exitcode ) ) I'll run this through jdk/submit before I push. Thoughts? Thanks, Severin
