Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v3]

[Stuart Monteith]

On Thu, 15 Oct 2020 08:57:27 GMT, Bernhard Urban-Forster  
wrote:

>> I organized this PR so that each commit contains the warning emitted by MSVC 
>> as commit message and its relevant fix.
>> 
>> Verified on
>> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
>> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
>> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors` 
>> still works. Just mentioning this here, because
>>   it's yet another toolchain (Xcode / clang) that needs to be kept happy 
>> [going
>>   forward](https://openjdk.java.net/jeps/391).
>
> Bernhard Urban-Forster has updated the pull request with a new target base 
> due to a merge or a rebase. The pull request
> now contains 20 commits:
>  - disable warning only for hotspot
>  - Merge remote-tracking branch 'upstream/master' into 
> 8254072-fix-windows-arm64-warnings
>  - Merge remote-tracking branch 'upstream/master' into 
> 8254072-fix-windows-arm64-warnings
>  - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267: 
> 'argument': conversion from 'size_t' to
>'int', possible loss of data
>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267: 
> 'argument': conversion from 'size_t' to
>'int', possible loss of data 
> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267: 
> 'argument':
>conversion from 'size_t' to 'int', possible loss of data
>  - Revert changes for "warning C4146: unary minus operator applied to 
> unsigned type, result still unsigned"
>  - msvc: disable unary minus warning for unsigned types
>  - ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123): 
> warning C4267: 'initializing': conversion
>from 'size_t' to 'int', possible loss of data
>./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123): 
> warning C4267: 'initializing': conversion
>from 'size_t' to 'const int', possible loss of data
>  - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267: 
> 'argument': conversion from 'size_t' to
>'unsigned int', possible loss of data
>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267: 
> 'argument': conversion from 'size_t' to
>'int', possible loss of data 
> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146: 
> unary minus
>operator applied to unsigned type, result still unsigned 
> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441):
>warning C4267: 'argument': conversion from 'size_t' to 'int', possible 
> loss of data
>  - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312: 
> 'type cast': conversion from 'unsigned int'
>to 'address' of greater size
>  - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267: 
> 'argument': conversion from 'size_t' to
>'int', possible loss of data
>  - ... and 10 more: 
> https://git.openjdk.java.net/jdk/compare/9359ff03...32e922da

src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp line 658:

> 656: }
> 657:   }
> 658:   size_t size_in_bytes() { return 1ull << size(); }

Capital ULL - I find that easer to search for and it is more consistent.

-

PR: https://git.openjdk.java.net/jdk/pull/530

Re: RFR: 8252114: Windows-AArch64: Enable and test ZGC and ShenandoahGC [v2]

[Stuart Monteith]

On 11/09/2020 04:48, Monica Beckwith wrote:

On Thu, 10 Sep 2020 17:09:00 GMT, Monica Beckwith  wrote:


This looks good to me with Alexsey's changes applied.


Ref: https://github.com/openjdk/jdk/pull/97#issuecomment-690077743
Hi David,
Thanks. I wanted to start a conversation here as it does modify common code 
(the aarch64-linux combo check) and also
wanted to highlight the difference between Shenandoah and ZGC checks. IMHO, 
Shenandoah checks are cleaner and if people
agree, I would like to apply those to ZGC as well. (When that happens, it 
*will* be outside of the windows-aarch64 port
and I will change the info in JBS accordingly) Please let me know what you 
think.


Ref: https://github.com/openjdk/jdk/pull/97#issuecomment-690811308
I empathize with your comment, David. Here's a bit of a background for all -
When I started the port, both ZGC, and Shenandoah had the "xlinux-aarch64" check whereas 
the "xx86" check was more
elaborate for ZGC.  At that time I chose the ZGC route for both the GCs and 
added separate checks for each OS. This is
what the windows-aarch64 port has as well. (Since then Shenandoah has moved on 
to just checking the arch.) I already
had a JBS entry, and the (partial) patch, I decided to use that for my first 
GitHub PR. and was hoping to start the
conversation of the simpler route that Shenandoah took in the comments 
mentioning that as my preference and modify the
JBS entry (and the patch) accordingly.

Now coming back to this - @stooart-mon, do you think it is OK to just check for 
aarch64 (similar to Shenandoah)?
@stefank what do you think of simplifying the x86 checks (similar to 
Shenandoah)? Thanks all.

-

PR: https://git.openjdk.java.net/jdk/pull/97



Hi Monica,
	I guess the choice is either to enable aarch64, and have the Mac port disable ZGC if necessary, or do what you have 
done here and just enable by by architecture and supported platform. If Anton is happy with the simple check for 
aarch64, then I'm happy.


BR,
Stuart

Re: RFR: 8252114: Windows-AArch64: Enable and test ZGC and ShenandoahGC [v2]

[Stuart Monteith]

On Thu, 10 Sep 2020 05:16:00 GMT, Monica Beckwith  wrote:

>> Ah, Shenandoah does not filter for OS, so it is enabled for AArch64 already, 
>> right?
>
>> Ah, Shenandoah does not filter for OS, so it is enabled for AArch64 already, 
>> right?
> 
> +1

This looks good to me with Alexsey's changes applied.

-

PR: https://git.openjdk.java.net/jdk/pull/97

Re: RFR: JDK-8179892 Update build documentation for JDK 9

[Stuart Monteith]

Hi,
   It's good to see updated documentation - a project always feels a
bit stale if its documentation is out of date. However, I'd appreciate
it if Derek White's issue could be addressed:

"JDK-8182733: aarch64 build documentation misleading"
https://bugs.openjdk.java.net/browse/JDK-8182733

OpenJDK builds perfectly fine on AArch64 machines natively.

I don't think you need to make a specific recommendation for AArch64,
although my smallest machine has 8 cores and 16 GB, while, I'd guess
that the SPARC minimum requirements ought to be sufficient, I'd go
with that.

Thanks,
Stuart


On 22 June 2017 at 10:31, Volker Simonis  wrote:
> On Wed, Jun 21, 2017 at 12:59 PM, Magnus Ihse Bursie
>  wrote:
>> On 2017-06-21 09:02, Thomas Stüfe wrote:
>>>
>>> Hi Magnus,
>>>
>>> excellent work!
>>
>>
>> Thank you! :-)
>>
>>> Some minor remarks:
>>>
>>> You also mentioned Cygwin hg in the "Special considerations" section.
>>> However, because updating Cygwin is a bit of a pain (you need to close all
>>> consoles to do this) it may make sense to move the "Install Cygwin and all
>>> required packages" up to the beginning of the document and include
>>> "mercurial" in the list of cygwin packages to install.
>>>
>>> In addition to that (if contributing to the OpenJDK is in the scope for
>>> this document?) it may make sense to install at least ksh (for webrevs) and
>>> sftp (for uploading webrevs) for Cygwin.
>>
>>
>> For JDK 10, we can discuss where to draw the line of the responsibility of
>> this document. I don't think it can cover all OpenJDK developer stories,
>> like webrev, etc. But we should probably make sure that such a document
>> exists somewhere, in the repo or on the OpenJDK web site or wiki.
>>
>> I've tried putting information about what to install and how grouped to the
>> specific requirements. This has both drawbacks and benefits. One thing you
>> don't get is a "one stop" description of "install all these packages". That
>> could perhaps be added to the TL;DR section -- if you just want something
>> quick that is likely to work, and you don't care why, or if you install
>> slightly too much.
>>
>>> As for AIX, so far we have not yet broken compatibility with AIX 5.3.
>>> While this is something which we may give up in the future, currently
>>> jdk9/10 still build and run on AIX 5.3 and 6.1.
>>
>>
>> Ok, I updated the document to include this information.
>>
>>> I think we can flesh out this section a lot. We will discuss this in our
>>> team.
>>
>>
>> That sounds great! I realize the AIX part was a bit thin, but at least it is
>> mentioned, as compared to the old document. :)
>>
>> I'm leaving for vacation now, but I'll be happy to discuss with you and your
>> team how to improve the documentation for AIX/PPC in JDK 10.
>>
>
> Great stuff Magnus!
>
> Happy midsummer and happy holidays :)
>
>> /Magnus
>>
>>>
>>> Kind Regards, Thomas
>>>
>>>
>>> On Wed, Jun 21, 2017 at 2:46 AM, Magnus Ihse Bursie
>>> mailto:magnus.ihse.bur...@oracle.com>>
>>> wrote:
>>>
>>> Also known as: "!!! THIS IS A MAJOR RE-WRITE of this
>>> document. !" :-)
>>>
>>> The build documentation has been in need of an overhaul for a very
>>> long time. The document is not very well structured, and contains
>>> not only outdated but also irrelevant information, all the while
>>> valuable information is missing.
>>>
>>> For some time now, I've worked as a background project to rewrite
>>> the build documentation to address these shortcomings. Now it's
>>> finally done.
>>>
>>> Here is the new documentation in html format:
>>>
>>> http://cr.openjdk.java.net/~ihse/demo-new-build-readme/common/doc/building.html
>>>
>>> 
>>>
>>> And here it is in markdown:
>>>
>>> http://cr.openjdk.java.net/~ihse/JDK-8179892-update-build-readme/webrev.01/raw_files/new/common/doc/building.md
>>>
>>> 
>>>
>>> I've tried to make the document as readable as possible in raw
>>> markdown, hoping it can be of assistance for command-line users.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8179892
>>> 
>>> WebRev:
>>>
>>> http://cr.openjdk.java.net/~ihse/JDK-8179892-update-build-readme/webrev.01
>>>
>>> 
>>>
>>> /Magnus
>>>
>>>
>>