Re: [llvm-dev] OpenJDK8 failed to work after compiled by LLVM 8 for X86

2018-09-13 Thread Leslie Zhai

Hi Zhengyu,

Thanks for your patch!  I will backport it to jdk8u and test it for X86 
and mips64el.


Thanks,

Leslie Zhai


在 2018年09月12日 20:58, Zhengyu Gu 写道:
Probably should also backport the followup RFE: 
https://bugs.openjdk.java.net/browse/JDK-8206183


Thanks,

-Zhengyu

On 09/11/2018 10:58 PM, David Holmes wrote:
Or to be a little less obscure, this is a known issue and you should 
look into backporting:


https://bugs.openjdk.java.net/browse/JDK-8205965

David

On 12/09/2018 5:03 AM, Martin Buchholz wrote:

https://openjdk.markmail.org/thread/rwfcd6df6vhzli5m






Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2018-09-13 Thread Leslie Zhai

Hi Andrew,

Thanks for your kind response!


在 2018年09月11日 01:19, Andrew Hughes 写道:

On Wed, 5 Sep 2018 at 10:52, Leslie Zhai  wrote:

Hi Andrew,

Thanks for your response!

I just quote it from here:

http://mail.openjdk.java.net/pipermail/build-dev/2016-July/017464.html

I spotted that jsig is just a single C file and so doesn't
need the -std flag. In fact, it complains about it:

Compiling jsig.c (for libjsig.so)
( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2
-std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c >
  >(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
g.o.log) 2> >(/usr/bin/tee
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || (
exitcode=$? && /bin/cp /home/and\
rew/builder/dev/hotspot/libjsig/objs/jsig.o.log
/home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
.log && exit $exitcode ) ) && wait )
cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++
but not for C

It is still able to reproducible the warning for OpenJDK8 mips64el with
GCC and LLVM toolchains.  And gcc treat it as warning,  so it is often
be ignored,  but clang treat it as error.
- 8<  8<  8<  8<  8<  8< ---

Because CXXSTD_CXXFLAG="-std=gnu++98",  such flag might be effect
others,  so workaround for LLVM toolchain:
diff -r 1a87e769fb7f hotspot/make/linux/makefiles/jsig.make
--- a/hotspot/make/linux/makefiles/jsig.makeMon Sep 03 18:02:35 2018
+0800
+++ b/hotspot/make/linux/makefiles/jsig.makeWed Sep 05 15:53:22 2018
+0800
@@ -54,7 +54,7 @@
   $(LIBJSIG): $(JSIGSRCDIR)/jsig.c $(LIBJSIG_MAPFILE)
   @echo Making signal interposition lib...
   $(QUIETLY) $(CC) $(SYMFLAG) $(ARCHFLAG) $(SHARED_FLAG) $(PICFLAG) \
- $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
$(EXTRA_CFLAGS) -o $@ $< -ldl
+ $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
-fintegrated-as -o $@ $< -ldl
   ifeq ($(ENABLE_FULL_DEBUG_SYMBOLS),1)
   $(QUIETLY) $(OBJCOPY) --only-keep-debug $@ $(LIBJSIG_DEBUGINFO)
   $(QUIETLY) $(OBJCOPY) --add-gnu-debuglink=$(LIBJSIG_DEBUGINFO) $@
diff -r 1a87e769fb7f hotspot/make/linux/makefiles/saproc.make
--- a/hotspot/make/linux/makefiles/saproc.makeMon Sep 03 18:02:35
2018 +0800
+++ b/hotspot/make/linux/makefiles/saproc.makeWed Sep 05 15:53:22
2018 +0800
@@ -118,7 +118,7 @@
  $(SASRCFILES) \
  $(SA_LFLAGS) \
  $(SA_DEBUG_CFLAGS) \
-   $(EXTRA_CFLAGS) \
+ -fintegrated-as  \
  -o $@ \
  -lthread_db
   endif

Please give me some suggestion,  thanks a lot!

--
Regards,
Leslie Zhai



Is this the OpenJDK 8 build? It's quite different to the OpenJDK 9+ build I was
referring to in my e-mail.

Yes, I am compiling OpenJDK 8 with LLVM for mips64el
http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023074.html



There are effectively three different OpenJDK build systems:

1. OpenJDK 6 & 7 - the old legacy make & Ant build throughtout
2. OpenJDK 8 - new configure & make build for most repositories, but invoking
the old legacy build for HotSpot
3. OpenJDK 9 - full  new configure & make build across the board

This leaves OpenJDK 8 as unique and quite confusing. It means when backporting
build changes from 9+, they essentially have to be duplicated to apply
to the HotSpot
part of the build as well.

I'm aware of the build warning due to -std in the backport to 8. It
was unavoidable
as the older build only uses CFLAGS, and primarily for C++ code. There is simply
no variable to add it to instead. CXXFLAGS is used there as a variable for some
preprocessor directives :

hotspot/make/linux/makefiles/rules.make:CC_COMPILE   = $(CC)
$(CXXFLAGS) $(CFLAGS)
hotspot/make/linux/makefiles/rules.make:CXX_COMPILE  = $(CXX)
$(CXXFLAGS) $(CFLAGS)

Trying to alter this at this stage in 8u's development is really too
dangerous, considering
the range of systems relying on being able to build 8u. I'm already a
little worried by the
number of build system changes coming into 8u. I would regard the
build system for
a legacy release as needing the same stability guarantees as the code itself.

Certainly removing EXTRA_CFLAGS is not appropriate as this code will
then be built
without flags passed in by configure and other additions made by
configure checks.
Sorry for my monkey patch, I will rebase the workaround compile with 
llvm patch.



Being able to build without that is one thing. Being able to ensure
there are no regressions
on all builds is quite another and we've already seen an issue where
flags for GCC >= 6
were dropped because of a typo [0].

My suggestion would be that -std is filtered out locally where it is
known that 

Re: RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

2018-09-13 Thread Gustavo Romero

Hello Severin,

On 09/12/2018 04:48 AM, Severin Gehwolf wrote:

Hi David,

On Wed, 2018-09-12 at 13:57 +1000, David Holmes wrote:

Hi Severin,

Sorry I'm a bit confused now.

Originally for ppc/s390x/aarch64 the optimization level for fdlibm was
HIGH. But now it will be LOW due to:

45 ifneq ($(FDLIBM_CFLAGS), )
46   BUILD_LIBFDLIBM_OPTIMIZATION := LOW

as those platforms will use gcc/clang with support for -ffp-contract and
so FDLIBM_CFLAGS will not be empty.

??


Correct. That was done based on Andrew Haley's comment here:
http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023160.html

I've done some measurements with -O2 and -O3. -O2 is sufficient for a
2.75 speedup for sin/cos on ppc64le as compared to the -O0 baseline. On
the other hand the speedup of -O3 as compared to -O2 is only 1.05 for
sin/cos on ppc64le.

Since the difference between them were not huge, I've used -O2, a.k.a
LOW. See also:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/microbenchmarks_results/

To me it seems -O2 is sufficiently good. Note that HIGH == -O3 and LOW
== -O2 on gcc arches. Does that clear things up?

