-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8357079: Fix Windows AArch64 DevKit Creation #25259
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
Open
Luigi96
wants to merge
1
commit into
openjdk:master
Choose a base branch
from
Luigi96:win-aarch64-devkit
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking we could create the devkit based on the platform where the script is being run. For example, running
createWindowsDevkit.sh
on: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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you download the Visual Studio Build Tools on an ARM64 machine, it downloads the native binaries for the
x64
architecture underHostx64
and the native binaries forarm64
underHostarm64
. This devkit script does not generate any binaries; it simply copies the Build Tools from Visual Studio.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 find this confusing, but that's probably since I don't understand what Microsoft is shipping and where.
There are 4 relevant compilers:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if I download VS on aarch64 I get the four compilers installed.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
andGenerating devkit for x64 and aarch64
, respectively. Or something like that.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 was planning adding something like this: