RE: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when linking libjvm

2021-01-18 Thread Doerr, Martin
Hi Christoph,

thanks for reviewing, testing and for the approval. Pushed.

Best regards,
Martin


From: Langer, Christoph 
Sent: Samstag, 16. Januar 2021 12:03
To: Doerr, Martin ; jdk-updates-...@openjdk.java.net; 
build-dev@openjdk.java.net
Cc: Lindenmaier, Goetz 
Subject: RE: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when 
linking libjvm

Hi Martin,

looks good. I also verified that it removes the warning messages in the build. 
Approved.

Best regards
Christoph

From: Doerr, Martin mailto:martin.do...@sap.com>>
Sent: Donnerstag, 14. Januar 2021 17:50
To: jdk-updates-...@openjdk.java.net<mailto:jdk-updates-...@openjdk.java.net>; 
build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>
Cc: Lindenmaier, Goetz 
mailto:goetz.lindenma...@sap.com>>; Langer, 
Christoph mailto:christoph.lan...@sap.com>>
Subject: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when linking 
libjvm

Hi,

JDK-8257633 is backported to 11.0.11-oracle. I'd like to backport it for parity.
Change doesn't apply cleanly because of an unrelated context change in the 
neighboring code.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8257633

Original change:
https://git.openjdk.java.net/jdk/commit/51d325e6

11u backport:
http://cr.openjdk.java.net/~mdoerr/8257633_build_11u/webrev.00/

Please review.

Best regards,
Martin



RE: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when linking libjvm

2021-01-15 Thread Doerr, Martin
Hi Götz,

thanks for the review.

Best regards,
Martin


From: Lindenmaier, Goetz 
Sent: Freitag, 15. Januar 2021 11:29
To: Doerr, Martin ; jdk-updates-...@openjdk.java.net; 
build-dev@openjdk.java.net
Cc: Langer, Christoph 
Subject: RE: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when 
linking libjvm

Hi Martin,

Trivial adaptions. Looks good.

Best regards
  Goetz.

From: Doerr, Martin mailto:martin.do...@sap.com>>
Sent: Thursday, January 14, 2021 5:50 PM
To: jdk-updates-...@openjdk.java.net<mailto:jdk-updates-...@openjdk.java.net>; 
build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>
Cc: Lindenmaier, Goetz 
mailto:goetz.lindenma...@sap.com>>; Langer, 
Christoph mailto:christoph.lan...@sap.com>>
Subject: [11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when linking 
libjvm

Hi,

JDK-8257633 is backported to 11.0.11-oracle. I'd like to backport it for parity.
Change doesn't apply cleanly because of an unrelated context change in the 
neighboring code.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8257633

Original change:
https://git.openjdk.java.net/jdk/commit/51d325e6

11u backport:
http://cr.openjdk.java.net/~mdoerr/8257633_build_11u/webrev.00/

Please review.

Best regards,
Martin



[11u] RFR: 8257633: Missing -mmacosx-version-min=X flag when linking libjvm

2021-01-14 Thread Doerr, Martin
Hi,

JDK-8257633 is backported to 11.0.11-oracle. I'd like to backport it for parity.
Change doesn't apply cleanly because of an unrelated context change in the 
neighboring code.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8257633

Original change:
https://git.openjdk.java.net/jdk/commit/51d325e6

11u backport:
http://cr.openjdk.java.net/~mdoerr/8257633_build_11u/webrev.00/

Please review.

Best regards,
Martin



RE: [11u] RFR: 8256810: Incremental rebuild broken on Macosx

2021-01-11 Thread Doerr, Martin
Hi Roland,

> Sure. Is a comment stating that this was backported what you have in mind?
Preferrably comment + link "backportet by" to your backport issue.

> Would you mind reviewing my RFR given you've already taken a look at
> this area?
I'll take a look.

Best regards,
Martin


> -Original Message-
> From: Roland Westrelin 
> Sent: Montag, 11. Januar 2021 17:40
> To: Doerr, Martin ; jdk-updates-
> d...@openjdk.java.net; build-dev@openjdk.java.net
> Cc: Lindenmaier, Goetz ; Langer, Christoph
> 
> Subject: RE: [11u] RFR: 8256810: Incremental rebuild broken on Macosx
> 
> 
> > thanks for letting me know. My plan was to backport both changes in the
> original order, but I'm fine if you do change to the new version at once.
> > May I ask you to flag both issues as backported once you're done?
> 
> Sure. Is a comment stating that this was backported what you have in mind?
> 
> Would you mind reviewing my RFR given you've already taken a look at
> this area?
> 
> Roland.



RE: [11u] RFR: 8256810: Incremental rebuild broken on Macosx

2021-01-11 Thread Doerr, Martin
Hi Roland,

thanks for letting me know. My plan was to backport both changes in the 
original order, but I'm fine if you do change to the new version at once.
May I ask you to flag both issues as backported once you're done?

Best regards,
Martin


> -Original Message-
> From: Roland Westrelin 
> Sent: Montag, 11. Januar 2021 17:16
> To: Doerr, Martin ; jdk-updates-
> d...@openjdk.java.net; build-dev@openjdk.java.net
> Cc: Lindenmaier, Goetz ; Langer, Christoph
> 
> Subject: Re: [11u] RFR: 8256810: Incremental rebuild broken on Macosx
> 
> 
> Hi Martin,
> 
> > JDK-8256810 is backported to 11.0.11-oracle. I'd like to backport it for 
> > parity.
> > Change doesn't apply cleanly because of an unlrelated context change
> (Solaris removal).
> 
> See my RFR for the backport of 8257547. It basically replaces all of
> that code.
> 
> Roland.



[11u] RFR: 8256810: Incremental rebuild broken on Macosx

2021-01-11 Thread Doerr, Martin
Hi,

JDK-8256810 is backported to 11.0.11-oracle. I'd like to backport it for parity.
Change doesn't apply cleanly because of an unlrelated context change (Solaris 
removal).

Bug:
https://bugs.openjdk.java.net/browse/JDK-8256810

Original change:
https://github.com/openjdk/jdk/commit/4c86e46d

11u backport:
http://cr.openjdk.java.net/~mdoerr/8256810_build_11u/webrev.00/

Please review.

Best regards,
Martin



RE: PING: RFR: 8250598: Hyper-V is detected in spite of running on host OS

2020-08-18 Thread Doerr, Martin
Hi Yasumasa,

thanks a lot for measuring and improving the overhead. This version looks much 
better.

I still wonder what the reason was for using a stub. There was probably a 
reason, but I don't remember.

Hopefully, Matthias can find time for another review when he's back next week.

Thanks and best regards,
Martin