FWIW, I'm happy to change it back to HIGH if there are concerns. See
also Gustavo's reply here for some more data points:
http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023193.html


I'm wondering why the possible issue of using -O3 on fdlibm would not affect
other parts of the JVM code (like the Opto code in libjvm.so) that afaics
are also built using -O3. Also, the gap of 1.05 for sin/cos, for instance,
still sounds like a regression to me.

Maybe a better approach to this would be to figure out a way to test -O3 and
-O2 with the real world case in which -O3 can have a bad impact. I'm not
sure if SPECjbb would qualify for that.


Best regards,
Gustavo


Thanks,
Severin


David

On 12/09/2018 2:14 AM, Severin Gehwolf wrote:

Hi Erik,

Thanks for the review!

On Tue, 2018-09-11 at 08:57 -0700, Erik Joelsson wrote:

Hello Severin,

Even if using the macro, I still think you need to add a condition on
the compiler types where the switch can be reasonably expected to exist.


How about this?
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.05/

Thanks,
Severin


On 2018-09-11 05:02, Severin Gehwolf wrote:

On Mon, 2018-09-10 at 09:29 -0700, Erik Joelsson wrote:

I see. I was not aware of that issue, but we clearly need to file a bug
for it and fix it. In this case I think it's fine to us the macro however.


OK. Update webrev, which now uses FLAGS_COMPILER_CHECK_ARGUMENTS.

http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/webrev.04/

Micro-benchmark results from running [1] for x86_64 and ppc64le are
here (-O2 is sufficient it seems):

http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210416/microbenchmarks_results/

More thoughts?

Thanks,
Severin

[1] https://github.com/gromero/strictmath/










Re: RFR: JDK-8210731 PropertiesParser does not produce reproducible output

2018-09-13 Thread Erik Joelsson

Hello,

Looks good. Perhaps add a comment explaining why the otherwise unusual 
choice of collection class is used.


/Erik

On 2018-09-13 15:20, Magnus Ihse Bursie wrote:
The file make/langtools/tools/propertiesparser/PropertiesParser.java 
b/make/langtools/tools/propertiesparser/PropertiesParser.java is used 
to convert .properties files into .java files as part of the gensrc step.


However, due to it's use of creating it's output directly from 
HashMaps, it's not guaranteed to be stable, and is causing spurios 
differences in our cmp-baseline builds.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210731
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210731-properties-parser-is-not-stable/webrev.01


/Magnus





Re: Why static_jli for java/javaw on Windows?

2018-09-13 Thread Erik Joelsson
I checked and the copying of java.exe was done in the now legacy jre 
installer, so from what I can tell, there is no longer a need for static 
linking.


/Erik


On 2018-09-13 09:14, Erik Joelsson wrote:

Hello,

Reading that bug, it seems the problem is when the installer copies 
java.exe into the Windows system directory. In that case, it may not 
have access to the msvcr re-distributables. I will try to find out if 
our installers are still doing this.


/Erik


On 2018-09-13 06:32, Alan Bateman wrote:



On 13/09/2018 14:07, Magnus Ihse Bursie wrote:

:

Apparently, someone was trying to get rid of dll dependencies from 
java.exe. Why that would be desirable it does not say. Neither why 
this should not apply to all other launchers. (Perhaps there were 
not that many in these days?)


Do anyone think this still seems relevant? Otherwise I'd like to get 
rid of this hack, and link java and javaw just like all the other 
launchers.


I don't know if it is still needed but it seems to be related to the 
upgrade to VC 7 and an issue related to redistribution issue of the 
MS runtime. JDK-6282039 and JDK-6382014 have some info on this.


-Alan.






Re: RFR: JDK-8210729 Clean up macosx static library handling

2018-09-13 Thread Erik Joelsson

Hello,

Nice cleanup overall.

In CoreLibraries.gmk: I don't think it's safe to use $(BUILD_LIBFDLIB) 
directly as input to LIBS. That variable may contain other targets as 
well (such as debug symbol files and soon compile_commands.js snippets). 
A safer variable to use is $(BUILD_FDLIBM_TARGET). I even documented 
this as an output variable in NativeCompilation.gmk. I realize this was 
already used before your patch however.


I would expect a certain level of testing to be done for a change like 
this to make sure all affected libraries are verified.


/Erik

On 2018-09-13 15:06, Magnus Ihse Bursie wrote:
There's a bunch of interrelated smelly code regarding static libraries 
on macosx.


I started by turning libfdlibm into a static library on macosx, as on 
all other platforms. It turned out that we don't have proper support 
for static libraries on macosx, so I fixed this too.


Secondly, I removed the libjli_static.a for macosx. There is no good 
reason to have it. It's probably just a left-over from the old Apple 
port to macosx. This in turn prompted some additional related cleanup 
in LauncherCommon.gmk.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210729
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210729-cleanup-macosx-static-library-handling/webrev.01


/Magnus





Re: RFR: JDK-8210731 PropertiesParser does not produce reproducible output

2018-09-13 Thread Jonathan Gibbons

+1

-- Jon

On 09/13/2018 03:25 PM, mandy chung wrote:

Looks okay to me.

Mandy
P.S. I cc'ed compiler-dev since I think you meant to cc compiler-dev 
instead of hotspot-compiler-dev.


On 9/13/18 3:20 PM, Magnus Ihse Bursie wrote:
The file make/langtools/tools/propertiesparser/PropertiesParser.java 
b/make/langtools/tools/propertiesparser/PropertiesParser.java is used 
to convert .properties files into .java files as part of the gensrc 
step.


However, due to it's use of creating it's output directly from 
HashMaps, it's not guaranteed to be stable, and is causing spurios 
differences in our cmp-baseline builds.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210731
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210731-properties-parser-is-not-stable/webrev.01


/Magnus







Re: RFR: JDK-8210731 PropertiesParser does not produce reproducible output

2018-09-13 Thread mandy chung

Looks okay to me.

Mandy
P.S. I cc'ed compiler-dev since I think you meant to cc compiler-dev 
instead of hotspot-compiler-dev.


On 9/13/18 3:20 PM, Magnus Ihse Bursie wrote:
The file make/langtools/tools/propertiesparser/PropertiesParser.java 
b/make/langtools/tools/propertiesparser/PropertiesParser.java is used 
to convert .properties files into .java files as part of the gensrc step.


However, due to it's use of creating it's output directly from 
HashMaps, it's not guaranteed to be stable, and is causing spurios 
differences in our cmp-baseline builds.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210731
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210731-properties-parser-is-not-stable/webrev.01


/Magnus





RFR: JDK-8210731 PropertiesParser does not produce reproducible output

2018-09-13 Thread Magnus Ihse Bursie
The file make/langtools/tools/propertiesparser/PropertiesParser.java 
b/make/langtools/tools/propertiesparser/PropertiesParser.java is used to 
convert .properties files into .java files as part of the gensrc step.


However, due to it's use of creating it's output directly from HashMaps, 
it's not guaranteed to be stable, and is causing spurios differences in 
our cmp-baseline builds.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210731
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210731-properties-parser-is-not-stable/webrev.01


/Magnus



RFR: JDK-8210729 Clean up macosx static library handling

