Skip to content
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

8199138: Add RISC-V support to Zero #573

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DingliZhang
Copy link
Member

@DingliZhang DingliZhang commented Sep 3, 2024

Hi all,

I'd like to backport this patch to jdk8u. Since most linux distributions support riscv64 and provide zero version jdk8 for riscv64. However, we may need to do some workarounds each time we upgrade the version, so I think it is helpful to provide zero support for riscv64 here.

common/autoconf/build-aux/config.guess and hotspot/src/os/linux/vm/os_linux.cpp do not apply cleanly due to context difference, but it is easy to resolve them manually. common/autoconf/platform.m4 just changed file path and remove test of x$OPENJDK_$1_CPU.

common/autoconf/generated-configure.sh regenerated by bash common/autoconf/autogen.sh. I used autoconf-2.69 and build from the source . But it looks a bit different from the one generated in the repo. I also tried autoconf-2.69 that comes with ubuntu 20.04 and the generated file is similar to the one in #413. I'm not quite sure which version of autoconf2.69 to use, if anyone has a better suggestion or is willing to help me generate a different one, I'd appreciate it!

Both cross-compile build and native build on riscv64 hardware is tested.

cross-compile on x86

boot jdk

openjdk version "1.8.0_43"
OpenJDK Runtime Environment (build 1.8.0_43-b03)
OpenJDK 64-Bit Server VM (build 25.40-b25, mixed mode)

run on qemu

openjdk version "1.8.0_432-internal"
OpenJDK Runtime Environment (build 1.8.0_432-internal-zhangdingli_2024_09_02_21_37-b00)
OpenJDK 64-Bit Zero VM (build 25.432-b00, interpreted mode)

native build on lp4a

boot jdk

openjdk version "1.8.0_412"
OpenJDK Runtime Environment Bisheng (build 1.8.0_412-b08)
OpenJDK 64-Bit Zero VM Bisheng (build 25.412-b08, interpreted mode)

run on lp4a

openjdk version "1.8.0_432-internal"
OpenJDK Runtime Environment (build 1.8.0_432-internal-openeuler_2024_09_02_21_38-b00)
OpenJDK 64-Bit Zero VM (build 25.432-b00, interpreted mode)

Thanks,
Dingli


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8199138 needs maintainer approval

Issue

  • JDK-8199138: Add RISC-V support to Zero (Bug - P4 - Requested)

Reviewers

Contributors

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/573/head:pull/573
$ git checkout pull/573

Update a local copy of the PR:
$ git checkout pull/573
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/573/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 573

View PR using the GUI difftool:
$ git pr show -t 573

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/573.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 3, 2024

👋 Welcome back dzhang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Sep 3, 2024

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot changed the title Backport edc4ffe54b8c1f042e98c0ddb9dce757d88b01f4 8199138: Add RISC-V support to Zero Sep 3, 2024
@openjdk
Copy link

openjdk bot commented Sep 3, 2024

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr Pull request is ready for review labels Sep 3, 2024
@mlbridge
Copy link

mlbridge bot commented Sep 3, 2024

Webrevs

@DingliZhang
Copy link
Member Author

DingliZhang commented Sep 3, 2024

Thanks to Gui Cao for the original patch.
/contributor add gcao

@openjdk
Copy link

openjdk bot commented Sep 3, 2024

@DingliZhang
Contributor Gui Cao <[email protected]> successfully added.

@openjdk openjdk bot added the approval label Sep 3, 2024
@DingliZhang
Copy link
Member Author

/approval request allow Zero build on RISC-V

@openjdk
Copy link

openjdk bot commented Sep 3, 2024

@DingliZhang
8199138: The approval request has been created successfully.

Copy link
Member

@RealFYang RealFYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I have only one minor comment. Thanks.

@@ -1949,6 +1949,9 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen)
#ifndef EM_AARCH64
#define EM_AARCH64 183 /* ARM AARCH64 */
#endif
#ifndef EM_RISCV
#define EM_RISCV 243 /* RISC-V */
#endif
Copy link
Member

@RealFYang RealFYang Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be more consistent if you put this after definition of EM_LOONGARCH like you do for the other places.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! Fixed.

Copy link
Member

@RealFYang RealFYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. I am not sure about the autoconf issue here. You need to ask for the maintainer's comments/suggestions (@gnu-andrew once mentioned the possibility of backporting https://bugs.openjdk.org/browse/JDK-8195689 on #413) to remove the generated configure at the start of the next release cycle). The rest of the change looks good to me (Not a 8u Reviewer).

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 1, 2024

@DingliZhang This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 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!

@gnu-andrew
Copy link
Member

@DingliZhang This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 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!

Keep this open. I'll review after the upcoming release on the 15th.

@gnu-andrew
Copy link
Member

Thanks for the update. I am not sure about the autoconf issue here. You need to ask for the maintainer's comments/suggestions (@gnu-andrew once mentioned the possibility of backporting https://bugs.openjdk.org/browse/JDK-8195689 on #413) to remove the generated configure at the start of the next release cycle). The rest of the change looks good to me (Not a 8u Reviewer).

Yes, the in-tree auto-generated configure is a recurring problem. I certainly would like to backport the other fix. I'll have a look when I review this as to whether we should wait on that here or just commit this as is.

@DingliZhang
Copy link
Member Author

Thanks for the update. I am not sure about the autoconf issue here. You need to ask for the maintainer's comments/suggestions (@gnu-andrew once mentioned the possibility of backporting https://bugs.openjdk.org/browse/JDK-8195689 on #413) to remove the generated configure at the start of the next release cycle). The rest of the change looks good to me (Not a 8u Reviewer).

Yes, the in-tree auto-generated configure is a recurring problem. I certainly would like to backport the other fix. I'll have a look when I review this as to whether we should wait on that here or just commit this as is.

Thanks for the reply, I'm looking forward to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval backport rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

3 participants