> -Original Message-
> From: Yasumasa Suenaga 
> Sent: Dienstag, 18. August 2020 05:03
> To: David Holmes ; Baesken, Matthias
> ; hotspot-runtime-...@openjdk.java.net;
> build-dev@openjdk.java.net; Doerr, Martin 
> Subject: Re: PING: RFR: 8250598: Hyper-V is detected in spite of running on
> host OS
> 
> On 2020/08/18 10:42, David Holmes wrote:
> > Hi Yasumasa,
> >
> > On 14/08/2020 12:36 pm, Yasumasa Suenaga wrote:
> >> Hi David, Matthias, Martin,
> >>
> >> I uploaded new webrev. Could you review again?
> >>
> >>    http://cr.openjdk.java.net/~ysuenaga/JDK-8250598/webrev.02/
> >>
> >> It generates runtime stub for hypervisor detection, and it can distinguish
> between Hyper-V host (root partition) and guest. And also it works on 32bit
> platforms.
> >>
> >> I measured startup performance (java --version) with Measure-Command
> on PowerShell, this change is faster than current implementation.
> >>
> >> TotalMilliseconds : 57.8196
> >> TotalMilliseconds : 66.8379
> >> TotalMilliseconds : 64.7693
> >> TotalMilliseconds : 55.6546
> >> TotalMilliseconds : 63.848
> >> average : 61.78588
> >
> > This seems good to me, but again I can't review the details. My main
> concern is how you are testing this across a range of virtualized hosts? But 
> if it
> fixes your Hyper-V issue and doesn't break anything else then I am fine with
> it.
> 
> Thanks David!
> 
> I don't have any other virtualized hosts excepting Hyper-V, so I cannot test
> any more.
> However detection logic is equivalent from current implementation, so I
> believe it does not break anything else if it works fine on Hyper-V.
> 
> And also Matthias has queued this change to internal tests in SAP. I guess he
> will share the result next week.
> 
> 
> > I will leave the technical review to Martin and Matthias.
> >
> > FYI I'm heading out on a weeks vacation after today so won't be able to
> follow up further.
> 
> Have a nice vacation!
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> > Thanks,
> > David
> >
> >> This change has passed tests on submit repo (mach5-one-ysuenaga-JDK-
> 8250598-20200814-0119-13424118), and also it works fine on following
> environments:
> >>
> >>    - Hyper-V host (Windows 10 x64)
> >>    - Hyper-V guest (Windows 10 x64)
> >>    - Hyper-V guest (Linux x64)
> >>    - Hyper-V guest (32bit JDK on Linux x64)
> >>
> >>
> >> Thanks,
> >>
> >> Yasumasa
> >>
> >>
> >> On 2020/08/13 20:39, David Holmes wrote:
> >>> On 13/08/2020 5:39 pm, Yasumasa Suenaga wrote:
> >>>> Hi Matthias, David,
> >>>>
> >>>> On 2020/08/13 15:52, Baesken, Matthias wrote:
> >>>>>
> >>>>>>
> >>>>>> Should we make the change to determine just before it is needed
> (e.g. VM.info or hs_err log) at first?
> >>>>>> It is out of scope of this change, so I want to work as another issue 
> >>>>>> if
> it is needed.
> >>>>>>
> >>>>>
> >>>>> Hi ,
> >>>>> I think we better do not call into WMI , when  writing an hs_err file   
> >>>>> .
> >>>>
> >>>> I found out from Hypervisor Top Level Functional Specification [1]
> section 2.4.8.
> >>>> That says CPUID leaf 0x4007 is available to the root partition only.
> >>>>
> >>>> I changed VM_Version::is_in_VM() as below, and it works fine.
> >>>> (I use __cpuid intrinsic in here because this code would be compiled by
> cl.exe only.)
> >>>>
> >>>> ```
> >>>> #include 
> >>>>
> >>>> #define EAX 0
> >>>> #define EBX 1
> >>>> #define ECX 2
> >>>> #define EDX 3
> >>>>
> >>>> bool VM_Version::is_in_VM() {
> >>>>    int regs[4];
> >>>>    __cpuid(regs, 0x4007);
> >>>>
> >>>>    // CPUID leaf 0x4007 is available to the root partition only.
> >>>>    return (regs[EAX] == 0x0) && (r

Re: RFR [XS]: 8248334: hs build errors on ppc64 and s390x platforms

2020-06-26 Thread Doerr, Martin
Hi Matthias,

Looks good. Thanks for fixing it.

Best regards,
Martin

> Am 26.06.2020 um 09:59 schrieb David Holmes :
> 
> Hi Matthias,
> 
> That all seems fine to me.
> 
> David
> 
>> On 26/06/2020 5:06 pm, Baesken, Matthias wrote:
>> Hello, please review this small  patch  that fixes the  ppc64(le) and s390x  
>> builds .
>> ( They started to break since the 24th June,  looks like some inclusions  
>> were   missing after recent  HS changes )
>> Bug/webrev :
>> https://bugs.openjdk.java.net/browse/JDK-8248334
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8248334.0/
>> Thanks, Matthias


RE: RFR: 8246032: Implementation of JEP 347: Adopt C++14 Language Features in HotSpot

2020-06-05 Thread Doerr, Martin
Hi Kim,

builds out of the box on AIX with IBM XL C/C++ for AIX, V16.1.0 (We already use 
clang frontend by default.)
Very cool!

Best regards,
Martin


> -Original Message-
> From: build-dev  On Behalf Of Kim
> Barrett
> Sent: Freitag, 5. Juni 2020 09:53
> To: build-dev 
> Cc: 2d-...@openjdk.java.net; hotspot-dev developers  d...@openjdk.java.net>; Java Core Libs 
> Subject: RFR: 8246032: Implementation of JEP 347: Adopt C++14 Language
> Features in HotSpot
> 
> [Changes are only to the build system, but since the changes have jdk-wide
> effect I’ve cc’d what I think are the relevant dev lists.]
> 
> This change is part of JEP 347; the intent is to target JDK 16.
> 
> Please review this change to the building of C++ code in the JDK to
> enable the use of C++14 language features.  This change does not make
> any code changes to use new features provided by C++11 or C++14.
> 
> This requires trimming the supported compiler versions, moving the
> minimum supported versions forward (significantly, in some cases).
> The new minimums are based on compiler documentation rather than
> testing.  It may be that more recent versions are actually required.
> 
> This change needs to be tested on platforms not supported by Oracle.
> The JEP test plan includes additional Oracle testing beyond what I’ve done.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8246032
> 
> Webrev:
> https://cr.openjdk.java.net/~kbarrett/8246032/open.02/
> 
> Testing:
> mach5 tier1-5 on Oracle supported platforms.
> 
> Performance testing showed no significant changes in either direction.
> 
> Build-only (no tests) for linux-arm32, linux-s390x, linux-ppc64le



RE: RFR(XS): 8244756: Build broken with some awk version after JDK-8244248

2020-05-12 Thread Doerr, Martin
Thanks for the reviews! Pushed.

Best regards,
Martin


> -Original Message-
> From: Baesken, Matthias 
> Sent: Dienstag, 12. Mai 2020 11:01
> To: build-dev@openjdk.java.net
> Cc: Doerr, Martin 
> Subject: RE: RFR(XS): 8244756: Build broken with some awk version after JDK-
> 8244248
> 
> 
> Reviewed !
> 
> Best regards, Matthias
> 
> >thank you for proposing a fix which allows building with old awk on AIX.
> Works fine.
> >
> >Here's the webrev:
> >http://cr.openjdk.java.net/~mdoerr/8244756_AIX_fix_awk_expr/webrev.0
> 0/
> >
> 
> 
> 



RFR(XS): 8244756: Build broken with some awk version after JDK-8244248

2020-05-11 Thread Doerr, Martin
Hi Magnus,

thank you for proposing a fix which allows building with old awk on AIX. Works 
fine.

Here's the webrev:
http://cr.openjdk.java.net/~mdoerr/8244756_AIX_fix_awk_expr/webrev.00/

Would you like to be mentioned as author?

Best regards,
Martin



RE: RFR [XXS]: 8237382: Cleanup the OPT_SPEED_SRC file list in JvmFeatures.gmk

2020-01-20 Thread Doerr, Martin
Hi Matthias,

thanks for removing no longer existing files from the list.
I guess the list will need further updates to become really useful, but your 
change looks good.

Best regards,
Martin


> -Original Message-
> From: hotspot-dev  On Behalf Of
> Erik Joelsson
> Sent: Freitag, 17. Januar 2020 15:09
> To: Baesken, Matthias ; 'build-
> d...@openjdk.java.net' ; 'hotspot-
> d...@openjdk.java.net' 
> Subject: Re: RFR [XXS]: 8237382: Cleanup the OPT_SPEED_SRC file list in
> JvmFeatures.gmk
> 
> Looks good.
> 
> /Erik
> 
> On 2020-01-16 22:47, Baesken, Matthias wrote:
> > Hello, please review this very small change .
> >
> > It removes file that are not present any more from the  OPT_SPEED_SRC
> file list in JvmFeatures.gmk  .
> >
> > ( this is a list of files to be optimized for speed when we otherwise 
> > optimize
> for size  in the minimal-JVM build)
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8237382
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8237382.0/
> >
> > Thanks, Matthias
> >


RE: [11u] RFR: 8236500: Windows ucrt.dll should be looked up in versioned WINSDK subdirectory

2019-12-23 Thread Doerr, Martin
Hi Christoph,

looks good. Thanks for fixing this part in 11u.

Best regards,
Martin


> -Original Message-
> From: build-dev  On Behalf Of
> Langer, Christoph
> Sent: Montag, 23. Dezember 2019 15:30
> To: jdk-updates-dev 
> Cc: build-dev 
> Subject: [11u] RFR: 8236500: Windows ucrt.dll should be looked
> up in versioned WINSDK subdirectory
> 
> Hi,
> 
> please review a backport of a carveout of JDK-8215445: "Enable building for
> Windows in WSL" [0]. When building 11u after I've reinstalled Visual Studio
> and the Windows Devkit to non-default folder locations, I ran into the issue
> that ucrt.dll would not be correctly resolved, since it sits in a versioned
> subdirectory of ...\WindowsKits, e.g.
> ...\WindowsKits\10\Lib\10.0.17763.0\ucrt\. This problem was fixed as part of
> JDK-8215445. However, I don't want to backport the whole change, at least
> not at the moment, so I created a new bug to fix this specific issue in
> OpenJDK 11 Updates. Please review.
> 
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8236500.11u/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8236500
> 
> Thanks
> Christoph
> 
> [0] https://bugs.openjdk.java.net/browse/JDK-8215445



RE: [11u] RFR: 8232167: Visual Studio install found through --with-tools-dir value is discarded

2019-12-23 Thread Doerr, Martin
Hi Christoph,

looks good. Thanks for backporting.

Best regards,
Martin


> -Original Message-
> From: jdk-updates-dev  On
> Behalf Of Langer, Christoph
> Sent: Montag, 23. Dezember 2019 15:00
> To: jdk-updates-dev 
> Cc: build-dev 
> Subject: [11u] RFR: 8232167: Visual Studio install found through --
> with-tools-dir value is discarded
> 
> Hi,
> 
> please review this simple backport of a simple build system fix which
> unfortunately did not apply cleanly. It's just context changes that needed to
> be resolved. I ran into this with jdk11u after I reinstalled my Visual Studio 
> to a
> non-default folder and used configure option --with-tools-dir. So it should be
> backported.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232167
> Original change: https://hg.openjdk.java.net/jdk/jdk/rev/5a4b4544b810
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8232167.11u/
> 
> Thanks
> Christoph



RE: building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code

2019-11-28 Thread Doerr, Martin
Hi Claes,

yeah, that makes sense.
 
> Hopefully we don't have to do one RFE per file.. :-)

I should have written set of files or directories or whatever.

Thanks for your input.

Best regards,
Martin


> -Original Message-
> From: Claes Redestad 
> Sent: Mittwoch, 27. November 2019 23:35
> To: Doerr, Martin ; Baesken, Matthias
> ; Erik Joelsson ;
> 'build-dev@openjdk.java.net' ; 'hotspot-
> d...@openjdk.java.net' 
> Subject: Re: building libjvm with -Os for space optimization - was : RE: RFR:
> 8234525: enable link-time section-gc for linux s390x to remove unused code
> 
> Hi Martin,
> 
> On 2019-11-27 19:03, Doerr, Martin wrote:
> > Hi Claes,
> >
> > that kind of surprises me. I'd expect files which rather benefit from -O3 to
> be far less than those which benefit from -Os.
> > Most performance critical code lives inside the code cache and is not
> dependent on C++ compiler optimizations.
> > I'd expect GC code, C2's register allocation and a few runtime files to be 
> > the
> most performance critical C++ code.
> > So the list of files for -Os may become long.
> 
> that might very well be the end result, and once/if we've gone down this
> path long enough to see that -O3 becomes the exception, we can re-
> examine the default. Changing the default and then trying to recuperate
> would be hard/impossible to do incrementally.
> 
> >
> > Yeah, I think we should use native profiling information to find out what's
> really going on >
> > Your idea to change file by file and check for performance regression
> makes sense to me, though
> Hopefully we don't have to do one RFE per file.. :-)
> 
> /Claes


RE: building libjvm with -Os for space optimization - was : RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code

2019-11-27 Thread Doerr, Martin
Hi Claes,

that kind of surprises me. I'd expect files which rather benefit from -O3 to be 
far less than those which benefit from -Os.
Most performance critical code lives inside the code cache and is not dependent 
on C++ compiler optimizations.
I'd expect GC code, C2's register allocation and a few runtime files to be the 
most performance critical C++ code.
So the list of files for -Os may become long.

Yeah, I think we should use native profiling information to find out what's 
really going on.

Your idea to change file by file and check for performance regression makes 
sense to me, though.

Best regards,
Martin


> -Original Message-
> From: Claes Redestad 
> Sent: Mittwoch, 27. November 2019 18:57
> To: Baesken, Matthias ; Doerr, Martin
> ; Erik Joelsson ; 'build-
> d...@openjdk.java.net' ; 'hotspot-
> d...@openjdk.java.net' 
> Subject: Re: building libjvm with -Os for space optimization - was : RE: RFR:
> 8234525: enable link-time section-gc for linux s390x to remove unused code
> 
> Hi,
> 
> we discussed doing the opposite for Mac OS X recently, where builds are
> currently set to -Os by default. -O3 helped various networking
> (micro)benchmarks by up to 20%.
> 
> Rather than doing -Os by default and then cherry-pick things over to -O3
> on a case-by-case basis, I'd suggest the opposite: keep -O3 as the
> default, start evaluating -Os on a case-by-case basis. This allows for
> an incremental approach where we identify things that are definitely not
> performance critical, e.g., never shows up in profiles, and switch those
> compilation units over to -Os. Check for harmful performance impact and
> expected footprint improvement; rinse; repeat.
> 
> $.02
> 
> /Claes
> 
> 
> On 2019-11-27 17:36, Baesken, Matthias wrote:
> > Hello Martin,  I checked  building  libjvm.so  with -Os  (instead of -O3) .
> >
> > I used gcc-7  on linux x86_64 .
> > The size  of  libjvm.so   dropped   from24M  (normal night make with 
> > -O3)
> to   18M   ( test make with -Os)   .
> >   (adding the link-time gc might  reduce the size by another ~ 10 % ,  but
> those 2 builds were without the ltgc )
> >
> > Cannot say much so far about performance impact .
> >
> > Best regards, Matthias
> >
> >
> >
> >>
> >> Hi Matthias and Erik,
> >>
> >> I also think this is an interesting option.
> >>
> >> I like the idea to generate smaller libraries. In addition to that, I 
> >> could also
> >> imagine building with -Os (size optimized) by default and only select -O3
> for
> >> performance critical files (e.g. C2's register allocation, some gc code, 
> >> ...).
> >>
> >> If we want to go into such a direction for all linux platforms and want to
> use
> >> this s390 only change as some kind of pipe cleaner, I think this change is
> fine
> >> and can get pushed.
> >> Otherwise, I think building s390 differently and not intending to do the
> same
> >> for other linux platforms would be not so good.
> >>
> >> We should only make sure the exported symbols are set up properly to
> avoid
> >> that this optimization throws out too much.
> >>
> >> My 50 Cents.
> >>
> >> Best regards,
> >> Martin
> >>
> >


RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code

2019-11-26 Thread Doerr, Martin
Hi Matthias and Erik,

I also think this is an interesting option.

I like the idea to generate smaller libraries. In addition to that, I could 
also imagine building with -Os (size optimized) by default and only select -O3 
for performance critical files (e.g. C2's register allocation, some gc code, 
...).

If we want to go into such a direction for all linux platforms and want to use 
this s390 only change as some kind of pipe cleaner, I think this change is fine 
and can get pushed.
Otherwise, I think building s390 differently and not intending to do the same 
for other linux platforms would be not so good.

We should only make sure the exported symbols are set up properly to avoid that 
this optimization throws out too much.

My 50 Cents.

Best regards,
Martin



> -Original Message-
> From: build-dev  On Behalf Of
> Baesken, Matthias
> Sent: Freitag, 22. November 2019 16:01
> To: Erik Joelsson ; 'build-dev@openjdk.java.net'
> ; core-libs-...@openjdk.java.net
> Subject: RE: RFR: 8234525: enable link-time section-gc for linux
> s390x to remove unused code
> 
> Hi Erik,
>yes it makes the build more chatty -
> our   linux s390   product   build  log  of  jdk/jdk   goes from  ~ 60.000  
> lines to  ~
> 123.000  lines with the patch.
> 
> However the change  is  linux s390 only so I guess it will not disturb other
> people ( in case we bring it to more platforms later on, we could remove the
> printing or make it configurable ).
> 
> Additionally the "chatty" output  is used currently for eliminating   unused
> functions + datacross-platform
> (see for example   https://bugs.openjdk.java.net/browse/JDK-8234629  ).
> 
> Best regards, Matthias
> 
> 
> 
> >
> > Hello Matthias,
> >
> > This looks like an interesting change. If you want to enable this for
> > s390x, I'm ok with it. If it works out well, perhaps we can find a way
> > to expand this to other architectures.
> >
> > Do you really want to set --print-gc-sections by default? I would assume
> > that makes the build quite chatty.
> >
> > /Erik
> >
> > On 2019-11-21 00:54, Baesken, Matthias wrote:
> > > Hello,
> > >
> > > gcc and ld can be instructed to work together to "garbage collect" unused
> > input sections.
> > > This feature eliminates unused code from native libraries. As a
> prerequisite
> > to take full advantage of the feature,
> > > the source files need to be compiled with "-ffunction-sections -fdata-
> > sections".
> > >
> > > Details on what happens can be found in the ld documentation:
> > https://linux.die.net/man/1/ld .
> > > See the description of --gc-sections and --print-gc-sections therein.
> > > For more detailed insights there is a talk available from ELC2010:
> > https://elinux.org/images/2/2d/ELC2010-gc-sections_Denys_Vlasenko.pdf
> > >
> > > My change enables the unused code elimination on linux s390x .
> > > (on the other Linux platforms, there are still issues to be solved with 
> > > the
> > serviceability agent, but we do not have the serviceability agent  on linux
> > s390x).
> > >
> > > The change has 2 benefits :
> > > - native libs with unused code get smaller (some get alot smaller)
> > > some example lib sizes  from  linuxs390x (product build) :
> > >  default settings  / link-time gc-sections
> > > libmlib_image.so556K 536K
> > > libjavajpeg.so  300K 292K
> > > libsplashscreen.so  412K 268K
> > > libfontmanager.so   1.4M 864K
> > > libjvm.so19M  17M
> > >
> > > - the flag --print-gc-sections  outputs the removed sections when calling
> > the linker;
> > > this helps a lot to find coding "waiting for" cross-platform removal.
> > >
> > >
> > > Here is an example output of --print-gc-sections for the libnet-build 
> > > (linux
> > s390x) :
> > >
> > > /bin/ld: Removing unused section '.bss.my_gconf_init_func' in file
> > '/builddir/support/native/java.base/libnet/DefaultProxySelector.o'  <---
> > seems to be dead
> > > /bin/ld: Removing unused section '.text.NET_ReadV' in file
> > '/builddir/support/native/java.base/libnet/linux_close.o'   
> > <---
> seems
> > to be dead, I requested cross-platform removal with
> > https://bugs.openjdk.java.net/browse/JDK-8234501
> > > /bin/ld: Removing unused section '.text.getInet6Address_scopeifname'
> in
> > file '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to
> be
> > dead
> > > /bin/ld: Removing unused section '.text.getInet6Address_scopeid_set' in
> > file '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to
> be
> > dead
> > > /bin/ld: Removing unused section '.text.getInetAddress_hostName' in
> file
> > '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to 
> > be
> > dead
> > > /bin/ld: Removing unused section '.text.setDefaultScopeID' in file
> > '/builddir/support/native/java.base/libnet/net_util_md.o'   <--- 
> > seems

RE: RFR [XS] [jdk11] : 8233203: fix non-product build on AIX when compiling with xlc16/legacy-xlc

2019-10-30 Thread Doerr, Martin
Hi Matthias,

thanks for fixing xlc16 support for jdk11u. I appreciate it.
Fix looks good to me.

Best regards,
Martin


From: Baesken, Matthias 
Sent: Mittwoch, 30. Oktober 2019 15:38
To: jdk-updates-...@openjdk.java.net; 'build-dev@openjdk.java.net' 

Cc: Langer, Christoph ; Doerr, Martin 

Subject: RFR [XS] [jdk11] : 8233203: fix non-product build on AIX when 
compiling with xlc16/legacy-xlc

Hello, please review the following small  AIX related fix.
It is a JDK11 only change ,  and  fixes issues  when building JDK11  with 
xlc16/legacy xlc .
Currently the product build of jdk11   already works with xlc16 ; however  
without this change ,  the (fast)debug build  still fails
  because  we call into  operator new   ( operator_new.cpp) from   
Iostream_init::Iostream_init()()   and this  leads to  a failing build .

( The described  issue   was not seen  when  using  the old  xlc12  for the 
build . )


Bug/webrev :

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

http://cr.openjdk.java.net/~mbaesken/webrevs/8233203.0/

Thanks, Matthias


RE: RFR: 8233078 : fix minimal VM build on Linux ppc64(le)

2019-10-29 Thread Doerr, Martin
Hi Matthias,

> Not sure if there are any plans to  support  OptimizeFill  on ppc64 ?
This question is not related to this issue.
Commenting out parts of it is not a good style.

Thank you for your update. The new webrev looks good to me.

Best regards,
Martin


From: Baesken, Matthias 
Sent: Dienstag, 29. Oktober 2019 13:25
To: Doerr, Martin ; 'hotspot-...@openjdk.java.net' 

Cc: 'build-dev@openjdk.java.net' 
Subject: RE: RFR: 8233078 : fix minimal VM build on Linux ppc64(le)

Hi Martin,  thanks for the input .
I  did the adjustments you suggested;  new webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.1/

Regarding  : stubGenerator_ppc.cpp: "Code should better be protected by #ifdef 
COMPILER2 than commenting out."
Currently  the  if (OptimizeFill) { ... }coding  is dead on ppc  .

See  :
c2_globals.hpp

234 /* OptimizeFill not yet supported on PowerPC. */ \
235 product(bool, OptimizeFill, true PPC64_ONLY(&& false), \

c2_init_ppc.cpp

53 if (OptimizeFill) {
54   warning("OptimizeFill is not supported on this CPU.");
55   FLAG_SET_DEFAULT(OptimizeFill, false);


Not sure if there are any plans to  support  OptimizeFill  on ppc64 ?

Best regards, Matthias




Hi Matthias,

thanks for fixing it. I have a few requests:

disassembler_ppc.cpp:
Please remove includes completely if no longer needed (instead of commenting 
out).

sharedRuntime_ppc.cpp:
I think it's better to remove the 2 align(InteriorEntryAlignment). Succeeding 
code is not performance critical.

stubGenerator_ppc.cpp:
Code should better be protected by #ifdef COMPILER2 than commenting out.

Otherwise, looks good to me.

Thanks,
Martin


From: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>
Sent: Dienstag, 29. Oktober 2019 12:42
To: 'hotspot-...@openjdk.java.net' 
mailto:hotspot-...@openjdk.java.net>>
Cc: 'build-dev@openjdk.java.net' 
mailto:build-dev@openjdk.java.net>>; Doerr, Martin 
mailto:martin.do...@sap.com>>
Subject: RFR: 8233078 : fix minimal VM build on Linux ppc64(le)

Hello, please review the following fix .
I recently  experimented a bit with the  minimal  VM  build  on  linux x86_64   
 (--with-jvm-features=minimal --with-jvm-variants=minimal) .
This worked fine .

However  when I tried  the minimal vm build   on linux  ppc64 / ppc64le ,   I 
noticed that it fails  because of a few  wrong dependencies .
Thanks to Martin for the advice regarding

Register ic = as_Register(Matcher::inline_cache_reg_encode());

Replacement with


Register ic = R19_inline_cache_reg;

In 
http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp.frames.html


Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8233078
http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/


Thanks, Matthias





RE: RFR: 8233078 : fix minimal VM build on Linux ppc64(le)

2019-10-29 Thread Doerr, Martin
Hi Matthias,

thanks for fixing it. I have a few requests:

disassembler_ppc.cpp:
Please remove includes completely if no longer needed (instead of commenting 
out).

sharedRuntime_ppc.cpp:
I think it's better to remove the 2 align(InteriorEntryAlignment). Succeeding 
code is not performance critical.

stubGenerator_ppc.cpp:
Code should better be protected by #ifdef COMPILER2 than commenting out.

Otherwise, looks good to me.

Thanks,
Martin


From: Baesken, Matthias 
Sent: Dienstag, 29. Oktober 2019 12:42
To: 'hotspot-...@openjdk.java.net' 
Cc: 'build-dev@openjdk.java.net' ; Doerr, Martin 

Subject: RFR: 8233078 : fix minimal VM build on Linux ppc64(le)

Hello, please review the following fix .
I recently  experimented a bit with the  minimal  VM  build  on  linux x86_64   
 (--with-jvm-features=minimal --with-jvm-variants=minimal) .
This worked fine .

However  when I tried  the minimal vm build   on linux  ppc64 / ppc64le ,   I 
noticed that it fails  because of a few  wrong dependencies .
Thanks to Martin for the advice regarding

Register ic = as_Register(Matcher::inline_cache_reg_encode());

Replacement with


Register ic = R19_inline_cache_reg;

In 
http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp.frames.html


Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8233078
http://cr.openjdk.java.net/~mbaesken/webrevs/8233078.0/


Thanks, Matthias





RE: RFR: 8228426: xlc: switch to clang-style warning disabling

2019-07-22 Thread Doerr, Martin
Hi Matthias,

looks good to me. I think this makes sense for jdk14 where we only support 
xlclang++ on AIX.

Best regards,
Martin


> -Original Message-
> From: build-dev  On Behalf Of
> Baesken, Matthias
> Sent: Montag, 22. Juli 2019 12:03
> To: Baesken, Matthias ; 'build-
> d...@openjdk.java.net' ; 'ppc-aix-port-
> d...@openjdk.java.net' 
> Subject: RE: RFR: 8228426: xlc: switch to clang-style warning
> disabling
> 
> Hello, any comments please ?
> 
> From: ppc-aix-port-dev  On
> Behalf Of Baesken, Matthias
> Sent: Freitag, 19. Juli 2019 12:20
> To: 'build-dev@openjdk.java.net' ; 'ppc-aix-
> port-...@openjdk.java.net' 
> Subject: [CAUTION] RFR: 8228426: xlc: switch to clang-style warning disabling
> 
> Please review the following change that switches to  clang-style warning
> disabling  on AIX,  and  adjusts the warning  disabling to current needs .
> 
> Recently the jdk/jdk build on AIX switched to xlc16/xlclang.
> 
> This means we can now use the standard clang-style warning disabling (the
> old legacy qsuppress warning disablings from legacy xlc
> might not fully work any more).
> 
> I think we are pretty close in reducing the compiler warnings to 0 on AIX,
> which would make it possible to build with warnings as errors at some point
> in the future .
> 
> In a first step I would disable these warnings in HS for xlc16/xlclang :
> 
> tautological-compare : (seen in os_aix.cpp)
> 
> /jdk/src/hotspot/os/aix/os_aix.cpp:2400:15: warning: comparison of
> unsigned expression >= 0 is always true [-Wtautological-compare]
> if (bytes >= Use64KPagesThreshold) {
> ~ ^  
> /jdk/src/hotspot/os/aix/os_aix.cpp:2607:15: warning: comparison of
> unsigned expression >= 0 is always true [-Wtautological-compare]
> if (bytes >= Use64KPagesThreshold) {
> 
> In the product  build   the   "Use64KPagesThreshold"   is a constant  so clang
> complains .  However  in the  (fast)debug  builds one can set
> Use64KPagesThreshold  with an -XX setting.
> So I think it is best to disable the warning.
> 
> 
> shift-negative-value : (seen in c1_LIRGenerator_ppc.cpp)
> 
> /jdk/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp:429:97: warning:
> shifting a negative signed value is undefined [-Wshift-negative-value]
>   (x->op() == Bytecodes::_lsub && right.value()->type()-
> >as_LongConstant()->value() == ((-1)<<15)) ) {
>   
>   ^
> /jdk/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp:483:96: warning:
> shifting a negative signed value is undefined [-Wshift-negative-value]
>   (x->op() == Bytecodes::_isub && right.value()->type()->as_IntConstant()-
> >value() == ((-1)<<15)) ) {
> 
> We could probably replace  the -1 shift by a constant but I think it  is 
> nicely
> readable .
> 
> 
> 
> Bug/webrev :
> 
> https://bugs.openjdk.java.net/browse/JDK-8228426
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8228426.0/
> 
> 
> Thanks, Matthias



RE: 8227389: Remove unsupported xlc16 compile options on aix - was : RE: AIX xlc16 options langlvl=c99vla / langlvl=noredefmac is not supported

2019-07-08 Thread Doerr, Martin
+1

Thanks,
Martin


> -Original Message-
> From: build-dev  On Behalf Of
> Langer, Christoph
> Sent: Montag, 8. Juli 2019 15:26
> To: Baesken, Matthias ; Thomas Stüfe
> 
> Cc: build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
> Subject: [CAUTION] RE: 8227389: Remove unsupported xlc16 compile options
> on aix - was : RE: AIX xlc16 options langlvl=c99vla / langlvl=noredefmac is 
> not
> supported
> 
> Hi Matthias,
> 
> looks good!
> 
> Best regards
> Christoph
> 
> > -Original Message-
> > From: build-dev  On Behalf Of
> > Baesken, Matthias
> > Sent: Montag, 8. Juli 2019 14:00
> > To: Thomas Stüfe 
> > Cc: build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
> > Subject: [CAUTION] 8227389: Remove unsupported xlc16 compile options
> on
> > aix - was : RE: AIX xlc16 options langlvl=c99vla / langlvl=noredefmac is not
> > supported
> >
> > Hello,  here is a small webrev  removing  the  2 mentioned options .
> >
> > When compiling jdk/jdk on AIX (using current xlc16 / xlclang) we run into
> > these warnings :
> >
> > warning: 1540-5200 The option "langlvl=c99vla" is not supported. :
> > This option controls variable length arrays / Enables or disables support 
> > for
> > C99-type variable length arrays.
> > warning: 1540-5200 The option "langlvl=noredefmac" is not supported. :
> > Controls whether a macro can be redefined without a prior #undef or
> > undefine() statement.
> >
> > see
> >
> https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.
> > compilers.aix.doc/compiler.pdf
> >
> >
> > The mentioned options are only available with the legacy xlc (non xlclang).
> > I propose to remove these options, because xlc16 / xlclang is now the
> default
> > compiler.
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8227389
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8227389.0/
> >
> >
> >
> > Thanks and best regards, Matthias
> >
> >
> >
> > I think this is fine for 14.
> >
> > Cheers, Thomas
> >
> > On Mon, Jul 8, 2019 at 9:23 AM Baesken, Matthias
> > mailto:matthias.baes...@sap.com>>
> wrote:
> > Hello,   when building   jdk/jdk  on AIX  (using the current default xlc16) 
> >   I
> get
> > a ton of warnings  :
> >
> > warning: 1540-5200 The option "langlvl=c99vla" is not supported.
> > warning: 1540-5200 The option "langlvl=noredefmac" is not supported.
> >
> >
> > The 2  options seem  to be gone with the current xlc16  / xlclang++  .   I
> would
> > like to remove them -  any objections ?
> > (removing them might be bad for older xlc  versions but I think we
> probably
> > cannot compile with older versions  any more  ) .
> >
> >
> > Best regards, Matthias


RE: FW: RFR: 8223307: enable the Stack Execution Disable flag for JDK binaries on AIX - was : AIX : -bnorwexec linker flag

2019-05-09 Thread Doerr, Martin
Hi Matthias,

the change looks good to me.
According to an old redbook [1], this flag makes stacks and r/w sections of the 
lib non-executable. This makes sense.

Best regards,
Martin

[1] AIX 5L Differences Guide Version 5.3 Edition



-Original Message-
From: Erik Joelsson mailto:erik.joels...@oracle.com>>
Sent: Freitag, 3. Mai 2019 15:32
To: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>; 
ppc-aix-port-...@openjdk.java.net; 
'build-dev@openjdk.java.net' 
mailto:build-dev@openjdk.java.net>>
Subject: Re: RFR: 8223307: enable the Stack Execution Disable flag for JDK 
binaries on AIX - was : AIX : -bnorwexec linker flag

Change looks good from a build perspective. I can't comment on the
validity of the flag itself for a platform I don't know.

/Erik

On 2019-05-03 05:48, Baesken, Matthias wrote:
> Hello,  I created a webrev for the previously discussed issue (AIX : 
> -bnorwexec linker flag).
>
> AIX supports a binary hardening option called SED, see :
>
> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.security/stack_exec_disable.htm
>
> System wide configuration can be done with  the sedmgr tool :
>
> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.cmds5/sedmgr.htm
>
>
> The system-wide configuration supports various modes, very often  "select" 
> mode is configured where
> binaries can opt-in  to  use  SED :
>
> "select:
> Only a select set of files are enabled and monitored for SED protection. The 
> select set of files are chosen by reviewing the SED related flags in the 
> executable program binary headers.
> The executable program header enables SED related flags to request to be 
> included in the select mode."
>
>
> We can set a linker option on AIX to configure "select" mode for the JDK 
> binaries; our tests show that this does not "kill" the JIT (jitted code).
>
> changed binary shows then the opt-in flag ("request" ) :
>
> bash-4.4$ sedmgr -d  /rs6000_64/nightly/output-jdk-test/images/jdk/bin/java
> /rs6000_64/nightly/output-jdk-test/images/jdk/bin/java : request
>
> while the unchanged binary does not have the flag set ("system") :
>
> bash-4.4$ sedmgr -d /rs6000_64/nightly/output-jdk11-test/images/jdk/bin/java
> /rs6000_64/nightly/output-jdk11-test/images/jdk/bin/java : system
>
>
>
> Bug/webrev :
>
> https://bugs.openjdk.java.net/browse/JDK-8223307
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8223307.0/
>
>
> Thanks, Matthias
>
>
>
>> -Original Message-
>> From: Erik Joelsson 
>> mailto:erik.joels...@oracle.com>>
>> Sent: Freitag, 12. April 2019 15:30
>> To: Baesken, Matthias 
>> mailto:matthias.baes...@sap.com>>; ppc-aix-port-
>> d...@openjdk.java.net; 
>> 'build-dev@openjdk.java.net' > d...@openjdk.java.net>
>> Subject: Re: AIX : -bnorwexec linker flag
>>
>>   From a build point of view, the patch looks good. I cannot comment on
>> the validity of adding the flag though.
>>
>> /Erik
>>
>> On 2019-04-12 02:15, Baesken, Matthias wrote:
>>> Hello,  I have a question regarding the  AIX  -bnorwexec linker flag .
>>> I think it is related to an AIX  security feature  SED , see also  :
>>>
>>>
>> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.
>> aix.security/stack_exec_disable_flags.htm
>>> When building with the additional  -bnorwexec linker flagwe signal  the
>> OS  that we "request"  the SED  feature .
>>> Please compare  a patched  and an unpatched  java   ( patched is flagged
>> "request"  while   unpatched uses the  "system" setting ).
>>> bash-4.3$ sedmgr -d /patched_jdk/images/jdk/bin/java
>>> /patched_jdk/images/images/jdk/bin/java : request
>>>
>>>
>>> bash-4.3$ sedmgr -d /normal_jdk/images/jdk/bin/java
>>> /normal_jdk/images/jdk/bin/java : system
>>>
>>>
>>> System config on the example machine is "normal" (default) select :
>>> bash-4.3$ sedmgr
>>> Stack Execution Disable (SED) mode: select
>>> SED configured in kernel: select
>>>
>>>
>>> In our  internal tests  I noticed so far no issues when setting the  -
>> bnorwexec linker flag  in OpenJDK  on AIX  .
>>> Do you have any experience  with it, do you see issues when setting the
>> flag ?
>>>
>>> The documentation of the  flag is a bit short .
>>>
>>>
>>>
>> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.
>> aix.cmds3/ld.htm
>>>
>>> norwexec
>>>
>>> Specifies that if the system's sed_config setting is not off, the process'
>> private data areas will have non-execute permission.
>>>
>>>
>>>
>>> Patch would be :
>>>
>>> diff -r 0d7fb7f07134 make/autoconf/flags-ldflags.m4
>>> --- a/make/autoconf/flags-ldflags.m4Mon Apr 08 06:56:37 2019 +0100
>>> +++ b/make/autoconf/flags-ldflags.m4 Mon Apr 08 10:50:07 2019 +0200
>>> @@ -1,5 +1,5 @@
>>> #
>>> -# Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights 
>>> reserved.
>>> +# Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights 
>>> reserved.
>>> # DO 

RE: RFR(XS): 8220164: Fix build instructions for AIX

2019-03-05 Thread Doerr, Martin
Hi Volker,

the wiki is already up to date, so I like your new version which just refers to 
it. Reviewed.

Best regards,
Martin


-Original Message-
From: build-dev  On Behalf Of Volker Simonis
Sent: Dienstag, 5. März 2019 16:17
To: build-dev 
Subject: RFR(XS): 8220164: Fix build instructions for AIX

Hi,

can I please have a review for the following trivial change which
updates the AIX/XLC information in the the building.{md, html} files:

https://bugs.openjdk.java.net/browse/JDK-8220164
http://cr.openjdk.java.net/~simonis/webrevs/2019/8220164/

The build instructions for AIX in doc/building.{md, html} still
reference our old AIX build results page which isn't active since
quite some time and refers to old OS/compiler versions.

Remove the old information and link to the OpenJDK build Wiki instead
(https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms)
which should always have the current information.

Thank you and best regards,
Volker


RE: RFR : 8218965: aix: support xlclang++ in the compiler detection

2019-02-18 Thread Doerr, Martin
Hi Matthias,

excellent. Looks good to me. This should make AIX ready for JEP 347.

Thanks
Martin


From: Baesken, Matthias
Sent: Montag, 18. Februar 2019 13:53
To: Magnus Ihse Bursie ; 
'build-dev@openjdk.java.net' 
Cc: Doerr, Martin 
Subject: RE: RFR : 8218965: aix: support xlclang++ in the compiler detection

Hello Martin and Magnus,

I  included Martin’s  harfbuzz fix  and adjusted  the  xlc version check   ( 
renamed  variable to XLC_USES_CLANG and also check the TOOLCHAIN_PATH ) .

>
>If we're talking about a short migration story, where soon XLC 16 will be 
>required, and we can just replace

>TOOLCHAIN_CC_BINARY_xlc="xlc_r"
>with

>TOOLCHAIN_CC_BINARY_xlc="xlclang"
> then I can accept it anyway, so we don't need to complicate things.
>

Yes , that’s the idea -  to do the replacement above   sooner or later ;  
depends of course also on the  introduction of the C++11/14 features in the 
code base .


New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8218965.1/


Best Regards, Matthias



From: Magnus Ihse Bursie 
mailto:magnus.ihse.bur...@oracle.com>>
Sent: Montag, 18. Februar 2019 11:18
To: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>; 
'build-dev@openjdk.java.net' 
mailto:build-dev@openjdk.java.net>>
Cc: Doerr, Martin mailto:martin.do...@sap.com>>
Subject: Re: RFR : 8218965: aix: support xlclang++ in the compiler detection


On 2019-02-15 14:30, Baesken, Matthias wrote:



Are they both pointing to the same binary, and the mode of operation

(legacy xlc vs xlclang) is determined by the name of the executable?





Hello, in the installation I use   I have separate  binaries .







Is xlclang++ always available for version 16+ of xlc?





I think so,  as least I am not  aware of an installation mode with separate  
binaries .

However I am not an XLC  installation guru  .





If so, maybe we should just make sure we call the compiler with the

correct name if version 16+ is detected?





I thought  that we currently  first set  the  toolchain  name and then  set a 
fix  name for the binary  and check the version .

But I might be wrong.  Maybe  we need  to adjust this .

Or just at some future  point in time  declare  xlc16   as minimum   
requirement  (this makes things  easier , we can then use  the new binary names 
  ).

Yeah, we can adjust the process if needed. And to solve this *properly*, we 
should. I still think this looks like the wrong way to do it. But...

If we're talking about a short migration story, where soon XLC 16 will be 
required, and we can just replace

TOOLCHAIN_CC_BINARY_xlc="xlc_r"
with

TOOLCHAIN_CC_BINARY_xlc="xlclang"
then I can accept it anyway, so we don't need to complicate things.

I also don't like how xlclang is just run from the path, but OTOH it's only you 
guys who are going to run that in practice, and it's just going to be 
temporary, so, whatever.

The name AIX_USE_CLANG is not really correct, though. This is not about AIX. It 
should be XLC_USE_CLANG (or maybe better XLC_USES_CLANG, even perhaps 
XLC_IS_CLANG?!). And, as I said, it should use true/false, not 0/1.

If you fix this, and we agree that this is a temporary measure, I'm OK with the 
patch.

/Magnus





RE: RFR : 8218965: aix: support xlclang++ in the compiler detection

2019-02-18 Thread Doerr, Martin
Hi Matthias,

thanks for working on xlclang++ support.

Seems like we only 2 changes away from C++14 support on AIX:
1. The TOOLCHAIN_CC/CXX_BINARY_xlc change you proposed.
2. Backport the following harfbuzz change:
Support xlclang++ on AIX. (#1584)
src/java.desktop/share/native/libfontmanager/harfbuzz/hb-atomic-private.hh
-#elif !defined(HB_NO_MT) && defined(_AIX) && defined(__IBMCPP__)
+#elif !defined(HB_NO_MT) && defined(_AIX) && (defined(__IBMCPP__) || 
defined(__ibmxl__))

Can you add this tiny harfbuzz backport to your change once the xlC/xlclang++ 
selection mechanism is clearified?

Best regards,
Martin


-Original Message-
From: Baesken, Matthias 
Sent: Freitag, 15. Februar 2019 14:31
To: Magnus Ihse Bursie ; 
'build-dev@openjdk.java.net' 
Cc: Doerr, Martin 
Subject: RE: RFR : 8218965: aix: support xlclang++ in the compiler detection

>
> Are they both pointing to the same binary, and the mode of operation
> (legacy xlc vs xlclang) is determined by the name of the executable?
> 

Hello, in the installation I use   I have separate  binaries .


>
> Is xlclang++ always available for version 16+ of xlc?
> 

I think so,  as least I am not  aware of an installation mode with separate  
binaries .
However I am not an XLC  installation guru  .

>
> If so, maybe we should just make sure we call the compiler with the
> correct name if version 16+ is detected?
>

I thought  that we currently  first set  the  toolchain  name and then  set a 
fix  name for the binary  and check the version .
But I might be wrong.  Maybe  we need  to adjust this .
Or just at some future  point in time  declare  xlc16   as minimum   
requirement  (this makes things  easier , we can then use  the new binary names 
  ).

Best regards, Matthias


> -Original Message-
> From: Magnus Ihse Bursie 
> Sent: Freitag, 15. Februar 2019 13:32
> To: Baesken, Matthias ; 'build-
> d...@openjdk.java.net' 
> Cc: Doerr, Martin 
> Subject: Re: RFR : 8218965: aix: support xlclang++ in the compiler detection
> 
> On 2019-02-15 12:53, Baesken, Matthias wrote:
> > Hi Magnus,
> >
> > we are currently  able to build (+ test  )jdk/jdk   on AIX   with the
> xlclang++  based build .
> > Only needed  are  this change ,
> > plus   a one-liner  in harfhuzz  is needed   (we try to get this upstream  
> > into
> harbuzz directly,  in case the next harfhuzz update to jdk/jdk  is in the 
> mid/far
> future ,  I would add this one liner to my patch).
> >
> > So I  hope  that  there will be not so many follow ups   (but you never know
> ).
> 
> Ok, that's good to hear.
> >
> >> If so, could the choice between -g ang -g1 be handled with the normal
> >> TOOLCHAIN_CHECK_COMPILER_VERSION?
> >>
> > I'll look into this .   Unfortunately  the  version output is the  same for 
> >  both
> frontends :
> >
> > New one:
> >
> > bash-4.4$ xlclang++ -qversion
> > IBM XL C/C++ for AIX, V16.1.0  (some-strange-hex-string)
> > Version: 16.01..0001
> >
> > Legacy xlc:
> >
> > bash-4.4$ xlC_r -qversion
> > IBM XL C/C++ for AIX, V16.1.0  (some-strange-hex-string)
> > Version: 16.01..0001
> >
> >
> > (and  some-strange-hex-string  is  the same for both)
> Hm.
> 
> Are they both pointing to the same binary, and the mode of operation
> (legacy xlc vs xlclang) is determined by the name of the executable?
> 
> Is xlclang++ always available for version 16+ of xlc?
> 
> If so, maybe we should just make sure we call the compiler with the
> correct name if version 16+ is detected?
> 
> Or is there a way to force xlclang mode using a flag?
> 
> /Magnus
> 



RE: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread Doerr, Martin
Hi Matthias,

looks good to me, too.

Best regards,
Martin


-Original Message-
From: hotspot-dev  On Behalf Of Baesken, 
Matthias
Sent: Donnerstag, 7. Februar 2019 10:13
To: Magnus Ihse Bursie ; 
'hotspot-...@openjdk.java.net' ; 
'build-dev@openjdk.java.net' 
Subject: RE: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for 
clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

Thanks .  
A second review would be great .

Best regards, Matthias

> -Original Message-
> From: Magnus Ihse Bursie 
> Sent: Mittwoch, 6. Februar 2019 17:38
> To: Baesken, Matthias ; 'hotspot-
> d...@openjdk.java.net' ; 'build-
> d...@openjdk.java.net' 
> Subject: Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for
> clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings
> 
> On 2019-02-06 16:17, Baesken, Matthias wrote:
> > Hello, please review this small change .
> >
> > I noticed (when looking into AIX xlc16 / xlclang++)  the following:in 
> > the
> case that  clang is used for compilation,HOTSPOT_BUILD_COMPILER   claims
> to be a gcc .
> > This is a bit misleading.
> > This change  adjusts the setting for clang usage on  AIX  (for future usage
> with xlclang++)   and macOSX.
> >
> > Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for  legacy
> Oracle / Sun Studio versions ).
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218562
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/
> Looks good to me.
> 
> /Magnus
> >
> >
> > Thanks, Matthias



RE: 8212110: Build of saproc.dll broken on Windows 32 bit after JDK-8210647

2018-10-12 Thread Doerr, Martin
Hi Severin,

thank you for providing this fix so quickly. It looks good to me.

I was able to build jdk11u with this patch.

Just for information:
I'm using Visual Studio 2013 and configure with --disable-warnings-as-errors.
(Otherwise I got
Creating support/modules_cmds/jdk.hotspot.agent/jhsdb.exe from 1 file(s)
jdk11u/src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp(773) : 
error C2220: warning treated as error - no 'object' file generated
jdk11u/src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp(773) : 
warning C4244: 'argument' : conversion from 'jlong' to 'HCRYPTKEY', possible 
loss of data
jdk11u/src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp(773) : 
warning C4244: 'argument' : conversion from 'jlong' to 'HCRYPTPROV', possible 
loss of data
jdk11u/src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp(967) : 
warning C4244: 'argument' : conversion from 'jlong' to 'HCRYPTKEY', possible 
loss of data
jdk11u/src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp(967) : 
warning C4244: 'argument' : conversion from 'jlong' to 'HCRYPTPROV', possible 
loss of data
)

But that's unrelated to your fix. It works fine.

Best regards,
Martin


-Original Message-
From: Severin Gehwolf  
Sent: Freitag, 12. Oktober 2018 11:20
To: build-dev 
Cc: Doerr, Martin ; Baesken, Matthias 

Subject: RFR: 8212110: Build of saproc.dll broken on Windows 32 bit after 
JDK-8210647

Hi,

Please review this fix for a build failure of saproc.dll on Windows 32
bit. For 32 bit Windows builds flag -RTC1 gets added unconditionally.
Yet, as per compiler doc[1] it should only be added for development
builds. This causes a build failure post JDK-8210647 as in
release/fastdebug variant builds of the JDK saproc.dll will get
optimization flags producing an incompatible set of flags. This used to
work prior JDK-8210647, because saproc.dll was never optimized (not
even in release/fastdebug variants).

The proposed fix is to only add the flag for slowdebug builds, which
corresponds to development builds in OpenJDK. Thoughts?

Bug: https://bugs.openjdk.java.net/browse/JDK-8212110
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8212110/webrev.01/

Unfortunately, I don't have a system to actually test this on. So will
rely on Martin or Matthias to test whether this fixes the build failure
[2] they are seeing. AFAIK, Oracle no longer builds on Windows 32 bit
either.

Thanks,
Severin

[1] https://msdn.microsoft.com/en-us/library/8wtf2dfz.aspx
[2] 
http://mail.openjdk.java.net/pipermail/jdk-updates-dev/2018-October/000213.html



RE: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-08 Thread Doerr, Martin
+1

Thanks for improving the code.

Best regards,
Martin


-Original Message-
From: Lindenmaier, Goetz 
Sent: Montag, 8. Oktober 2018 12:36
To: Volker Simonis ; Doerr, Martin 

Cc: build-dev ; 
hotspot-runtime-...@openjdk.java.net runtime 

Subject: RE: RFR(XS): 8211837: Creation of the default CDS Archive should 
depend on ENABLE_CDS

Still looks good (now touches much more scenarios, but we should 
get this straight.)

Best regards,
  Goetz.

> -Original Message-
> From: hotspot-runtime-dev  boun...@openjdk.java.net> On Behalf Of Volker Simonis
> Sent: Montag, 8. Oktober 2018 12:11
> To: Doerr, Martin 
> Cc: build-dev ; hotspot-runtime-
> d...@openjdk.java.net runtime 
> Subject: Re: RFR(XS): 8211837: Creation of the default CDS Archive should
> depend on ENABLE_CDS
> 
> Hi Martin, Goetz,
> 
> thanks for reviewing my patch! As Aleksey posted a similar patch for
> fixing the Zero build I've extended my patch to handle
> zero/minimal/core as well.
> 
> In fact the patch now disables CDS on AIX, minimal, core and zero
> unless the user explicitly requests it with the help of
> `--with-jvm-features=cds`. The configuration variant with
> `--with-jvm-features=-cds` should now also be handled correctly (I
> hope caught all possible combinations :)
> 
> If CDS is disabled, the creation of the default class list and default
> archive will now be disabled as well.
> 
> @Aleksey Shipilev : can you please check if this new version of my
> patch also works for you?
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/
> 
> Thank you and best regards,
> Volker
> On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin 
> wrote:
> >
> > Hi Volker,
> >
> > looks good. Thanks for fixing.
> > Of course, it would be great if this could be used to fix minimal/zero 
> > build,
> too.
> >
> > Best regards,
> > Martin
> >
> >
> > -Original Message-
> > From: hotspot-runtime-dev  boun...@openjdk.java.net> On Behalf Of Volker Simonis
> > Sent: Montag, 8. Oktober 2018 10:19
> > To: build-dev ; hotspot-runtime-
> d...@openjdk.java.net runtime 
> > Subject: RFR(XS): 8211837: Creation of the default CDS Archive should
> depend on ENABLE_CDS
> >
> > Hi,
> >
> > can I please have a review for the following trivial build fix:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
> > https://bugs.openjdk.java.net/browse/JDK-8211837
> >
> > It makes no sense to try to create a default CDS archive on VMs which
> > don't support CDS at all. We already have '--enable_cds' which
> > defaults to 'true' on all platforms except AIX.
> >
> > The check for '--enable_cds_archive' should use the result of
> > '--enable_cds' (which is saved in "ENABLE_CDS") and only enable
> > default archive creation if CDS is enabled.
> >
> > Failure to do so currently breaks the AIX build (after JDK-)8202951 was
> pushed).
> >
> > Thank you and best regards,
> > Volker


RE: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-08 Thread Doerr, Martin
Hi Volker,

looks good. Thanks for fixing.
Of course, it would be great if this could be used to fix minimal/zero build, 
too.

Best regards,
Martin


-Original Message-
From: hotspot-runtime-dev  On 
Behalf Of Volker Simonis
Sent: Montag, 8. Oktober 2018 10:19
To: build-dev ; 
hotspot-runtime-...@openjdk.java.net runtime 

Subject: RFR(XS): 8211837: Creation of the default CDS Archive should depend on 
ENABLE_CDS

Hi,

can I please have a review for the following trivial build fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
https://bugs.openjdk.java.net/browse/JDK-8211837

It makes no sense to try to create a default CDS archive on VMs which
don't support CDS at all. We already have '--enable_cds' which
defaults to 'true' on all platforms except AIX.

The check for '--enable_cds_archive' should use the result of
'--enable_cds' (which is saved in "ENABLE_CDS") and only enable
default archive creation if CDS is enabled.

Failure to do so currently breaks the AIX build (after JDK-)8202951 was pushed).

Thank you and best regards,
Volker


RE: RFR(XS): 8211097: aix: fix build after JDK-8210919

2018-09-27 Thread Doerr, Martin
Thanks for the reviews. Pushed.

Best regards,
Martin


-Original Message-
From: Erik Joelsson  
Sent: Mittwoch, 26. September 2018 20:02
To: Doerr, Martin ; 'build-dev@openjdk.java.net' 

Subject: Re: RFR(XS): 8211097: aix: fix build after JDK-8210919

Looks good.

/Erik


On 2018-09-26 06:26, Doerr, Martin wrote:
> Hi,
>
> we need to fix the build on AIX as suggested by Magnus.
>
> Webrev:
> http://cr.openjdk.java.net/~mdoerr/8211097_aix_fix_build_after_8210919/webrev.00/
>
> Please review.
>
> Thanks,
> Martin
>



RFR(XS): 8211097: aix: fix build after JDK-8210919

2018-09-26 Thread Doerr, Martin
Hi,

we need to fix the build on AIX as suggested by Magnus.

Webrev:
http://cr.openjdk.java.net/~mdoerr/8211097_aix_fix_build_after_8210919/webrev.00/

Please review.

Thanks,
Martin



RE: RFR : 8210205 : build fails on AIX in hotspot cpp tests (for example getstacktr001.cpp)

2018-08-30 Thread Doerr, Martin
Hi Matthias,

thanks for fixing the build. Looks good.
I think it can be treated as trivial and pushed, because it just renames 
NUMBER_OF_FRAMES to avoid a conflict with AIX stuff.

Best regards,
Martin


From: Baesken, Matthias
Sent: Donnerstag, 30. August 2018 16:14
To: Lindenmaier, Goetz ; 
'hotspot-...@openjdk.java.net' ; 
'build-dev@openjdk.java.net' 
Cc: Doerr, Martin 
Subject: RE: RFR : 8210205 : build fails on AIX in hotspot cpp tests (for 
example getstacktr001.cpp)

Thanks ,  can I have a second review please ?

Best regards, Matthias

From: Lindenmaier, Goetz
Sent: Donnerstag, 30. August 2018 14:17
To: Baesken, Matthias 
mailto:matthias.baes...@sap.com>>; 
'hotspot-...@openjdk.java.net' 
mailto:hotspot-...@openjdk.java.net>>; 
'build-dev@openjdk.java.net' 
mailto:build-dev@openjdk.java.net>>
Cc: Doerr, Martin mailto:martin.do...@sap.com>>
Subject: RE: RFR : 8210205 : build fails on AIX in hotspot cpp tests (for 
example getstacktr001.cpp)

Hi Matthias,

Aix is a nuisance.  Thanks for fixing this.
Looks good.

As this is a build fix, and only changes tests, I think you don't need
to follow the 24h rule, i.e., you can push it sooner.

Best regards,
  Goetz.

From: Baesken, Matthias
Sent: Donnerstag, 30. August 2018 13:21
To: 'hotspot-...@openjdk.java.net' 
mailto:hotspot-...@openjdk.java.net>>; 
'build-dev@openjdk.java.net' 
mailto:build-dev@openjdk.java.net>>
Cc: Lindenmaier, Goetz 
mailto:goetz.lindenma...@sap.com>>; Doerr, Martin 
mailto:martin.do...@sap.com>>
Subject: RFR : 8210205 : build fails on AIX in hotspot cpp tests (for example 
getstacktr001.cpp)

Hello,  please review this small fix to repair our AIX build .
Recent changes to jdk/jdk broke  the  build .

We  get clashes   with defines from system headers  in a few compilation units, 
for example :


"/openjdk/nb/rs6000_64/nightly/jdk/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetStackTrace/getstacktr001/getstacktr001.cpp",
 line 69.9: 1540-0848 (S)
The macro name "NUMBER_OF_FRAMES" is already defined with a different 
definition.
"/usr/include/sys/mstsave.h", line 260.9: 1540-0425 (I) "NUMBER_OF_FRAMES" is 
defined on line 260 of "/usr/include/sys/mstsave.h".



Renaming NUMBER_OF_FRAMES  fixes the issue.

Bug :

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


webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8210205/



Thanks, Matthias




RE: RFR(L) : 8199370: [TESTBUG] Open source vm testbase GC tests

2018-05-18 Thread Doerr, Martin
Hi Igor,

we get compiler warnings on linux ppc64le (GCC 4.8.5):

libnativeGC05.c:80:19: error: 'pair_getj_mid' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
 j = (*env)->CallIntMethod(env, pair, pair_getj_mid);
   ^

libnativeGC05.c:78:19: error: 'pair_geti_mid' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
 i = (*env)->CallIntMethod(env, pair, pair_geti_mid);
   ^

Unfortunately, the files are compiled with warnings as errors. We think this 
should get fixed.

Best regards,
Martin


-Original Message-
From: hotspot-gc-dev [mailto:hotspot-gc-dev-boun...@openjdk.java.net] On Behalf 
Of Igor Ignatyev
Sent: Dienstag, 15. Mai 2018 20:59
To: Erik Helin 
Cc: hotspot-gc-dev ; build-dev 
; hotspot-dev developers 

Subject: Re: RFR(L) : 8199370: [TESTBUG] Open source vm testbase GC tests

Hi Erik,

please see my answers inline.

can I consider this RFR as reviewed by you? 

Thanks,
-- Igor

> On May 14, 2018, at 4:23 AM, Erik Helin  wrote:
> 
> On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
>> Hi all,
> 
> Hi Igor,
> 
> On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
>> could you please review the patch which open sources GC tests from vm 
>> testbase? it introduces the following test groups:
>> - vmTestbase_vm_g1classunloading
>> - vmTestbase_vm_gc_compact
>> - vmTestbase_vm_gc_concurrent
>> - vmTestbase_vm_gc_container
>> - vmTestbase_vm_gc_juggle
>> - vmTestbase_vm_gc_locker
>> - vmTestbase_vm_gc_ref
>> - vmTestbase_vm_gc_misc
> 
> This is a very welcome, and pretty massive, change :) I won't be able to read 
> through each test, there are simple too many, but I can sample a few of them 
> and have a look.
> 
> On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
>> As usually w/ VM testbase code, these tests are old, they have been run in 
>> hotspot testing for a long period of time. Originally, these tests were run 
>> by a test harness different from jtreg and had different build and execution 
>> schemes, some parts couldn't be easily translated to jtreg, so tests might 
>> have actions or pieces of code which look weird. In a long term, we are 
>> planning to rework them.
> 
> I'm also assuming that to help the open sourcing of these tests, most 
> comments will likely be deferred until later? If so, that is fine with me.
y, unless it is something very important and cost of delaying changes is high, 
I'd prefer to defer all comments/improvements till later. 
> 
> On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8199370
>> webrev: http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html
> 
> Many of the tests are a bit cryptic, but that can be refactored later. Could 
> you please file bugs for the two tests written in shell? Particularly 
> parOld/test.sh should be trivial to rewrite in Java.
sure, I've filed 8203239 and 8203238. 
> 
> It seems like a lot of tests contains a TEST.properties file with the content 
> `exclusiveAccess.dirs=.`. Could this become the default value somewhere, so 
> we don't need all those TEST.properties files?
y, but this will require finding a most top directory whose all tests have this 
TEST.properties file and it will also make it harder to understand how a test 
is executed. there was/is ongoing discussing w/ Jon on moving 'exclusiveAccess' 
to test description. anyhow I've filed an RFE to clean that up -- 8203241.

> 
> Thanks,
> Erik
> 
>> Thanks,
>> -- Igor



RE: 8201226 missing JNIEXPORT / JNICALL at some places in function declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some places in function declarations/implementations

2018-04-10 Thread Doerr, Martin
Hi Matthias,

thank you for providing the fix. I think these issues should really get fixed 
regardless if 32 bit is supported or not. It's not good to have inconsistent 
JNI declarations.

The fix looks good to me.

I guess it should get pushed to the submission forest because it contains 
hotspot changes. Or did Oracle run the necessary tests?

Best regards,
Martin


-Original Message-
From: Baesken, Matthias 
Sent: Dienstag, 10. April 2018 12:15
To: Alexey Ivanov <alexey.iva...@oracle.com>; Magnus Ihse Bursie 
<magnus.ihse.bur...@oracle.com>
Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com>
Subject: RE: 8201226 missing JNIEXPORT / JNICALL at some places in function 
declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at some 
places in function declarations/implementations

Hello,  I  had to  do another small adjustment  to make jimage.hpp/cpp match.   
 Please review :

http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.2/

With the latest webrev I could finallybuildjdk/jdk   successfully on 
both  win32bit   and win64 bit .



Thanks again  to Alexey  to provide  the   incorporated patch .


Best regards, Matthias



> -Original Message-
> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
> Sent: Montag, 9. April 2018 17:14
> To: Baesken, Matthias <matthias.baes...@sap.com>; Magnus Ihse Bursie
> <magnus.ihse.bur...@oracle.com>
> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin
> <martin.do...@sap.com>
> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in function
> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
> some places in function declarations/implementations
> 
> Hi Matthias,
> 
> On 09/04/2018 15:38, Baesken, Matthias wrote:
> > Hi  Alexey,thanks  for  the diff provided by you, and  for  the  
> > explanations
> .
> >
> > I created  a second  webrev :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8201226.1/
> >
> > -   it  adds  the diff  provided by you(hope that’s fine with you)
> 
> Yes, that's fine with me.
> There could be only one author ;)
> 
> > -changes  2 launcherssrc/java.base/share/native/launcher/main.c
> > and
> src/jdk.pack/share/native/unpack200/main.cppwhere we face similar
> issues after mapfile removal for exes
> 
> I'd rather remove both JNIEXPORT and JNICALL from main().
> It wasn't exported, and it shouldn't be.
> 
> Regards,
> Alexey
> 
> >
> >
> >
> > Best regards , Matthias
> >
> >
> >> -Original Message-
> >> From: Alexey Ivanov [mailto:alexey.iva...@oracle.com]
> >> Sent: Montag, 9. April 2018 15:52
> >> To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; Baesken,
> >> Matthias <matthias.baes...@sap.com>
> >> Cc: build-dev <build-dev@openjdk.java.net>; Doerr, Martin
> >> <martin.do...@sap.com>
> >> Subject: Re: 8201226 missing JNIEXPORT / JNICALL at some places in
> function
> >> declarations/implementations - was : RE: missing JNIEXPORT / JNICALL at
> >> some places in function declarations/implementations
> >>
> >> Hi Matthias, Magnus,
> >>
> >> The problem is with JNICALL added to the functions. JNICALL expands to
> >> __stdcall [1] on Windows. On 64 bit, the modifier has no effect and is
> >> ignored. On 32 bit, the modifier changes the way parameters are pass and
> >> the function name is decorated with an underscore and with @ + the size
> >> of arguments.
> >>
> >> If I remove JNICALL modifier from the exported functions, they're
> >> exported with plain name as in source code (plus, __cdecl [2] calling
> >> convention is used.)
> >>
> >> zip.dll and jimage.dll are affected by this. It's because the exported
> >> functions are looked up by name rather than using a header file with
> >> JNIIMPORT. See
> >>
> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla
> >> ssLoader.cpp#l1155
> >>
> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla
> >> ssLoader.cpp#l1194
> >>
> >> JNICALL modifier must also be removed from type declarations for
> >> functions from zip.dll:
> >>
> http://hg.openjdk.java.net/jdk/client/file/tip/src/hotspot/share/classfile/cla
> >> ssLoader.cpp#l81
> >>
> >> I'm attaching the patch to zip and jimage as well as classLoader.cpp
> >> which takes these changes into account. It does not replace Matthias'
> >&g

RE: Unification of jni_.h

2017-10-23 Thread Doerr, Martin
Hi,

we only support 64 bit on s390. Seems like the code could be cleaned up or 
updated. Thanks for looking into it.

Best regards,
Martin


-Original Message-
From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-boun...@openjdk.java.net] 
On Behalf Of David Holmes
Sent: Montag, 23. Oktober 2017 05:47
To: Magnus Ihse Bursie ; 
hotspot-runtime-...@openjdk.java.net; build-dev 
Subject: Re: Unification of jni_.h

Hi Magnus,

On 18/10/2017 11:10 PM, Magnus Ihse Bursie wrote:
> In the context of JDK-8167078 (Duplicate header files in hotspot and 
> jdk), I was looking at unifying the platform specific extensions to 
> jni.h, the jni_md.h files between hotspot and java.base.
> 
> The main difference here is that the hotspot jni_* files are divided 
> into individual files based on cpu, while the java.base versions are 
> divided according to operating system (which, in turn, implicates 
> compiler). On a second look, however, it turns out to not be so 
> problematic -- a hotspot file like jni_x86.h contains basically
> #ifdef WIN32
> ... // do windows stuff
> #else
> ... // do posix/macos stuff
> #endif
> 
> and the two blocks match very well what the java.base versions are doing 
> for the different operating systems.
> 
> In fact, the OS (and compiler) division seems more natural, since there 
> is much redundancy in the hotspot files, and the java.base OS-based 
> versions are much simpler.
> 
> I'm proposing to remove the hotspot CPU-based files and just replace 
> them with the java.base versions. However, a few differences crept out. 
> I'd like to discuss them before proceeding.
> 
> For jni_aarch64.h: There's a windows (or rather, "not unix") part of the 
> jni_aarch64.h that I do not believe have ever been used. Nevertheless, 
> it contains "typedef int jint" rather than "typedef long jint", which 
> the java.base windows version uses. Since I've never heard of aarch64 
> building on Windows, I presume this is irrelevant.

Likely just copied from the x86 version at some point. There is no 
Windows-aarch64 support in the OpenJDK.

> For jni_aarch64.h and jni_sparc.h: The unix version will match with the 
> java.base version as long as _LP64 is defined. This should be just fine, 
> since we define that when building hotspot/the JDK, and the java.base 
> version that is exported by the JDK on linux/aarch64 and solaris/sparc 
> has always required the _LP64 define.

Yep 64-bit only.

> (In fact, the macos and unix versions in java.base only contain trivial 
> differences. The macos version should probably be removed in favor of a 
> single unix version.)
> 
> For jni_x86.h: There windows part suffers the same problem as for 
> aarch64, but here it's potentially problematic. The hotspot version uses 
> "typedef int jint" while the java.base version uses "typedef long jint". 
> If this *really* were a problem, we would probably have gotten reports 
> from JNI code unable to work on Windows, so I assume this turns out to 
> be the same datatype in the end. Don't know if it's due to luck, 
> compiler flags or by definition.

Windows is either ILP32 or LLP64 - in both cases int and long are the 
same and 32-bits.

> 
> For jni_s390.h: Here's the most problematic version. The exported 
> java.base version uses:
> #ifdef _LP64
> typedef long jlong;
> #else
> typedef long long jlong;
> #endif
> but s390 uses:
> typedef long int jlong;
> 
> My best assumption here is that we're only really using s390x (the 
> 64-bit version) and that "long int" is functionally equivalent to 
> "long". Or that jni is broken on s390 and nobody really noticed.
> 
> Also, s390 uses a simplified version of the gcc attribute dance used for 
> JNIEXPORT/JNIIMPORT. I think the s390 version is just not really 
> updated, and that using the newer in java.base is harmless.

Can't comment on s390.

> For jni_arm.h: Finally, the jni_arm.h (the 32-bit formerly closed Oracle 
> port), the JNIEXPORT/JNIIMPORT is different, by defining 
> __attribute__((externally_visible)). This might have been relevant due 
> to compile/link time symbol processing for that platform, but 
> technically it should probably not have been connected to the platform 
> per se, but rather to the compilation procedure. Since the arm-32 
> platform is kind of abandoned right now, I propose to modify the 
> compilation to be more standard, if this is required, rather than 
> keeping these attributes.

As Dean stated this likely related to LTO. And unfortunately attention 
was not paid to the comment:

// Note: please do not change these without also changing jni_md.h in 
the JDK
// repository

possibly due to open/closed issues back in time. But I'd say we would 
want the hotspot version so that nothing that would previously work is 
now broken.

Thanks,
David

> 
> /Magnus


RE: RFR(s) 8170153: PPC64: Poor StrictMath performance due to non-optimized compilation

2016-11-23 Thread Doerr, Martin
Hi Gustavo,

thanks for providing the webrevs.

I have ran the StrictMath jck tests which fail when building with -O3 and 
without -ffp-contract=off:
FailedTests:
api/java_lang/StrictMath/desc.html#acos 
javasoft.sqe.tests.api.java.lang.StrictMath.acos_test
api/java_lang/StrictMath/desc.html#asin 
javasoft.sqe.tests.api.java.lang.StrictMath.asin_test
api/java_lang/StrictMath/desc.html#atan 
javasoft.sqe.tests.api.java.lang.StrictMath.atan_test
api/java_lang/StrictMath/desc.html#atan2 
javasoft.sqe.tests.api.java.lang.StrictMath.atan2_test
api/java_lang/StrictMath/desc.html#cos 
javasoft.sqe.tests.api.java.lang.StrictMath.cos_test
api/java_lang/StrictMath/desc.html#exp 
javasoft.sqe.tests.api.java.lang.StrictMath.exp_test
api/java_lang/StrictMath/desc.html#log 
javasoft.sqe.tests.api.java.lang.StrictMath.log_test
api/java_lang/StrictMath/desc.html#sin 
javasoft.sqe.tests.api.java.lang.StrictMath.sin_test
api/java_lang/StrictMath/desc.html#tan 
javasoft.sqe.tests.api.java.lang.StrictMath.tan_test
api/java_lang/StrictMath/index.html#expm1 
javasoft.sqe.tests.api.java.lang.StrictMath.expm1Tests -TestCaseID ALL
api/java_lang/StrictMath/index.html#log10 
javasoft.sqe.tests.api.java.lang.StrictMath.log10Tests -TestCaseID ALL
api/java_lang/StrictMath/index.html#log1p 
javasoft.sqe.tests.api.java.lang.StrictMath.log1pTests -TestCaseID ALL

All of them have passed when building with -O3 and -ffp-contract=off (on 
linuxppc64le).

So thumbs up from my side.

Thanks and best regards,
Martin



-Original Message-
From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-boun...@openjdk.java.net] On 
Behalf Of Volker Simonis
Sent: Mittwoch, 23. November 2016 15:06
To: Gustavo Romero 
Cc: build-dev ; ppc-aix-port-...@openjdk.java.net; 
Java Core Libs ; hotspot-...@openjdk.java.net
Subject: Re: RFR(s) 8170153: PPC64: Poor StrictMath performance due to 
non-optimized compilation

Hi Gustavo,

thanks a lot for tracking this down!

The change looks good and I a can sponsor it once you get another review from 
the build group and the FC Extension Request was approved.

In general I'd advise to sign the OCTLA [1] to get access to the Java SE TCK 
[2] as this contains quite a lot of additional conformance tests which can be 
quite valuable for changes like this.

Regards,
Volker

[1] http://openjdk.java.net/legal/octla-java-se-8.pdf
[2] http://openjdk.java.net/groups/conformance/JckAccess/


On Tue, Nov 22, 2016 at 1:43 AM, Gustavo Romero  
wrote:
> Hi,
>
> Could the following change be reviewed, please?
>
> webrev 1/2: http://cr.openjdk.java.net/~gromero/8170153/
> webrev 2/2: http://cr.openjdk.java.net/~gromero/8170153/jdk/
> bug:https://bugs.openjdk.java.net/browse/JDK-8170153
>
> It enables fdlibm optimization on Linux PPC64 LE & BE and hence speeds 
> up the StrictMath methods (in some cases up to 3x) on that platform.
>
> On PPC64 fdlibm optimization can be done without precision issues if 
> floating-point expression contraction is disable, i.e. if the compiler 
> does not use floating-point multiply-add (FMA). For further details 
> please refer to gcc
> bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78386
>
> No regression was observed on Math and StrictMath tests:
>
> Passed: java/lang/Math/AbsPositiveZero.java
> Passed: java/lang/Math/Atan2Tests.java
> Passed: java/lang/Math/CeilAndFloorTests.java
> Passed: java/lang/Math/CubeRootTests.java
> Passed: java/lang/Math/DivModTests.java
> Passed: java/lang/Math/ExactArithTests.java
> Passed: java/lang/Math/Expm1Tests.java
> Passed: java/lang/Math/FusedMultiplyAddTests.java
> Passed: java/lang/Math/HyperbolicTests.java
> Passed: java/lang/Math/HypotTests.java
> Passed: java/lang/Math/IeeeRecommendedTests.java
> Passed: java/lang/Math/Log10Tests.java
> Passed: java/lang/Math/Log1pTests.java
> Passed: java/lang/Math/MinMax.java
> Passed: java/lang/Math/MultiplicationTests.java
> Passed: java/lang/Math/PowTests.java
> Passed: java/lang/Math/Rint.java
> Passed: java/lang/Math/RoundTests.java
> Passed: java/lang/Math/SinCosCornerCasesTests.java
> Passed: java/lang/Math/TanTests.java
> Passed: java/lang/Math/WorstCaseTests.java
> Test results: passed: 21
>
> Passed: java/lang/StrictMath/CubeRootTests.java
> Passed: java/lang/StrictMath/ExactArithTests.java
> Passed: java/lang/StrictMath/Expm1Tests.java
> Passed: java/lang/StrictMath/HyperbolicTests.java
> Passed: java/lang/StrictMath/HypotTests.java
> Passed: java/lang/StrictMath/Log10Tests.java
> Passed: java/lang/StrictMath/Log1pTests.java
> Passed: java/lang/StrictMath/PowTests.java
> Test results: passed: 8
>
> and also on the following hotspot tests:
>
> Passed: compiler/intrinsics/mathexact/sanity/AddExactIntTest.java
> Passed: compiler/intrinsics/mathexact/sanity/AddExactLongTest.java
> Passed: 
> compiler/intrinsics/mathexact/sanity/DecrementExactIntTest.java
> Passed: 
>