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

2020-11-03 Thread Andrew Haley 
On 02/11/2020 18:37, Bernhard Urban-Forster wrote:
> @theRealAph what gcc version?
> 
> I can reproduce with
> $ gcc --version
> gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008
> which ships in Ubuntu 19.10 as default

My mistake: I think it only triggers with a debug build, because assert
is a macro.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

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

2020-11-02 Thread Bernhard Urban-Forster 
On Mon, 2 Nov 2020 17:43:31 GMT, Andrew Haley  wrote:

>> https://github.com/openjdk/jdk/pull/1013
>
>> @lewurm
>> This patch seems to break on linux-aarch64 with gcc:
> 
> Builds cleanly on Linux/GCC or me.

@theRealAph what gcc version?

I can reproduce with
$ gcc --version
gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008
which ships in Ubuntu 19.10 as default

-

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

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

2020-11-02 Thread Andrew Haley 
On Mon, 2 Nov 2020 17:05:19 GMT, Bernhard Urban-Forster  
wrote:

>> @lewurm Open a new JBS issue with the bug. If you can find a fix in a short 
>> amount of time (which I would believe should be possible; probably just need 
>> a proper cast) it's acceptable to fix it directly. What amounts to a "short 
>> amount of time" is left to reasonable judgement; minutes to hours are okay, 
>> days are not.
>> 
>> Otherwise, create an anti-delta (revert changeset) to back out your changes, 
>> and open yet another JBS issue for re-implementing them correctly.  
>> 
>>  In this case, an alternative short-term fix could also be to remove the 
>> assert instead of backing out the entire patch.
>
> https://github.com/openjdk/jdk/pull/1013

> @lewurm
> This patch seems to break on linux-aarch64 with gcc:

Builds cleanly on Linux/GCC or me.

-

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

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

2020-11-02 Thread Bernhard Urban-Forster 
On Mon, 2 Nov 2020 16:16:25 GMT, Magnus Ihse Bursie  wrote:

>> @magicus I did test the initial version of this PR on linux+arm64, but not 
>> the latest iteration. sorry about that  
>> 
>> What is the policy here? Submit a revert right away or investigate a fix?
>
> @lewurm Open a new JBS issue with the bug. If you can find a fix in a short 
> amount of time (which I would believe should be possible; probably just need 
> a proper cast) it's acceptable to fix it directly. What amounts to a "short 
> amount of time" is left to reasonable judgement; minutes to hours are okay, 
> days are not.
> 
> Otherwise, create an anti-delta (revert changeset) to back out your changes, 
> and open yet another JBS issue for re-implementing them correctly.  
> 
>  In this case, an alternative short-term fix could also be to remove the 
> assert instead of backing out the entire patch.

https://github.com/openjdk/jdk/pull/1013

-

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

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

2020-11-02 Thread Magnus Ihse Bursie 
On Mon, 2 Nov 2020 16:06:15 GMT, Bernhard Urban-Forster  
wrote:

>> @lewurm 
>> This patch seems to break on linux-aarch64 with gcc:
>> open/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp:1501:52: error: 
>> comparison of integer expressions of different signedness: 'size_t' {aka 
>> 'long unsigned int'} and 'int' [-Werror=sign-compare]
>>  1501 | assert(StackOverflow::stack_shadow_zone_size() == 
>> (int)StackOverflow::stack_shadow_zone_size(), "must be same");
>>   |
>> ^~~
>> 
>> Did you test building this on any aarch64 platforms besides Windows?
>
> @magicus I did test the initial version of this PR on linux+arm64, but not 
> the latest iteration. sorry about that  
> 
> What is the policy here? Submit a revert right away or investigate a fix?

@lewurm Open a new JBS issue with the bug. If you can find a fix in a short 
amount of time (which I would believe should be possible; probably just need a 
proper cast) it's acceptable to fix it directly. What amounts to a "short 
amount of time" is left to reasonable judgement; minutes to hours are okay, 
days are not.

Otherwise, create an anti-delta (revert changeset) to back out your changes, 
and open yet another JBS issue for re-implementing them correctly.  

 In this case, an alternative short-term fix could also be to remove the assert 
instead of backing out the entire patch.

-

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

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

2020-11-02 Thread Bernhard Urban-Forster 
On Mon, 2 Nov 2020 15:41:06 GMT, Magnus Ihse Bursie  wrote:

