-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8276799: Implementation of JEP 422: Linux/RISC-V Port #6294
Conversation
👋 Welcome back fyang! A progress list of the required criteria for merging this PR into |
@RealFYang The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
/contributor add Yadong Wang yadonn.wang@huawei.com |
@RealFYang |
@RealFYang |
@RealFYang |
@RealFYang |
@RealFYang |
@RealFYang |
@RealFYang |
@RealFYang |
a19a1ff
to
2f89b96
Compare
This comment has been minimized.
This comment has been minimized.
@RealFYang |
2f89b96
to
668d7ef
Compare
668d7ef
to
a68980f
Compare
This comment has been minimized.
This comment has been minimized.
@RealFYang |
a68980f
to
299b3ee
Compare
299b3ee
to
00c43fc
Compare
@RealFYang This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Rebased to master |
00c43fc
to
ced5ef4
Compare
@RealFYang This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
/reviewers 3 reviewer |
@magicus Usage: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Thanks again for looking at the build changes :-) |
Hi David, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test/jdk files look ok. (I didn't look at the rest)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked on C1/C2 changes and compiler tests. Seems reasonable.
But before approval I would need to run changes through our testing.
Thank you for looking at that part. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update looks good.
Testing results are also good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay to me.
/integrate |
@RealFYang Pushed as commit 5905b02. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
libffi offers to APIs for preparing a native call: https://github.com/libffi/libffi/blob/5dcb741f1544c5e18c9dbf96aeb8b61cc556a616/src/prep_cif.c#L221-L235 The latter (`ffi_prep_cif_var`) is ought to be used for variadic functions. In practice we only ever used `ffi_prep_cif` as it didn't matter so far, because the underlying implementation only differs if `FFI_TARGET_SPECIFIC_VARIADIC` is defined: https://github.com/libffi/libffi/blob/5dcb741f1544c5e18c9dbf96aeb8b61cc556a616/src/prep_cif.c#L211-L215 And that wasn't relevant until darwin-aarch64: https://github.com/libffi/libffi/blob/5dcb741f1544c5e18c9dbf96aeb8b61cc556a616/src/aarch64/ffitarget.h#L79 Bonus: It's also needed for linux-riscv: https://github.com/libffi/libffi/blob/5dcb741f1544c5e18c9dbf96aeb8b61cc556a616/src/riscv/ffitarget.h#L66 OpenJDK support has landed recently, so we might care about linux-riscv "soon" ;-) openjdk/jdk#6294 ------------------------------------------------------------------------ The infrastructure to use `ffi_prep_cif_var` is actually there, but since we didn't include the ellipsis in the signature it didn't propgate down through the SignatureBuilder.
instruct cmpU_loop(cmpOpU cmp, iRegI op1, iRegI op2, label lbl) | ||
%{ | ||
// Same match rule as `far_cmpU_loop'. | ||
match(CountedLoopEnd cmp (CmpU op1 op2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which testcases can test this instruct and the following instructs?
match(CountedLoopEnd cmp (CmpP op1 op2));
match(CountedLoopEnd cmp (CmpN op1 op2));
match(CountedLoopEnd cmp (CmpF op1 op2));
match(CountedLoopEnd cmp (CmpD op1 op2));
I suspect this instruction is useless.
@magicus The command |
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
Progress
Issue
Reviewers
Contributors
<yadonn.wang@huawei.com>
<zhuyanhong2@huawei.com>
<jiangfeilong@huawei.com>
<wangkun49@huawei.com>
<nizhuxuan@huawei.com>
<guotaiping1@huawei.com>
<hekang6@huawei.com>
<shade@openjdk.org>
<yunyao.zxl@alibaba-inc.com>
<kuaiwei.kw@alibaba-inc.com>
<ihse@openjdk.org>
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/6294/head:pull/6294
$ git checkout pull/6294
Update a local copy of the PR:
$ git checkout pull/6294
$ git pull https://git.openjdk.java.net/jdk pull/6294/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 6294
View PR using the GUI difftool:
$ git pr show -t 6294
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/6294.diff