Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-22 Thread David Holmes
On Tue, 22 Mar 2022 12:08:01 GMT, Fei Yang  wrote:

>> make/autoconf/libraries.m4 line 152:
>> 
>>> 150:   fi
>>> 151: 
>>> 152:   # Programs which use C11 or C++11 atomics, like #include ,
>> 
>> Use of C++ atomics is not allowed in hotspot code base. See the style guide:
>> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md
>> 
>> That said, I don't see any actual use of C++ atomics. ??
>
> I think the old code comment here is a bit too general. It does not mean we 
> introduce any use of C++ atomics here.
> The fact is that RISC-V only has word-sized atomics, it requries libatomic 
> where other common architectures do not [1].
> So atomic support would require explicit linking against -latomic on RISC-V. 
> Otherwise we got build errors like:

New comment looks good - thanks for clarifying.

-

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


Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-22 Thread Fei Yang
On Tue, 22 Mar 2022 04:13:17 GMT, David Holmes  wrote:

>> Fei Yang has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains two additional commits 
>> since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into JDK-8276799
>>  - 8276799: Implementation of JEP 422: Linux/RISC-V Port
>
> make/autoconf/libraries.m4 line 152:
> 
>> 150:   fi
>> 151: 
>> 152:   # Programs which use C11 or C++11 atomics, like #include ,
> 
> Use of C++ atomics is not allowed in hotspot code base. See the style guide:
> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md
> 
> That said, I don't see any actual use of C++ atomics. ??

I think the old code comment here is a bit too general. It does not mean we 
introduce any use of C++ atomics here.
The fact is that RISC-V only has word-sized atomics, it requries libatomic 
where other common architectures do not [1].
So atomic support would require explicit linking against -latomic on RISC-V. 
Otherwise we got build errors like:

-

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


Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-22 Thread Fei Yang
On Tue, 22 Mar 2022 05:12:46 GMT, David Holmes  wrote:

> Hi,
> 
> I've looked at everything that is not a RISC-V specific file, except for the 
> C1 changes as the compiler folk will need to approve those.
> 
> Some copyrights will need updating to 2022 on the Oracle copyright line 
> please.

Hi David,
  I have pushed one more commit updating the Oralce copyright line for existing 
files touched.
  Thanks for looking at this.

-

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


Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-22 Thread Fei Yang
On Tue, 22 Mar 2022 03:31:16 GMT, Fei Yang  wrote:

>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the 
>> https://openjdk.java.net/projects/riscv-port branch.
>> 
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are 
>> also carried out regularly. So it should be good enough to run most Java 
>> programs.
>> 
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains two additional commits since 
> the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into JDK-8276799
>  - 8276799: Implementation of JEP 422: Linux/RISC-V Port

> Build changes look good. I can't say anything about the rest of the code.
> 
> /reviewers 3

Thanks again for looking at the build changes :-)

-

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


Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-21 Thread David Holmes
On Tue, 22 Mar 2022 03:31:16 GMT, Fei Yang  wrote:

>> This PR implements JEP 422: Linux/RISC-V Port [1].
>> The PR starts as a squashed merge of the 
>> https://openjdk.java.net/projects/riscv-port branch.
>> 
>> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
>> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are 
>> also carried out regularly. So it should be good enough to run most Java 
>> programs.
>> 
>> [1] https://openjdk.java.net/jeps/422
>
> Fei Yang has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains two additional commits since 
> the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into JDK-8276799
>  - 8276799: Implementation of JEP 422: Linux/RISC-V Port

Hi,

I've looked at everything that is not a RISC-V specific file, except for the C1 
changes as the compiler folk will need to approve those.

Some copyrights will need updating to 2022 on the Oracle copyright line please.

I have flagged one issue in regard to C++ atomics - see below.

Thanks,
David

make/autoconf/libraries.m4 line 152:

> 150:   fi
> 151: 
> 152:   # Programs which use C11 or C++11 atomics, like #include ,

Use of C++ atomics is not allowed in hotspot code base. See the style guide:
https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

That said, I don't see any actual use of C++ atomics. ??

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v2]

2022-03-21 Thread Fei Yang
> This PR implements JEP 422: Linux/RISC-V Port [1].
> The PR starts as a squashed merge of the 
> https://openjdk.java.net/projects/riscv-port branch.
> 
> This has been tested with jtreg tier{1,2,3,4} and jcstress on HiFive 
> Unmatched board. Dacapo, SPECjbb2015 and SPECjvm2008 benchmark tests are also 
> carried out regularly. So it should be good enough to run most Java programs.
> 
> [1] https://openjdk.java.net/jeps/422

Fei Yang has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains two additional commits since the 
last revision:

 - Merge remote-tracking branch 'upstream/master' into JDK-8276799
 - 8276799: Implementation of JEP 422: Linux/RISC-V Port

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6294/files
  - new: https://git.openjdk.java.net/jdk/pull/6294/files/33402035..a144cd20

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6294=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6294=00-01

  Stats: 2517 lines in 698 files changed: 1371 ins; 865 del; 281 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6294.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294

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