>> Thank you Andrew.
>
> @lewurm 
> This patch seems to break on linux-aarch64 with gcc:
> open/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp:1501:52: error: 
> comparison of integer expressions of different signedness: 'size_t' {aka 
> 'long unsigned int'} and 'int' [-Werror=sign-compare]
>  1501 | assert(StackOverflow::stack_shadow_zone_size() == 
> (int)StackOverflow::stack_shadow_zone_size(), "must be same");
>   |
> ^~~
> 
> Did you test building this on any aarch64 platforms besides Windows?

@magicus I did test the initial version of this PR on linux+arm64, but not the 
latest iteration. sorry about that  

What is the policy here? Submit a revert right away or investigate a fix?

-

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

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

2020-11-02 Thread Magnus Ihse Bursie 
On Mon, 2 Nov 2020 14:00:33 GMT, Bernhard Urban-Forster  
wrote:

>>> Would you mind to sponsor it @theRealAph or @magicus?
>> 
>> Hmm, I think you have to integrate it first.
>> https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/sponsor
>
> Thank you Andrew.

@lewurm 
This patch seems to break on linux-aarch64 with gcc:
open/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp:1501:52: error: 
comparison of integer expressions of different signedness: 'size_t' {aka 'long 
unsigned int'} and 'int' [-Werror=sign-compare]
 1501 | assert(StackOverflow::stack_shadow_zone_size() == 
(int)StackOverflow::stack_shadow_zone_size(), "must be same");
  |
^~~

Did you test building this on any aarch64 platforms besides Windows?

-

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

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

2020-11-02 Thread Bernhard Urban-Forster 
On Mon, 2 Nov 2020 13:41:53 GMT, Andrew Haley  wrote:

>> Marked as reviewed by aph (Reviewer).
>
>> Would you mind to sponsor it @theRealAph or @magicus?
> 
> Hmm, I think you have to integrate it first.
> https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/sponsor

Thank you Andrew.

-

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

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

2020-11-02 Thread Andrew Haley 
On Tue, 27 Oct 2020 14:04:04 GMT, Andrew Haley  wrote:

>> Bernhard Urban-Forster has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - uppercase suffix
>>  - add assert
>
> Marked as reviewed by aph (Reviewer).

> Would you mind to sponsor it @theRealAph or @magicus?

Hmm, I think you have to integrate it first.
https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/sponsor

-

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

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

2020-10-29 Thread Bernhard Urban-Forster 
On Tue, 27 Oct 2020 14:04:04 GMT, Andrew Haley  wrote:

>> Bernhard Urban-Forster has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - uppercase suffix
>>  - add assert
>
> Marked as reviewed by aph (Reviewer).

Would you mind sponsor it @theRealAph or @magicus?

-

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

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

2020-10-27 Thread Bernhard Urban-Forster 
On Tue, 27 Oct 2020 14:04:04 GMT, Andrew Haley  wrote:

>> Bernhard Urban-Forster has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - uppercase suffix
>>  - add assert
>
> Marked as reviewed by aph (Reviewer).

Thank you for the reviews, Magnus and Andrew!

-

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

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

2020-10-27 Thread Andrew Haley 
On Thu, 15 Oct 2020 18:35:30 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 incrementally with two 
> additional commits since the last revision:
> 
>  - uppercase suffix
>  - add assert

Marked as reviewed by aph (Reviewer).

-

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

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

2020-10-27 Thread Bernhard Urban-Forster 
On Sun, 18 Oct 2020 09:07:17 GMT, Magnus Ihse Bursie  wrote:

>> Bernhard Urban-Forster has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - uppercase suffix
>>  - add assert
>
> Build changes look fine now.

@theRealAph does the PR look okay to you now?

-

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

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

2020-10-18 Thread Magnus Ihse Bursie 
On Thu, 15 Oct 2020 18:35:30 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 incrementally with two 
> additional commits since the last revision:
> 
>  - uppercase suffix
>  - add assert

Build changes look fine now.

-

Marked as reviewed by ihse (Reviewer).

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

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

2020-10-15 Thread Bernhard Urban-Forster 
> 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 incrementally with two 
additional commits since the last revision:

 - uppercase suffix
 - add assert

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/530/files
  - new: https://git.openjdk.java.net/jdk/pull/530/files/32e922da..901bbd48

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=530=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=530=02-03

  Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/530.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/530/head:pull/530

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