8357079: Fix Windows AArch64 DevKit Creation #25259
Conversation
|
Hi @Luigi96, welcome to this OpenJDK project and thanks for contributing! We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user Luigi96" as summary for the issue. If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing |
|
/covered |
|
@Luigi96 This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 612 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@erikj79) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated! |
bridgekeeper
bot
removed
oca
|
@Luigi96 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 issue a |
|
/touch |
|
@Luigi96 The pull request is being re-evaluated and the inactivity timeout has been reset. |
| if [ -d "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" ]; then | ||
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" $DEVKIT_ROOT/VC/bin/ | ||
| else | ||
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostx64/arm64" $DEVKIT_ROOT/VC/bin/ | ||
| fi |
There was a problem hiding this comment.
It seems to make sense to have a single devkit since there seem to be a lot of overlap and not much additional code added to a cross-compiling devkit to also have native binaries.
In fact, I could that not just be done as easily with something like:
| if [ -d "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" ]; then | |
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" $DEVKIT_ROOT/VC/bin/ | |
| else | |
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostx64/arm64" $DEVKIT_ROOT/VC/bin/ | |
| fi | |
| if [ -d "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" ]; then | |
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostarm64/arm64" $DEVKIT_ROOT/VC/bin/ | |
| fi | |
| if [ -d "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostx64/arm64" ]; then | |
| cp -r "$VS_INSTALL_DIR/${VC_SUBDIR}/bin/Hostx64/arm64" $DEVKIT_ROOT/VC/bin/ | |
| fi |
Then of course we need to pick up the right set of compilers from the devkit, but I guess that would sort itself out if we know the build platform and are given the target platform as configure argument..?
There was a problem hiding this comment.
I was thinking we could create the devkit based on the platform where the script is being run. For example, running createWindowsDevkit.sh on:
- Windows x64 would produce:
- Native x86 libraries
- Native x64 libraries
- Cross-compiled aarch64 libraries
- Windows aarch64 would produce:
- Native x86 libraries
- Native x64 libraries
- Native aarch64 libraries
From my analysis of your code suggestion, it looks like it might override the native aarch64 libraries and always set the cross-compiled ones. What do you think?
There was a problem hiding this comment.
I agree with your general premise, that the devkit should be created for the platform the devkit script is run on.
When you say that you should produce "native" x64 libraries on aarch64, do you mean that they should be run using x64 emulation? Otherwise you can't really have native x64 on aarch64... And even if that works, would it not be better to have pure native aarch64 cross-compiling to x64 (given that Microsoft in fact ships such compilers).
There was a problem hiding this comment.
When you download the Visual Studio Build Tools on an ARM64 machine, it downloads the native binaries for the x64 architecture under Hostx64 and the native binaries for arm64 under Hostarm64. This devkit script does not generate any binaries; it simply copies the Build Tools from Visual Studio.
There was a problem hiding this comment.
I find this confusing, but that's probably since I don't understand what Microsoft is shipping and where.
There are 4 relevant compilers:
- A compiler that runs on x64 and generates x64 code (x64 native compilation)
- A compiler that runs on x64 and generates aarch64 code (aarch64 cross-compilation on x64)
- A compiler that runs on aarch64 and generates aarch64 code (aarch64 native compilation)
- A compiler that runs on aarch64 and generates x64 code (x64 cross-compilation on aarch64)
For a devkit targeting x64, 1) and 2) should be included. For a devkit targeting aarch64, 3) (and ideally 4) as well) should be included.
Are you saying that if you download VS on aarch64, you will get all four compilers installed?
There was a problem hiding this comment.
Yes, if I download VS on aarch64 I get the four compilers installed.
There was a problem hiding this comment.
Is this the case when downloading VS on x64 as well, that you get a binary that can only be run on aarch64 bundled too? Or is the x64 installation just a subset of the aarch64 installation?
There was a problem hiding this comment.
When you download VS on x64 you just get cross-compilers for aarch64 (I don't know why). The Hostarm64 directory is not created in x64 machines.
There was a problem hiding this comment.
So if we run the devkit script on an aarch64 machine, we can create a devkit that will work on both x64 and aarch64? But if we run it on x64, the devkit that we create will only work on x64? Have I got this right?
If so, I think the script should support this, and make it clear what it does. That is, if you run it on aarch64, you get a multi-platform devkit, but on x64 you get a x64-only devkit. I think the devkit script should mention this, ideally in an echo statement, but a comment in the source code is acceptable. Like:
You are generating a devkit for x64 only. To include aarch64 as well, run the script on windows/aarch64 and Generating devkit for x64 and aarch64, respectively. Or something like that.
There was a problem hiding this comment.
I was planning adding something like this:
# Detect host architecture to determine devkit platform support
# Note: The devkit always includes x86, x64, and aarch64 libraries and tools
# The difference is in toolchain capabilities:
# - On x64|AMD64 hosts: aarch64 tools are cross-compilation tools (Hostx64/arm64)
# - On aarch64|ARMv8 hosts: aarch64 tools are native tools (Hostarm64/arm64)
HOST_ARCH=`echo $PROCESSOR_IDENTIFIER`
case $HOST_ARCH in
AMD64)
echo "Running on x64 host - generating devkit with native x86/x64 tools and cross-compiled aarch64 tools."
echo "For native aarch64 compilation tools, run this script on a Windows/aarch64 machine."
SUPPORTED_PLATFORMS="x86, x64 (native) and aarch64 (cross-compiled)"
;;
ARMv8)
echo "Running on aarch64 host - generating devkit with native tools for all platforms (x86, x64, aarch64)."
SUPPORTED_PLATFORMS="x86, x64, and aarch64 (all native)"
;;
*)
echo "Unknown host architecture: $HOST_ARCH"
echo "Proceeding with devkit generation - toolchain capabilities may vary."
SUPPORTED_PLATFORMS="x86, x64, and aarch64"
;;
esac
Fix path for Visual Studio arm64 binaries in createWindowsDevkit.sh
The DevKit currently created on a Windows AArch64 machine uses the x64 compiler binaries (e.g. cl.exe). This can be inspected by running "dumpbin /headers /out:cl.txt cl.exe" on the compiler in the generated DevKit and inspecting the machine architecture line. It should instead use the native Windows AArch64 compiler binaries
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25259/head:pull/25259$ git checkout pull/25259Update a local copy of the PR:
$ git checkout pull/25259$ git pull https://git.openjdk.org/jdk.git pull/25259/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25259View PR using the GUI difftool:
$ git pr show -t 25259Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25259.diff
Using Webrev
Link to Webrev Comment