-
Notifications
You must be signed in to change notification settings - Fork 243
8248238: Implementation: JEP 388: Windows AArch64 Support #301
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
Changes from all commits
3d94fcb
c58ab58
697c339
d33ac76
6792a10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -185,12 +185,14 @@ AC_DEFUN([FLAGS_SETUP_LDFLAGS_CPU_DEP], | |
$1_CPU_LDFLAGS_JVM_ONLY="-xarch=sparc" | ||
fi | ||
|
||
elif test "x$TOOLCHAIN_TYPE" = xmicrosoft; then | ||
elif test "x$TOOLCHAIN_TYPE" = xmicrosoft; then | ||
if test "x${OPENJDK_$1_CPU}" = "xx86"; then | ||
$1_CPU_LDFLAGS="-safeseh" | ||
# NOTE: Old build added -machine. Probably not needed. | ||
$1_CPU_LDFLAGS_JVM_ONLY="-machine:I386" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if the flag might not be needed, is it really prudent to make non-required changes in an update release? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, it is not prudent to do this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched back to the old version, just adding an aarch64-specific branch in c0073da. |
||
$1_CPU_EXECUTABLE_LDFLAGS="-stack:327680" | ||
elif test "x${OPENJDK_$1_CPU}" = "xaarch64"; then | ||
$1_CPU_EXECUTABLE_LDFLAGS="-stack:1048576" | ||
else | ||
$1_CPU_LDFLAGS_JVM_ONLY="-machine:AMD64" | ||
$1_CPU_EXECUTABLE_LDFLAGS="-stack:1048576" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -918,14 +918,18 @@ AC_DEFUN_ONCE([TOOLCHAIN_SETUP_BUILD_COMPILERS], | |
. $CONFIGURESUPPORT_OUTPUTDIR/build-devkit.info | ||
# This potentially sets the following: | ||
# A descriptive name of the devkit | ||
BASIC_EVAL_DEVKIT_VARIABLE([BUILD_DEVKIT_NAME]) | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_NAME]) | ||
# Corresponds to --with-extra-path | ||
BASIC_EVAL_DEVKIT_VARIABLE([BUILD_DEVKIT_EXTRA_PATH]) | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_EXTRA_PATH]) | ||
# Corresponds to --with-toolchain-path | ||
BASIC_EVAL_DEVKIT_VARIABLE([BUILD_DEVKIT_TOOLCHAIN_PATH]) | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_TOOLCHAIN_PATH]) | ||
# Corresponds to --with-sysroot | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or these changes. What have they to do with AArch64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's required to make cross-compilation work on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. |
||
BASIC_EVAL_DEVKIT_VARIABLE([BUILD_DEVKIT_SYSROOT]) | ||
# Skip the Window specific parts | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_SYSROOT]) | ||
|
||
if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_VS_INCLUDE]) | ||
BASIC_EVAL_BUILD_DEVKIT_VARIABLE([BUILD_DEVKIT_VS_LIB]) | ||
fi | ||
fi | ||
|
||
AC_MSG_CHECKING([for build platform devkit]) | ||
|
@@ -935,13 +939,22 @@ AC_DEFUN_ONCE([TOOLCHAIN_SETUP_BUILD_COMPILERS], | |
AC_MSG_RESULT([$BUILD_DEVKIT_ROOT]) | ||
fi | ||
|
||
BUILD_SYSROOT="$BUILD_DEVKIT_SYSROOT" | ||
PATH="$BUILD_DEVKIT_EXTRA_PATH:$PATH" | ||
|
||
# Fallback default of just /bin if DEVKIT_PATH is not defined | ||
# Fallback default of just /bin if DEVKIT_PATH is not defined | ||
if test "x$BUILD_DEVKIT_TOOLCHAIN_PATH" = x; then | ||
BUILD_DEVKIT_TOOLCHAIN_PATH="$BUILD_DEVKIT_ROOT/bin" | ||
fi | ||
PATH="$BUILD_DEVKIT_TOOLCHAIN_PATH:$BUILD_DEVKIT_EXTRA_PATH" | ||
PATH="$BUILD_DEVKIT_TOOLCHAIN_PATH:$PATH" | ||
|
||
BUILD_SYSROOT="$BUILD_DEVKIT_SYSROOT" | ||
|
||
if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then | ||
BUILD_VS_INCLUDE="$BUILD_DEVKIT_VS_INCLUDE" | ||
BUILD_VS_LIB="$BUILD_DEVKIT_VS_LIB" | ||
|
||
TOOLCHAIN_SETUP_VISUAL_STUDIO_SYSROOT_FLAGS([BUILD_]) | ||
fi | ||
fi | ||
fi | ||
|
||
|
@@ -967,9 +980,37 @@ AC_DEFUN_ONCE([TOOLCHAIN_SETUP_BUILD_COMPILERS], | |
BASIC_FIXUP_EXECUTABLE(BUILD_STRIP) | ||
# Assume the C compiler is the assembler | ||
BUILD_AS="$BUILD_CC -c" | ||
# Just like for the target compiler, use the compiler as linker | ||
BUILD_LD="$BUILD_CC" | ||
BUILD_LDCXX="$BUILD_CXX" | ||
if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then | ||
# In the Microsoft toolchain we have a separate LD command "link". | ||
# Make sure we reject /usr/bin/link (as determined in CYGWIN_LINK), which is | ||
# a cygwin program for something completely different. | ||
AC_CHECK_PROG([BUILD_LD], [link$EXE_SUFFIX],[link$EXE_SUFFIX],,, [$CYGWIN_LINK]) | ||
BASIC_FIXUP_EXECUTABLE(BUILD_LD) | ||
# Verify that we indeed succeeded with this trick. | ||
AC_MSG_CHECKING([if the found link.exe is actually the Visual Studio linker]) | ||
|
||
# Reset PATH since it can contain a mix of WSL/linux paths and Windows paths from VS, | ||
# which, in combination with WSLENV, will make the WSL layer complain | ||
old_path="$PATH" | ||
PATH= | ||
|
||
"$BUILD_LD" --version > /dev/null | ||
|
||
if test $? -eq 0 ; then | ||
AC_MSG_RESULT([no]) | ||
AC_MSG_ERROR([This is the Cygwin link tool. Please check your PATH and rerun configure.]) | ||
else | ||
AC_MSG_RESULT([yes]) | ||
fi | ||
|
||
PATH="$old_path" | ||
|
||
BUILD_LDCXX="$BUILD_LD" | ||
else | ||
# Just like for the target compiler, use the compiler as linker | ||
BUILD_LD="$BUILD_CC" | ||
BUILD_LDCXX="$BUILD_CXX" | ||
fi | ||
|
||
PATH="$OLDPATH" | ||
|
||
|
@@ -1025,6 +1066,10 @@ AC_DEFUN_ONCE([TOOLCHAIN_MISC_CHECKS], | |
if test "x$COMPILER_CPU_TEST" != "xx64"; then | ||
AC_MSG_ERROR([Target CPU mismatch. We are building for $OPENJDK_TARGET_CPU but CL is for "$COMPILER_CPU_TEST"; expected "x64".]) | ||
fi | ||
elif test "x$OPENJDK_TARGET_CPU" = "xaarch64"; then | ||
if test "x$COMPILER_CPU_TEST" != "xARM64"; then | ||
AC_MSG_ERROR([Target CPU mismatch. We are building for $OPENJDK_TARGET_CPU but CL is for "$COMPILER_CPU_TEST"; expected "arm64".]) | ||
fi | ||
fi | ||
fi | ||
|
||
|
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 don't really understand why any of these changes are necessary.
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.
This is hack to make cross-compilation on Windows work. On tip it's much cleaner thanks to the WINENV work that Magnus did here: openjdk/jdk@d29c78d However backporting this to 11 would be much riskier imho, as it touches a lot of things in the build system. Also it's by far not a clean backport; I attempted a backport myself and gave up after a couple of frustrating days. Therefore I suggest to go with what we have here, even though it has an uglierer usage.
I described how to use it here: https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2021-May/012724.html
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 how are you supposed to get the fixpath.exe on windows/aarch64?
On the surface, this backport seem to have the same problem as your original win/aarch64 patch for mainline, that it is not generally possible to build it without a lot of external hacks. I can only point this out; I'll have to leave it to the jdk11u maintainers to accept or drop the patch given this knowledge.
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.
Auto-testing is always going to be a pain as long as the port isn't bootstrappable because there aren't native tools, right?
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.
If you do not want to download an arbitrary
.exe
file as described mailing list post linked above (which I totally get), you can just do a build for Windows/x86 and copy over thefixpath.exe
that was generated by that.I understand having hacks around the build system issues is less than ideal. Ideally we would backport your WINENV patch to 11, but that would be much more invasive in terms of risk managment. The proposed solution in this PR does at least barely touch other platforms, so the risk for breaking something is rather low imho.