2018-09-13 Thread Magnus Ihse Bursie
There's a bunch of interrelated smelly code regarding static libraries 
on macosx.


I started by turning libfdlibm into a static library on macosx, as on 
all other platforms. It turned out that we don't have proper support for 
static libraries on macosx, so I fixed this too.


Secondly, I removed the libjli_static.a for macosx. There is no good 
reason to have it. It's probably just a left-over from the old Apple 
port to macosx. This in turn prompted some additional related cleanup in 
LauncherCommon.gmk.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210729
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210729-cleanup-macosx-static-library-handling/webrev.01


/Magnus



Re: Linux + Clang + execstack

2018-09-13 Thread Magnus Ihse Bursie




We're not entirely happy either.

A much higher interface might look like

TRY_ADD_LINKER_FLAGS -z noexecstack

Agreed. I'm working towards a solution like that.

which would add -Wl,-z,noexecstack to LDFLAGS when appropriate
 hmmm ...
I only just noticed that both gcc and clang accept simply
$CC -z noexecstack
(it's even documented!)
Should we switch to that instead?
No, I think it's better to keep -Wl,-z for consistency for all linker 
flags. Otherwise it just looks confusing.


/Magnus





Do you have a JBS issue?

I have
https://bugs.openjdk.java.net/browse/JDK-8205457 gcc and clang should
use the same ld flags
but the proposed patch only addresses part of that.  I could create a
sub-task (but I've never done that before) or a new bug or change the
description of this bug.  What do you think?




Re: RFR(8u): 8202557: OpenJDK fails to start in Windows 7 and 8.1 after upgrading compiler to VC 2017

2018-09-13 Thread Erik Joelsson

Looks good.

/Erik


On 2018-09-13 14:03, Kevin Walls wrote:

Hi,

I'd like to get a review of an 8u backport of:

8202557: OpenJDK fails to start in Windows 7 and 8.1 after upgrading 
compiler to VC 2017

https://bugs.openjdk.java.net/browse/JDK-8202557

11 change: http://hg.openjdk.java.net/jdk/jdk/rev/35b22ca681d1

11 review thread: 
http://mail.openjdk.java.net/pipermail/build-dev/2018-May/022000.html


8u webrevs: (base repo and jdk, and rebuild autoconf generated files)
http://cr.openjdk.java.net/~kevinw/8202557/webrev.base.01/
http://cr.openjdk.java.net/~kevinw/8202557/webrev.jdk.01/

Thanks
Kevin






RFR(8u): 8202557: OpenJDK fails to start in Windows 7 and 8.1 after upgrading compiler to VC 2017

2018-09-13 Thread Kevin Walls

Hi,

I'd like to get a review of an 8u backport of:

8202557: OpenJDK fails to start in Windows 7 and 8.1 after upgrading 
compiler to VC 2017

https://bugs.openjdk.java.net/browse/JDK-8202557

11 change: http://hg.openjdk.java.net/jdk/jdk/rev/35b22ca681d1

11 review thread: 
http://mail.openjdk.java.net/pipermail/build-dev/2018-May/022000.html


8u webrevs: (base repo and jdk, and rebuild autoconf generated files)
http://cr.openjdk.java.net/~kevinw/8202557/webrev.base.01/
http://cr.openjdk.java.net/~kevinw/8202557/webrev.jdk.01/

Thanks
Kevin




Re: Linux + Clang + execstack

2018-09-13 Thread Martin Buchholz
On Thu, Sep 13, 2018 at 12:48 PM, Magnus Ihse Bursie
 wrote:

>>
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/noexecstack/noexecstack.patch
>
> I'm not entirely happy, but it'll have to do. The problem here is that the
> underlying structure of the flags handling is still not good so this
> probably cannot be expressed better than this.

We're not entirely happy either.

A much higher interface might look like

TRY_ADD_LINKER_FLAGS -z noexecstack
which would add -Wl,-z,noexecstack to LDFLAGS when appropriate
 hmmm ...
I only just noticed that both gcc and clang accept simply
$CC -z noexecstack
(it's even documented!)
Should we switch to that instead?


> Do you have a JBS issue?

I have
https://bugs.openjdk.java.net/browse/JDK-8205457 gcc and clang should
use the same ld flags
but the proposed patch only addresses part of that.  I could create a
sub-task (but I've never done that before) or a new bug or change the
description of this bug.  What do you think?


Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Magnus Ihse Bursie

On 2018-09-13 21:59, Erik Joelsson wrote:

On 2018-09-13 12:21, Magnus Ihse Bursie wrote:

On 2018-09-13 19:58, Erik Joelsson wrote:
I believe it's because they are C++ instead of pure C. We always had 
a harder time comparing C++ binaries.

Hm, is that so? You mean as in linking using TOOLCHAIN_LINK_CXX?

More because there are C++ source files in them, .cpp in libsaproc and 
.cc in libfontmanager.
I was just about to argue that we have lots of other libraries with C++ 
source files in them, but when I checked I noticed that they coincided 
quite well with the libraries that has caused us most issues in the 
comparisons. Hm. I've never noticed that pattern before. Interesting.


/Magnus



/Erik
I'm just a bit "allergic" towards libfontmanager. There's a lot of 
odd things happening there; it's one of our most atypical libraries. 
Loads of disabled warnings. Some of them might warn about code that 
could cause this unstableness.


I plan to look more closely into libfontmanager in the 
hopefully-not-too-long-term future.


/Magnus



/Erik


On 2018-09-13 10:48, Phil Race wrote:

Ditto, although as the "owner" of libfontmanager I am curious what
are the rare characteristics that make just these two libraries 
"unstable" in this sense ?


-phil.

On 09/13/2018 10:24 AM, Erik Joelsson wrote:

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the 
compilation of libfontmanager and libsaproc is not stable (and 
thus can produce different disassembly each build).


This patch adds an exception for these libraries. (Note that one 
of them was already on the exception list, but only on slowdebug 
builds).


Ideally, we'd rather hunt down the problems with reproducability, 
but that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus















Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Erik Joelsson

On 2018-09-13 12:21, Magnus Ihse Bursie wrote:

On 2018-09-13 19:58, Erik Joelsson wrote:
I believe it's because they are C++ instead of pure C. We always had 
a harder time comparing C++ binaries.

Hm, is that so? You mean as in linking using TOOLCHAIN_LINK_CXX?

More because there are C++ source files in them, .cpp in libsaproc and 
.cc in libfontmanager.


/Erik
I'm just a bit "allergic" towards libfontmanager. There's a lot of odd 
things happening there; it's one of our most atypical libraries. Loads 
of disabled warnings. Some of them might warn about code that could 
cause this unstableness.


I plan to look more closely into libfontmanager in the 
hopefully-not-too-long-term future.


/Magnus



/Erik


On 2018-09-13 10:48, Phil Race wrote:

Ditto, although as the "owner" of libfontmanager I am curious what
are the rare characteristics that make just these two libraries 
"unstable" in this sense ?


-phil.

On 09/13/2018 10:24 AM, Erik Joelsson wrote:

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the 
compilation of libfontmanager and libsaproc is not stable (and 
thus can produce different disassembly each build).


This patch adds an exception for these libraries. (Note that one 
of them was already on the exception list, but only on slowdebug 
builds).


Ideally, we'd rather hunt down the problems with reproducability, 
but that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus













Re: Linux + Clang + execstack

2018-09-13 Thread Magnus Ihse Bursie

On 2018-09-12 18:35, Martin Buchholz wrote:

On Wed, Sep 12, 2018 at 4:01 AM, Magnus Ihse Bursie
 wrote:

On 2018-09-05 20:59, Martin Buchholz wrote:

So ... Magnus, are you happy with the current state of the proposed patch?

I'm sorry Martin, but I can't figure out what the current state is. I tried
backtracking the discussion but failed. :( Can you please repost the
currently proposed patch?

http://cr.openjdk.java.net/~martin/webrevs/jdk/noexecstack/noexecstack.patch
I'm not entirely happy, but it'll have to do. The problem here is that 
the underlying structure of the flags handling is still not good so this 
probably cannot be expressed better than this.


Do you have a JBS issue?

/Magnus




On Tue, Sep 4, 2018 at 11:50 PM, Magnus Ihse Bursie
 wrote:


For the gcc toolchain this can not be the case:
# Minimum supported linker versions, empty means unspecified
TOOLCHAIN_MINIMUM_LD_VERSION_gcc="2.18"

We make sure we have an ld that supports the basic flags we assume.


feature tests are better than version tests.

I've heard that argument many times, and I've never agreed with it. In some
cases, feature tests work. They typically work if someone has designed a
clear API and included a feature test in it. A lot of additional POSIX
functionality works that way. This means that you can rest assure that if
the feature is present, then you know what you are going to get.

In most of the rest of the world, functionality does not raise to that
golden standard. Gcc adds a flag in one version, but it's buggy. A later
version fixes the bugs. A later version still changes the behavior of the
flag. Functionality that we depend on works or does not works depending on
the intersection of things like our code, compiler version, operating
system, and so on.

In my experience, it's a rare thing for a feature test to actually work.
Version tests, on the other hand, tests against a specific setup, that can
be tested and proven to work. The downside of version tests is that they are
often open-ended; we say that "anything above this version is supposed to
work", even though we have not tested with gcc 8 or 9. The alternative is to
say that "we've tested this between gcc 4.7 and 7.3 and will only support it
for this version span", but that is in most cases more likely to break when
gcc 8 comes along.

Specific version tests are in principle more accurate, but they
require a level of testing and maintenance that is unlikely to be seen
in the real world.  The received wisdom is that one should prefer
feature tests whenever possible and I agree with that as well, based
on decades of experience.

Sometimes you need something in between, e.g. replacing a
configure-time check with a run-time check.




RFR: JDK-8210723 Better information in configure for invalid Xcode

2018-09-13 Thread Magnus Ihse Bursie
When running e.g. Xcode 9.4 on macosx 10.12, the configure scripts fails 
with: "Failed to determine Xcode version" This is not very helpful.


Adding the output of the failed xcodebuild call (typically something 
like "Executable requires at least macOS 10.13, but is being run on 
macOS 10.12.6, and so is exiting") will help the user diagnose the error.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210723
Patch inline:
diff --git a/make/autoconf/toolchain.m4 b/make/autoconf/toolchain.m4
--- a/make/autoconf/toolchain.m4
+++ b/make/autoconf/toolchain.m4
@@ -232,6 +232,7 @@
   XCODE_VERSION_OUTPUT=`"$XCODEBUILD" -version 2>&1 | $HEAD -n 1`
   $ECHO "$XCODE_VERSION_OUTPUT" | $GREP "Xcode " > /dev/null
   if test $? -ne 0; then
+    AC_MSG_NOTICE([xcodebuild output: $XCODE_VERSION_OUTPUT])
 AC_MSG_ERROR([Failed to determine Xcode version.])
   fi
   XCODE_MAJOR_VERSION=`$ECHO $XCODE_VERSION_OUTPUT | \

/Magnus



Re: [12]: RFR: 8209167: Use CLDR's time zone mappings for Windows

2018-09-13 Thread Magnus Ihse Bursie




On 2018-09-12 19:33, naoto.s...@oracle.com wrote:

Hi Magnus, Erik,

Thank you for your comments. I updated the webrev according to your 
suggestions:


http://cr.openjdk.java.net/~naoto/8209167/webrev.02/

Looks good to me.

/Magnus



As to the duplicated "common" in the dependency, yes that's a separate 
issue. Since it's obvious, I included the fix with this changeset (it 
was actually described as a comment in the jira issue).


Naoto

On 9/12/18 9:08 AM, Erik Joelsson wrote:

On 2018-09-12 03:19, Magnus Ihse Bursie wrote:



On 2018-09-10 23:34, Naoto Sato wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8209167

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8209167/webrev.01/


Some comments:
In make/copy/Copy-java.base.gmk:
+ifneq ($(findstring $(OPENJDK_TARGET_OS), aix),)

The findstring construct is hard to read and only needed when we 
test more than one OS. Please rewrite as a single ifeq test for aix.


In GensrcCLDR.gmk:
I don't understand the CLDR_WINTZMAPPINGS file? There's no rule to 
generate it, there's just a dependency..?


It's generated by the same rule as the other file. This is the 
easiest workaround for two files generated from the same rule. It 
sort of works (for clean builds and if the input is chagned), but 
won't handle all cases where one file is removed externally and the 
other is not. I suggested this construct to Naoto. Perhaps a comment 
explaining this would be good.
The removal of the duplicate "common", that seems like a separate 
bug fix?



It does indeed. Before this fix, the wildcards must have returned empty.

/Erik

/Magnus



This fix is to remove the hand crafted map file (lib/tzmappings) on 
Windows, which maps Windows time zones to Java's tzid. It is now 
dynamically generated at the build time, using CLDR's data file 
(windowsZones.xml)


Naoto








Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Magnus Ihse Bursie

On 2018-09-13 19:58, Erik Joelsson wrote:
I believe it's because they are C++ instead of pure C. We always had a 
harder time comparing C++ binaries.

Hm, is that so? You mean as in linking using TOOLCHAIN_LINK_CXX?

I'm just a bit "allergic" towards libfontmanager. There's a lot of odd 
things happening there; it's one of our most atypical libraries. Loads 
of disabled warnings. Some of them might warn about code that could 
cause this unstableness.


I plan to look more closely into libfontmanager in the 
hopefully-not-too-long-term future.


/Magnus



/Erik


On 2018-09-13 10:48, Phil Race wrote:

Ditto, although as the "owner" of libfontmanager I am curious what
are the rare characteristics that make just these two libraries 
"unstable" in this sense ?


-phil.

On 09/13/2018 10:24 AM, Erik Joelsson wrote:

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the 
compilation of libfontmanager and libsaproc is not stable (and thus 
can produce different disassembly each build).


This patch adds an exception for these libraries. (Note that one of 
them was already on the exception list, but only on slowdebug builds).


Ideally, we'd rather hunt down the problems with reproducability, 
but that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus











Re: RFR: JDK-8210704 Remove dead build tools hasher and jarreorder

2018-09-13 Thread mandy chung




On 9/13/18 1:46 AM, Magnus Ihse Bursie wrote:

On 2018-09-13 10:35, Magnus Ihse Bursie wrote:
The two build tools hasher and jarreorder is not used in the build 
anymore and should be removed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210704
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.01


Updated webrev that also removes the dead declaration for 
TOOL_GENMODULESXML. (The tool itself was previously removed)


http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.02 



Looks good.  Thanks for cleaning up TOOL_GENMODULESXML which was a 
temporary tool that we missed to remove.


Mandy


Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Erik Joelsson
I believe it's because they are C++ instead of pure C. We always had a 
harder time comparing C++ binaries.


/Erik


On 2018-09-13 10:48, Phil Race wrote:

Ditto, although as the "owner" of libfontmanager I am curious what
are the rare characteristics that make just these two libraries 
"unstable" in this sense ?


-phil.

On 09/13/2018 10:24 AM, Erik Joelsson wrote:

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the compilation 
of libfontmanager and libsaproc is not stable (and thus can produce 
different disassembly each build).


This patch adds an exception for these libraries. (Note that one of 
them was already on the exception list, but only on slowdebug builds).


Ideally, we'd rather hunt down the problems with reproducability, 
but that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus









Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Phil Race

Ditto, although as the "owner" of libfontmanager I am curious what
are the rare characteristics that make just these two libraries 
"unstable" in this sense ?


-phil.

On 09/13/2018 10:24 AM, Erik Joelsson wrote:

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the compilation 
of libfontmanager and libsaproc is not stable (and thus can produce 
different disassembly each build).


This patch adds an exception for these libraries. (Note that one of 
them was already on the exception list, but only on slowdebug builds).


Ideally, we'd rather hunt down the problems with reproducability, but 
that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus







Re: 8210425: [x86] sharedRuntimeTrig/sharedRuntimeTrans compiled without optimization

2018-09-13 Thread Erik Joelsson

On 2018-09-13 03:02, Severin Gehwolf wrote:

Hi Erik,

On Wed, 2018-09-12 at 10:02 -0700, Erik Joelsson wrote:

Hello Severin,

In configure, we now set FDLIBM_CFLAGS based on toolchain type and
capabilities. In JvmOverrideFiles.gmk, the use of it is still
conditional on Linux. I don't think it should be. We already have the
conditionals we need.

Thanks for the review. Updated webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.03/

I wasn't sure whether the precompiled headers handling for gcc/clang is
right and was reluctant to move it out of the linux conditional. The
assumption on my end is that if headers are compiled with -O3, we can't
used them for any other opt level. It should still all work. Thoughts?
Looking at this again, I now realize the whole file has the treatment 
for these files repeated for each OS, with slight variations. This 
becomes a clash with the change we are now attempting which is toolchain 
oriented. I think the easiest way here would be to keep it OS separated 
for now by reverting to your previous patch.


You could consider putting the FDLIBM_CFLAGS conditional block outside 
the linux block however and apply the "$(FDLIBM_CFLAGS) 
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)" flags on the lines in the macosx block 
further down as well. The changes for fdlibm as they are now will apply 
on macosx since we use clang there, so the jvm change should too.


/Erik

Thanks,
Severin


On 2018-09-12 05:44, Severin Gehwolf wrote:

Hi,

Updated webrev since fdlibm build change seems to have settled:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.02/

Optimization is being done at level -O2 (CXX_O_FLAG_NORM) with
-ffp-contract=off when the toolchain (gcc/clang at this point)
supports it. Otherwise no optimization will be done. Those overrides
are no longer in an x86 specific block. Thus, ppc64, aarch64, s390x
will get correct settings too.

How does this look?

Thanks,
Severin

On Wed, 2018-09-12 at 07:16 +, Schmidt, Lutz wrote:

I totally agree with Andrew's statement. FP calculations should be
evaluated as the programmer wrote them down. All fiddling around with
sequence or rounding is evil. You lose reproducibility of results.
Regards,
Lutz

On 08.09.18, 11:26, "hotspot-dev on behalf of Andrew Haley" 
 wrote:

  On 09/06/2018 03:32 PM, Severin Gehwolf wrote:
  > Right. I should note that ppc64, s390x and aarch64 ports don't have
  > this optimization turned off as those overrides are in a x86 specific
  > block. It appears it hasn't caused issues for these ports so far.
  
  That's just dumb luck. We really should turn off the merging of FP

  operations on all platforms.
  
  --

  Andrew Haley
  Java Platform Lead Engineer
  Red Hat UK Ltd. 
  EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
  







Re: RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Erik Joelsson

Looks good.

/Erik


On 2018-09-13 02:23, Magnus Ihse Bursie wrote:
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the compilation 
of libfontmanager and libsaproc is not stable (and thus can produce 
different disassembly each build).


This patch adds an exception for these libraries. (Note that one of 
them was already on the exception list, but only on slowdebug builds).


Ideally, we'd rather hunt down the problems with reproducability, but 
that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus





Re: RFR: JDK-8210704 Remove dead build tools hasher and jarreorder

2018-09-13 Thread Erik Joelsson

Looks good.

/Erik


On 2018-09-13 01:46, Magnus Ihse Bursie wrote:

On 2018-09-13 10:35, Magnus Ihse Bursie wrote:
The two build tools hasher and jarreorder is not used in the build 
anymore and should be removed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210704
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.01


Updated webrev that also removes the dead declaration for 
TOOL_GENMODULESXML. (The tool itself was previously removed)


http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.02 



/Magnus





Re: RFR: JDK-8210702 Remove dtrace mapfiles

2018-09-13 Thread Erik Joelsson

Looks ok to me.

/Erik


On 2018-09-13 01:00, Magnus Ihse Bursie wrote:
This is the last part of the mapfile cleanup. Two special libraries 
for supporting dtrace on solaris was not converted before.


As before, I have verified that there is no unwanted symbol changes in 
the resulting libraries.


(This fix also removes the libjsound mapfile once and for all -- the 
previous removal was lost due to hg merge issues.)


Bug: https://bugs.openjdk.java.net/browse/JDK-8210702
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210702-remove-last-mapfiles/webrev.01


/Magnus





Re: RFR: 8210703: vmStructs.cpp compiled with -O0

2018-09-13 Thread Erik Joelsson

Looks good to me. Thanks for cleaning this up!

/Erik


On 2018-09-13 08:06, Severin Gehwolf wrote:

Hi Magnus,

On Thu, 2018-09-13 at 13:52 +0200, Magnus Ihse Bursie wrote:

Severin,

Your patch looks good to me. I'm assuming you have tested that nothing breaks.

Thanks for the review!

Yes, I've tested with run-test-tier1 which seems OK. It includes
serviceability/sa/ClhsdbVmStructsDump. I've also run VMStructs gtest.
I'm currently running this through jdk-submit.

Thanks,
Severin


/Magnus


13 sep. 2018 kl. 10:43 skrev Severin Gehwolf :

Hi Erik,


On Wed, 2018-09-12 at 10:25 -0700, Erik Joelsson wrote:
Hello,

I very much doubt it was included with the new build system. We were
extremely careful to use the exact same flags then, and did binary
comparisons of all object files to verify equal builds.

Tracing back, it was caused (probably unintentionally) by this change:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/e796d52ca85b

Thanks for the detective work! This looks very much unintentional to me
as JDK-8114853 is about silencing a note.


In the old build, setting OPT_CFLAGS/ overrides the common
OPT_CFLAGS. In the new build, we have a more general way of adding flags
for specific files that does not directly override any other flags. To
get the same behavior for vmStructs.cpp in the new build as in the old
at the time of the switch, we had to add -O0 explicitly in the new build.

I've filed JDK-8210703 and posting this patch as review for the bug.
Note I've changed the subject. Does this look OK?

diff --git a/make/hotspot/lib/JvmOverrideFiles.gmk 
b/make/hotspot/lib/JvmOverrideFiles.gmk
--- a/make/hotspot/lib/JvmOverrideFiles.gmk
+++ b/make/hotspot/lib/JvmOverrideFiles.gmk
@@ -30,7 +30,7 @@
# status for individual files on specific platforms.

ifeq ($(TOOLCHAIN_TYPE), gcc)
-  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments -O0
+  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments
   BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := -fno-var-tracking-assignments
   BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
-fno-var-tracking-assignments
   BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized

Thanks,
Severin

On 2018-09-12 09:13, Severin Gehwolf wrote:

Hi,

Does anybody know why vmStructs.cpp gets an override in
JvmOverrideFiles.gmk:

$ grep -C3 -n vmStructs.cpp make/hotspot/lib/JvmOverrideFiles.gmk
30-# status for individual files on specific platforms.
31-
32-ifeq ($(TOOLCHAIN_TYPE), gcc)
33:  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments -O0
34-  BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := 
-fno-var-tracking-assignments
35-  BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
-fno-var-tracking-assignments
36-  BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized

It seems to have been introduced with the new build system. JDK 8
doesn't seem to have it. Thoughts?

Thanks,
Severin






Re: Why static_jli for java/javaw on Windows?

2018-09-13 Thread Erik Joelsson

Hello,

Reading that bug, it seems the problem is when the installer copies 
java.exe into the Windows system directory. In that case, it may not 
have access to the msvcr re-distributables. I will try to find out if 
our installers are still doing this.


/Erik


On 2018-09-13 06:32, Alan Bateman wrote:



On 13/09/2018 14:07, Magnus Ihse Bursie wrote:

:

Apparently, someone was trying to get rid of dll dependencies from 
java.exe. Why that would be desirable it does not say. Neither why 
this should not apply to all other launchers. (Perhaps there were not 
that many in these days?)


Do anyone think this still seems relevant? Otherwise I'd like to get 
rid of this hack, and link java and javaw just like all the other 
launchers.


I don't know if it is still needed but it seems to be related to the 
upgrade to VC 7 and an issue related to redistribution issue of the MS 
runtime. JDK-6282039 and JDK-6382014 have some info on this.


-Alan.




Re: RFR: 8210703: vmStructs.cpp compiled with -O0 (was: Why is vmStructs.cpp compiled with -O0?

2018-09-13 Thread Severin Gehwolf
Hi Magnus,

On Thu, 2018-09-13 at 13:52 +0200, Magnus Ihse Bursie wrote:
> Severin,
> 
> Your patch looks good to me. I'm assuming you have tested that nothing 
> breaks. 

Thanks for the review!

Yes, I've tested with run-test-tier1 which seems OK. It includes
serviceability/sa/ClhsdbVmStructsDump. I've also run VMStructs gtest.
I'm currently running this through jdk-submit.

Thanks,
Severin

> /Magnus
> 
> > 13 sep. 2018 kl. 10:43 skrev Severin Gehwolf :
> > 
> > Hi Erik,
> > 
> > > On Wed, 2018-09-12 at 10:25 -0700, Erik Joelsson wrote:
> > > Hello,
> > > 
> > > I very much doubt it was included with the new build system. We were 
> > > extremely careful to use the exact same flags then, and did binary 
> > > comparisons of all object files to verify equal builds.
> > > 
> > > Tracing back, it was caused (probably unintentionally) by this change:
> > > 
> > > http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/e796d52ca85b
> > 
> > Thanks for the detective work! This looks very much unintentional to me
> > as JDK-8114853 is about silencing a note.
> > 
> > > In the old build, setting OPT_CFLAGS/ overrides the common 
> > > OPT_CFLAGS. In the new build, we have a more general way of adding flags 
> > > for specific files that does not directly override any other flags. To 
> > > get the same behavior for vmStructs.cpp in the new build as in the old 
> > > at the time of the switch, we had to add -O0 explicitly in the new build.
> > 
> > I've filed JDK-8210703 and posting this patch as review for the bug.
> > Note I've changed the subject. Does this look OK?
> > 
> > diff --git a/make/hotspot/lib/JvmOverrideFiles.gmk 
> > b/make/hotspot/lib/JvmOverrideFiles.gmk
> > --- a/make/hotspot/lib/JvmOverrideFiles.gmk
> > +++ b/make/hotspot/lib/JvmOverrideFiles.gmk
> > @@ -30,7 +30,7 @@
> > # status for individual files on specific platforms.
> > 
> > ifeq ($(TOOLCHAIN_TYPE), gcc)
> > -  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments -O0
> > +  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments
> >   BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := 
> > -fno-var-tracking-assignments
> >   BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
> > -fno-var-tracking-assignments
> >   BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized
> > 
> > Thanks,
> > Severin
> > 
> > On 2018-09-12 09:13, Severin Gehwolf wrote:
> > > > Hi,
> > > > 
> > > > Does anybody know why vmStructs.cpp gets an override in
> > > > JvmOverrideFiles.gmk:
> > > > 
> > > > $ grep -C3 -n vmStructs.cpp make/hotspot/lib/JvmOverrideFiles.gmk
> > > > 30-# status for individual files on specific platforms.
> > > > 31-
> > > > 32-ifeq ($(TOOLCHAIN_TYPE), gcc)
> > > > 33:  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := 
> > > > -fno-var-tracking-assignments -O0
> > > > 34-  BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := 
> > > > -fno-var-tracking-assignments
> > > > 35-  BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
> > > > -fno-var-tracking-assignments
> > > > 36-  BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized
> > > > 
> > > > It seems to have been introduced with the new build system. JDK 8
> > > > doesn't seem to have it. Thoughts?
> > > > 
> > > > Thanks,
> > > > Severin
> 
> 



Re: Why static_jli for java/javaw on Windows?

2018-09-13 Thread Alan Bateman




On 13/09/2018 14:07, Magnus Ihse Bursie wrote:

:

Apparently, someone was trying to get rid of dll dependencies from 
java.exe. Why that would be desirable it does not say. Neither why 
this should not apply to all other launchers. (Perhaps there were not 
that many in these days?)


Do anyone think this still seems relevant? Otherwise I'd like to get 
rid of this hack, and link java and javaw just like all the other 
launchers.


I don't know if it is still needed but it seems to be related to the 
upgrade to VC 7 and an issue related to redistribution issue of the MS 
runtime. JDK-6282039 and JDK-6382014 have some info on this.


-Alan.


Why static_jli for java/javaw on Windows?

2018-09-13 Thread Magnus Ihse Bursie
On Windows, we compile two versions of libjli, one normal dynamic dll, 
and one static library *). Then when we create our launchers, we link 
with the normal, dynamic libjli for all launchers, except java.exe and 
javaw.exe, which are linked with the static library.


The build system needs to do some elaborate dancing to support this odd 
behavior, which affects code quality negatively. I would therefore like 
to remove this special treatment of java.exe and javaw.exe.


I've done some extensive amount of code archeology to try and determine 
where and how this was added. The only thing I can figure out is that it 
was this way in the old build system, in the initial load of OpenJDK. In 
jdk/make/java/main/java/Makefile, two comments might shed some light:


# Statically link java to avoid the dependency on msvcr71.dll.
and
# Statically link java to avoid the dependency on jli.dll.

Apparently, someone was trying to get rid of dll dependencies from 
java.exe. Why that would be desirable it does not say. Neither why this 
should not apply to all other launchers. (Perhaps there were not that 
many in these days?)


Do anyone think this still seems relevant? Otherwise I'd like to get rid 
of this hack, and link java and javaw just like all the other launchers.


/Magnus

*) Yeah, we do other funky stuff with libjli on other platforms as well. 
But for now, I'm focusing on Windows.


Re: RFR: 8210703: vmStructs.cpp compiled with -O0 (was: Why is vmStructs.cpp compiled with -O0?

2018-09-13 Thread Magnus Ihse Bursie
Severin,

Your patch looks good to me. I'm assuming you have tested that nothing breaks. 

/Magnus

> 13 sep. 2018 kl. 10:43 skrev Severin Gehwolf :
> 
> Hi Erik,
> 
>> On Wed, 2018-09-12 at 10:25 -0700, Erik Joelsson wrote:
>> Hello,
>> 
>> I very much doubt it was included with the new build system. We were 
>> extremely careful to use the exact same flags then, and did binary 
>> comparisons of all object files to verify equal builds.
>> 
>> Tracing back, it was caused (probably unintentionally) by this change:
>> 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/e796d52ca85b
> 
> Thanks for the detective work! This looks very much unintentional to me
> as JDK-8114853 is about silencing a note.
> 
>> In the old build, setting OPT_CFLAGS/ overrides the common 
>> OPT_CFLAGS. In the new build, we have a more general way of adding flags 
>> for specific files that does not directly override any other flags. To 
>> get the same behavior for vmStructs.cpp in the new build as in the old 
>> at the time of the switch, we had to add -O0 explicitly in the new build.
> 
> I've filed JDK-8210703 and posting this patch as review for the bug.
> Note I've changed the subject. Does this look OK?
> 
> diff --git a/make/hotspot/lib/JvmOverrideFiles.gmk 
> b/make/hotspot/lib/JvmOverrideFiles.gmk
> --- a/make/hotspot/lib/JvmOverrideFiles.gmk
> +++ b/make/hotspot/lib/JvmOverrideFiles.gmk
> @@ -30,7 +30,7 @@
> # status for individual files on specific platforms.
> 
> ifeq ($(TOOLCHAIN_TYPE), gcc)
> -  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments -O0
> +  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments
>   BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := -fno-var-tracking-assignments
>   BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
> -fno-var-tracking-assignments
>   BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized
> 
> Thanks,
> Severin
> 
> On 2018-09-12 09:13, Severin Gehwolf wrote:
>>> Hi,
>>> 
>>> Does anybody know why vmStructs.cpp gets an override in
>>> JvmOverrideFiles.gmk:
>>> 
>>> $ grep -C3 -n vmStructs.cpp make/hotspot/lib/JvmOverrideFiles.gmk
>>> 30-# status for individual files on specific platforms.
>>> 31-
>>> 32-ifeq ($(TOOLCHAIN_TYPE), gcc)
>>> 33:  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments 
>>> -O0
>>> 34-  BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := 
>>> -fno-var-tracking-assignments
>>> 35-  BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
>>> -fno-var-tracking-assignments
>>> 36-  BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized
>>> 
>>> It seems to have been introduced with the new build system. JDK 8
>>> doesn't seem to have it. Thoughts?
>>> 
>>> Thanks,
>>> Severin
> 



Re: 8210425: [x86] sharedRuntimeTrig/sharedRuntimeTrans compiled without optimization

2018-09-13 Thread Severin Gehwolf
Hi Erik,

On Wed, 2018-09-12 at 10:02 -0700, Erik Joelsson wrote:
> Hello Severin,
> 
> In configure, we now set FDLIBM_CFLAGS based on toolchain type and 
> capabilities. In JvmOverrideFiles.gmk, the use of it is still 
> conditional on Linux. I don't think it should be. We already have the 
> conditionals we need.

Thanks for the review. Updated webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.03/

I wasn't sure whether the precompiled headers handling for gcc/clang is
right and was reluctant to move it out of the linux conditional. The
assumption on my end is that if headers are compiled with -O3, we can't
used them for any other opt level. It should still all work. Thoughts?

Thanks,
Severin

> On 2018-09-12 05:44, Severin Gehwolf wrote:
> > Hi,
> > 
> > Updated webrev since fdlibm build change seems to have settled:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210425/webrev.02/
> > 
> > Optimization is being done at level -O2 (CXX_O_FLAG_NORM) with
> > -ffp-contract=off when the toolchain (gcc/clang at this point)
> > supports it. Otherwise no optimization will be done. Those overrides
> > are no longer in an x86 specific block. Thus, ppc64, aarch64, s390x
> > will get correct settings too.
> > 
> > How does this look?
> > 
> > Thanks,
> > Severin
> > 
> > On Wed, 2018-09-12 at 07:16 +, Schmidt, Lutz wrote:
> > > I totally agree with Andrew's statement. FP calculations should be
> > > evaluated as the programmer wrote them down. All fiddling around with
> > > sequence or rounding is evil. You lose reproducibility of results.
> > > Regards,
> > > Lutz
> > > 
> > > On 08.09.18, 11:26, "hotspot-dev on behalf of Andrew Haley" 
> > >  wrote:
> > > 
> > >  On 09/06/2018 03:32 PM, Severin Gehwolf wrote:
> > >  > Right. I should note that ppc64, s390x and aarch64 ports don't have
> > >  > this optimization turned off as those overrides are in a x86 
> > > specific
> > >  > block. It appears it hasn't caused issues for these ports so far.
> > >  
> > >  That's just dumb luck. We really should turn off the merging of FP
> > >  operations on all platforms.
> > >  
> > >  --
> > >  Andrew Haley
> > >  Java Platform Lead Engineer
> > >  Red Hat UK Ltd. 
> > >  EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
> > >  
> > > 
> 
> 



RFR: JDK-8207264 solaris-sparcv9-cmp-baseline fails

2018-09-13 Thread Magnus Ihse Bursie
We regularly use the COMPARE_BUILD framework to test build 
reproducability, by comparing two builds made directly after one 
another. This test currently fails on solaris, since the compilation of 
libfontmanager and libsaproc is not stable (and thus can produce 
different disassembly each build).


This patch adds an exception for these libraries. (Note that one of them 
was already on the exception list, but only on slowdebug builds).


Ideally, we'd rather hunt down the problems with reproducability, but 
that's a lng term project.


Bug: https://bugs.openjdk.java.net/browse/JDK-8207264
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8207264-solaris-compare-build-fails/webrev.01


/Magnus



Re: RFR: JDK-8210704 Remove dead build tools hasher and jarreorder

2018-09-13 Thread Alan Bateman

On 13/09/2018 09:46, Magnus Ihse Bursie wrote:

On 2018-09-13 10:35, Magnus Ihse Bursie wrote:
The two build tools hasher and jarreorder is not used in the build 
anymore and should be removed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210704
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.01


Updated webrev that also removes the dead declaration for 
TOOL_GENMODULESXML. (The tool itself was previously removed)


http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.02 

The tool to generate modules.xml was a temporary tool during JDK 9 
development, we missed the ToolsJdk.gmk when removing. The changes in 
the webrev to this and the hasher and jarreorder tools looks fine.


-Alan


Re: RFR: JDK-8210704 Remove dead build tools hasher and jarreorder

2018-09-13 Thread Magnus Ihse Bursie

On 2018-09-13 10:35, Magnus Ihse Bursie wrote:
The two build tools hasher and jarreorder is not used in the build 
anymore and should be removed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210704
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.01


Updated webrev that also removes the dead declaration for 
TOOL_GENMODULESXML. (The tool itself was previously removed)


http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.02

/Magnus



RFR: 8210703: vmStructs.cpp compiled with -O0 (was: Why is vmStructs.cpp compiled with -O0?

2018-09-13 Thread Severin Gehwolf
Hi Erik,

On Wed, 2018-09-12 at 10:25 -0700, Erik Joelsson wrote:
> Hello,
> 
> I very much doubt it was included with the new build system. We were 
> extremely careful to use the exact same flags then, and did binary 
> comparisons of all object files to verify equal builds.
> 
> Tracing back, it was caused (probably unintentionally) by this change:
> 
> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/e796d52ca85b

Thanks for the detective work! This looks very much unintentional to me
as JDK-8114853 is about silencing a note.

> In the old build, setting OPT_CFLAGS/ overrides the common 
> OPT_CFLAGS. In the new build, we have a more general way of adding flags 
> for specific files that does not directly override any other flags. To 
> get the same behavior for vmStructs.cpp in the new build as in the old 
> at the time of the switch, we had to add -O0 explicitly in the new build.

I've filed JDK-8210703 and posting this patch as review for the bug.
Note I've changed the subject. Does this look OK?

diff --git a/make/hotspot/lib/JvmOverrideFiles.gmk 
b/make/hotspot/lib/JvmOverrideFiles.gmk
--- a/make/hotspot/lib/JvmOverrideFiles.gmk
+++ b/make/hotspot/lib/JvmOverrideFiles.gmk
@@ -30,7 +30,7 @@
 # status for individual files on specific platforms.
 
 ifeq ($(TOOLCHAIN_TYPE), gcc)
-  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments -O0
+  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments
   BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := -fno-var-tracking-assignments
   BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
-fno-var-tracking-assignments
   BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized

Thanks,
Severin

On 2018-09-12 09:13, Severin Gehwolf wrote:
> > Hi,
> > 
> > Does anybody know why vmStructs.cpp gets an override in
> > JvmOverrideFiles.gmk:
> > 
> > $ grep -C3 -n vmStructs.cpp make/hotspot/lib/JvmOverrideFiles.gmk
> > 30-# status for individual files on specific platforms.
> > 31-
> > 32-ifeq ($(TOOLCHAIN_TYPE), gcc)
> > 33:  BUILD_LIBJVM_vmStructs.cpp_CXXFLAGS := -fno-var-tracking-assignments 
> > -O0
> > 34-  BUILD_LIBJVM_jvmciCompilerToVM.cpp_CXXFLAGS := 
> > -fno-var-tracking-assignments
> > 35-  BUILD_LIBJVM_jvmciCompilerToVMInit.cpp_CXXFLAGS := 
> > -fno-var-tracking-assignments
> > 36-  BUILD_LIBJVM_assembler_x86.cpp_CXXFLAGS := -Wno-maybe-uninitialized
> > 
> > It seems to have been introduced with the new build system. JDK 8
> > doesn't seem to have it. Thoughts?
> > 
> > Thanks,
> > Severin
> > 
> 
> 



RFR: JDK-8210704 Remove dead build tools hasher and jarreorder

2018-09-13 Thread Magnus Ihse Bursie
The two build tools hasher and jarreorder is not used in the build 
anymore and should be removed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210704
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210704-remove-hasher-and-jarreorder/webrev.01


/Magnus



RFR: JDK-8210702 Remove dtrace mapfiles

2018-09-13 Thread Magnus Ihse Bursie
This is the last part of the mapfile cleanup. Two special libraries for 
supporting dtrace on solaris was not converted before.


As before, I have verified that there is no unwanted symbol changes in 
the resulting libraries.


(This fix also removes the libjsound mapfile once and for all -- the 
previous removal was lost due to hg merge issues.)


Bug: https://bugs.openjdk.java.net/browse/JDK-8210702
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210702-remove-last-mapfiles/webrev.01


/Magnus



Re: RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

2018-09-13 Thread Severin Gehwolf
On Thu, 2018-09-13 at 11:00 +1000, David Holmes wrote:
> Correction ...
> 
> On 13/09/2018 8:31 AM, David Holmes wrote:
> > On 12/09/2018 6:16 PM, Severin Gehwolf wrote:
> > > On Wed, 2018-09-12 at 17:58 +1000, David Holmes wrote:
> > > > But I don't understand why the optimization setting is being tied to the
> > > > availability of the -ffp-contract flag?
> > > 
> > > In configure we perform a check for gcc or clang whether that flag is
> > > supported. If it is, it would be non-empty exactly having -ffp-contract
> > > as value. It could be another set of flags for other arches if somebody
> > > wanted to do the same, fwiw. In JDK 8, for example, it's "-mno-fused-
> > > madd -fno-strict-aliasing" for ppc64:
> > > http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/2660b127b407/make/lib/CoreLibraries.gmk#l63
> > >  
> > > 
> > > 
> > > We need support for that flag (or a set of flags) when we optimize
> > > fdlibm since otherwise we would lose precision. If the flag is empty
> > > we'd not optimize as we can't guarantee precision. That's why we tie
> > > optimization to the availability of that flag. The expectation is for
> > > this flag to be available on gcc/clang arches only at this point. Does
> > > that make sense?
> > 
> > Yes that makes sense - thanks. I didn't quite glean that from the comment:
> > 
> >42 # If FDLIBM_CFLAGS is non-empty we know that we can optimize
> >43 # fdlibm by adding those extra C flags. Currently GCC,
> 
> I think this should say "when adding" not "by adding".

OK. Thanks, David. I'll change that before pushing.

Cheers,
Severin

> Thanks,
> David
> 
> >44 # and clang only.
> >45 ifneq ($(FDLIBM_CFLAGS), )
> >46   BUILD_LIBFDLIBM_OPTIMIZATION := LOW
> > 
> > But now I can read it and understand.
> > 
> > Thanks,
> > David
> > 
> > > Thanks,
> > > Severin
> > >