Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Fri, 12 Feb 2021 12:22:09 GMT, Vladimir Kempik wrote: >> Where did this come from - some snippet/example/tech note code? Maybe other >> people can help figure it out if we provide more info. > > This is the version of w^x on-demand switch implemented by microsoft guys. > This is enabled only for debug builds. > @lewurm could you comment here please Those values can be observed in the debugger, but aren't documented or defined in header files. This mode was useful for the initial bootstrap of the platform (it helped with missing W^X transitions), but shouldn't be required anymore today. I'm fine with removing it altogether. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 22:48:33 GMT, Gerard Ziemski wrote: >> I don't like the idea of using masks on architectures that do not require >> them. How about something like this? >> >> `#if defined(__APPLE__)` >> ` // lldb (gdb) installs both standard BSD signal handlers, and mach >> exception` >> ` // handlers. By replacing the existing task exception handler, we disable >> lldb's mach` >> ` // exception handling, while leaving the standard BSD signal handlers >> functional.` >> ` //` >> ` // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking` >> ` // EXC_MASK_ARITHMETIC needed by i386` >> ` // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization` >> ` kern_return_t kr;` >> ` kr = task_set_exception_ports(mach_task_self(),` >> `EXC_MASK_BAD_ACCESS` >> `NOT_LP64(| EXC_MASK_ARITHMETIC)` >> `AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION),` >> `MACH_PORT_NULL,` >> `EXCEPTION_STATE_IDENTITY,` >> `MACHINE_THREAD_STATE);` >> ` ` >> ` assert(kr == KERN_SUCCESS, "could not set mach task signal handler");` >> `#endif` >> >> If I just knew why i386 needs `EXC_MASK_ARITHMETIC` and add that to the >> comment I would be personally happy with that chunk of code. > > No idea how to insert spaces and make text align :-( using ` ```c ` https://docs.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks I was wrong about `SIGFPE` / `EXC_MASK_ARITHMETIC`, it's used on i386, x86_64: https://github.com/openjdk/jdk/blob/2be60e37e0e433141b2e3d3e32f8e638a4888e3a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp#L467-L524 and aarch64: https://github.com/AntonKozlov/jdk/blob/80827176cbc5f0dd26003cf234a8076f3f557928/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp#L309-L323 (What happened with the formatting here, ugh?) Your suggestion sounds good otherwise. @AntonKozlov, do you mind to integrate that? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:29:48 GMT, Gerard Ziemski wrote: >> Part of the comment said `This work-around is not necessary for 10.5+, as >> CrashReporter no longer intercedes on caught fatal signals.` so I thought it >> was no longer needed, but it sounds like the part about `gdb` still applies >> then. >> >> We should update the comment to just say the `gdb` relevant part perhaps >> (and evaluate which of the EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION | >> EXC_MASK_ARITHMETIC) we actually need for gdb: >> >> `// gdb installs both standard BSD signal handlers, and mach exception` >> `// handlers. By replacing the existing task exception handler, we disable >> gdb's mach` >> `// exception handling, while leaving the standard BSD signal handlers >> functional.` >> >> Do you know if this also apply to `lldb` or is it `gdb` only specific? How >> do you run `gdb` on macOS nowadays anyhow? > > To answer my own question, it seems that code is still needed on `x86_64` for > `lldb` with `EXC_MASK_BAD_ACCESS` or we keep tripping over `EXC_BAD_ACCESS` > > Remaining questions: > > a) why we need `EXC_MASK_ARITHMETIC` ? > b) we hit `signal SIGSEGV` in debugger even with the code in place, any way > to avoid that? > c) does `BSD aarch64` need only `EXC_MASK_BAD_INSTRUCTION` or does it need > `EXC_MASK_BAD_ACCESS` as well? > d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to > apply to `aarch64`? Thanks for your questions Gerard. > Part of the comment said This work-around is not necessary for 10.5+, as > CrashReporter no longer intercedes on caught fatal signals. That comment can probably be deleted since minversion is anyway 10.9 (and soon 10.12 https://github.com/openjdk/jdk/pull/2268 ). > Do you know if this also apply to lldb or is it gdb only specific? How do you > run gdb on macOS nowadays anyhow? `lldb` is shipped with Xcode, `gdb` isn't. You would need to build and sign it yourself, I haven't tried that in a while. So, we should update that comment to talk about `lldb` > a) why we need `EXC_MASK_ARITHMETIC` ? I _believe_ this dates back to i386. As far as I can tell this isn't needed for x86_64 or aarch64. I guess we can remove it, the worst case is that it makes the debugging experience of the runtime a little bit worse. OTOH it doesn't hurt either to have it here. > b) we hit signal SIGSEGV in debugger even with the code in place, any way to > avoid that? The equivalent for `handle SIGSEGV nostop noprint` (gdb) in lldb is `process handle -n false -p true -s false SIGSEGV`. > c) does `BSD aarch6` need only `EXC_MASK_BAD_INSTRUCTION` or does it need > `EXC_MASK_BAD_ACCESS` as well? aarch64 needs `EXC_MASK_BAD_ACCESS` at least for implicit null checking, there might be other cases. > d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to > apply to `aarch64`? Maybe. I don't see any value in it though, except making the code more complicated to read - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 18:23:04 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/os/posix/signals_posix.cpp line 1297: > >> 1295: kern_return_t kr; >> 1296: kr = task_set_exception_ports(mach_task_self(), >> 1297: EXC_MASK_BAD_ACCESS | >> EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC, > > Could someone elaborate on why we need to add `EXC_MASK_BAD_INSTRUCTION` to > the mask here? See comment above about `gdb`, the same applies to `lldb` today. The AArch64 backend uses `SIGILL` (~= `EXC_MASK_BAD_INSTRUCTION`) to initiate a deoptimization. Without this change you cannot continue debugging once you the debuggee receives `SIGILL`. This wasn't needed before as x86 doesn't use `SIGILL`. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 17:43:35 GMT, Phil Race wrote:
>> Anton Kozlov has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Address feedback for signature generators
>> - Enable -Wformat-nonliteral back
>
> src/java.desktop/share/native/libharfbuzz/hb-coretext.cc line 193:
>
>> 191:* crbug.com/549610 */
>> 192: // 0x0007 stands for "kCTVersionNumber10_10", see CoreText.h
>> 193: #if TARGET_OS_OSX && MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10
>
> I need a robust explanation of these changes.
> They are not mentioned, nor are they in upstream harfbuzz.
> Likely these should be pushed to upstream first if there is a reason for them.
I'm afraid it's one of those dirty changes that we did to make progress, but
forgot to revisit later. Without the guard you would get:
* For target support_native_java.desktop_libharfbuzz_hb-coretext.o:
if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
^
uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0),
tvos(9.0, 14.0));
^
if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
^
uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0),
tvos(9.0, 14.0));
^
2 errors generated.
For now, it's probably better to go with what Anton did in the latest commit
https://github.com/openjdk/jdk/pull/2200/commits/b2b396fca679fbc7ee78fb5bc80191bc79e1c490
Eventually upstream will do something about it I assume? How often does it get
synced with upstream? Maybe worth to file an issue.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 16:07:19 GMT, Vladimir Kempik wrote: >> src/java.desktop/share/native/libharfbuzz/hb-common.h line 113: >> >>> 111: >>> 112: #define HB_TAG(c1,c2,c3,c4) >>> ((hb_tag_t)uint32_t)(c1)&0xFF)<<24)|(((uint32_t)(c2)&0xFF)<<16)|(((uint32_t)(c3)&0xFF)<<8)|((uint32_t)(c4)&0xFF))) >>> 113: #define HB_UNTAG(tag) (char)(((tag)>>24)&0xFF), >>> (char)(((tag)>>16)&0xFF), (char)(((tag)>>8)&0xFF), (char)((tag)&0xFF) >> >> I need a robust explanation of these changes. >> They are not mentioned, nor are they in upstream harfbuzz. >> Likely these should be pushed to upstream first if there is a reason for >> them. > > @lewurm This and other harfbuzz changes came from MS, could you please > comment here ? This looks like a merge fix mistake: https://github.com/openjdk/jdk/commit/051357ef977ecab77fa9b2b1e61f94f288e716f9#diff-e3815f37244d076e27feef0c778fb78a4e5691b47db9c92abcb28bb2a9c2865e - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 13:30:55 GMT, Vladimir Kempik wrote: >> make/modules/jdk.hotspot.agent/Lib.gmk line 34: >> >>> 32: >>> 33: else ifeq ($(call isTargetOs, macosx), true) >>> 34: SA_CFLAGS := -D_GNU_SOURCE -mno-omit-leaf-frame-pointer \ >> >> Is this really proper for macos-x64? I thought we used -Damd64 as a marker >> for all macos-x64 C/C++ files. (Most files get their flags from the common >> setup in configure, but SA is a different beast due to historical reasons). > > @luhenry , could you please check this comment, I think SA-agent was MS's job > here. The target is identified by the header file now: https://github.com/openjdk/jdk/pull/2200/files#diff-51442e74eeef2163f0f0643df0ae995083f666367e25fba2b527a9a5bc8743a6R35-R41 Do you think there is any problem with this approach? - PR: https://git.openjdk.java.net/jdk/pull/2200
Integrated: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow
On Mon, 23 Nov 2020 09:35:40 GMT, Bernhard Urban-Forster wrote: > This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. This pull request has now been integrated. Changeset: d3dddb6a Author:Bernhard Urban-Forster Committer: Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/d3dddb6a Stats: 92 lines in 1 file changed: 91 ins; 0 del; 1 mod 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow Reviewed-by: shade, ihse - PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v5]
> This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. Bernhard Urban-Forster has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - remove gtest and jtreg - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - merge mistakes - change default target to hotspot and align with updated x64 bits - remove devkit usage on win-aarch64 - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - todo note for caching devkit - remove release build - move windows_aarch64_build next to other builds - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - ... and 2 more: https://git.openjdk.java.net/jdk/compare/9ce3d806...c07f5d72 - Changes: https://git.openjdk.java.net/jdk/pull/1379/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1379=04 Stats: 92 lines in 1 file changed: 91 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v4]
On Tue, 8 Dec 2020 06:59:23 GMT, Aleksey Shipilev wrote: >> Minor nits. > > Also merge from master to get the clean workflow run everywhere? Thank you @shipilev for your comments, I've updated the PR. - PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v4]
> This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. Bernhard Urban-Forster has updated the pull request incrementally with one additional commit since the last revision: merge mistakes - Changes: - all: https://git.openjdk.java.net/jdk/pull/1379/files - new: https://git.openjdk.java.net/jdk/pull/1379/files/34d1ea24..4a1e08d5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1379=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1379=02-03 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v3]
> This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. Bernhard Urban-Forster has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - change default target to hotspot and align with updated x64 bits - remove devkit usage on win-aarch64 - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - todo note for caching devkit - remove release build - move windows_aarch64_build next to other builds - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - remove fixpath.exe workaround - 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow This adds the cross-compiled build only, as no Windows+Arm64 machines are available on GitHub Action that we could use to run the tests. Due to cross-compilation a build JDK is required. Initially I added EA builds to be downloaded from https://jdk.java.net/16/ and used for that, but then I saw how @shipiliv attempted it for the linux cross-compilation builds in https://github.com/openjdk/jdk/pull/1147. That is using the JDK image produced by the x64 variant. This however add more stress to the "critical path", as now two more jobs depend on the x64 build first. Let's see how it works out in the long-run. A Windows+AArch64 build takes 40-50min. - Changes: https://git.openjdk.java.net/jdk/pull/1379/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1379=02 Stats: 119 lines in 1 file changed: 117 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8257679: Improved unix compatibility layer in Windows build (winenv) [v7]
On Sat, 5 Dec 2020 01:13:34 GMT, Magnus Ihse Bursie wrote: >> For the build to work on Windows, we need a unix compatibility layer (known >> as the "winenv" in the build system). This can be e.g. Cygwin or Msys. The >> build system then needs to adapt various aspect to get the build to work in >> this winenv. Over time, our current solutions (which were never that >> well-designed) has collapsed into an unmaintainable mess. >> >> This rewrite takes on a fresh approach, by giving up on the native >> "fixpath.exe" converter, and instead relying on a platform-independent shell >> script "fixpath.sh", which can dynamically adapt to the current winenv. It >> also provides a proper framework on how to categorize, and support, >> different winenvs. As a result, we now easily support Cygwin, Msys2, WSL1 >> and WSL2 (the latter is unfortunately not mature enough to be able to >> compile the JDK). >> >> Furthermore, this rewrite removes all the kludges and hacks that were put in >> place all over the code base, and consolidates all tricky part of handling >> the winenv to basically two places: setting up in configure, and run-time >> handling by fixpath.sh. >> >> This patch also cleans up our handling of how we detect tools in configure, >> and makes a proper framework for cross-compilation on Windows. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Extract only the actual contents added to the PATH by VS SetEnv.cmd. Tested the cross compilation bits with a win-aarch64 build from win-x86_64: - Cygwin: `--openjdk-target=aarch64-unknown-cygwin --with-boot-jdk=/cygdrive/c/work/jdk-16+22` - WSL1: `--openjdk-target=aarch64-unknown-cygwin --with-boot-jdk=/mnt/c/work/jdk-16+22` And smoke tested each one on win-aarch64 with `jtreg:tier1_compiler_1`. Cosmetic: I get a bunch of warnings for non-existing paths in `$PATH` during configure on the wsl1 build, e.g.: configure: Setting extracted environment variables for x86_64 I'll give the "native compilation" on win-aarch64 a try again when this change has landed. Some bits (e.g. config.guess) required for it have made it into this PR, but some things are still missing (e.g. choose x86 binaries for MSVC, since no native bits are available for MSVC). Thank you for the hard work on this! make/autoconf/toolchain_microsoft.m4 line 632: > 630: [path to Microsoft Windows Kit UCRT DLL dir (Windows only) > @<:@probed@:>@])]) > 631: > 632: if test "x$USE_UCRT" = "xtrue" && test "x$OPENJDK_TARGET_CPU" != > xaarch64; then - Marked as reviewed by burban (Author). PR: https://git.openjdk.java.net/jdk/pull/1597
Re: RFR: 8257679: Improved unix compatibility layer in Windows build (winenv) [v7]
On Sat, 5 Dec 2020 01:13:34 GMT, Magnus Ihse Bursie wrote: >> For the build to work on Windows, we need a unix compatibility layer (known >> as the "winenv" in the build system). This can be e.g. Cygwin or Msys. The >> build system then needs to adapt various aspect to get the build to work in >> this winenv. Over time, our current solutions (which were never that >> well-designed) has collapsed into an unmaintainable mess. >> >> This rewrite takes on a fresh approach, by giving up on the native >> "fixpath.exe" converter, and instead relying on a platform-independent shell >> script "fixpath.sh", which can dynamically adapt to the current winenv. It >> also provides a proper framework on how to categorize, and support, >> different winenvs. As a result, we now easily support Cygwin, Msys2, WSL1 >> and WSL2 (the latter is unfortunately not mature enough to be able to >> compile the JDK). >> >> Furthermore, this rewrite removes all the kludges and hacks that were put in >> place all over the code base, and consolidates all tricky part of handling >> the winenv to basically two places: setting up in configure, and run-time >> handling by fixpath.sh. >> >> This patch also cleans up our handling of how we detect tools in configure, >> and makes a proper framework for cross-compilation on Windows. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Extract only the actual contents added to the PATH by VS SetEnv.cmd. make/autoconf/boot-jdk.m4 line 66: > 64: if test "x$BOOT_JDK_FOUND" = xmaybe; then > 65: # Do we have a bin/java? > 66: if test ! -x "$BOOT_JDK/bin/java" && test ! -x > "$BOOT_JDK/bin/java.exe"; then `BOOTJDK_CHECK_BUILD_JDK` also needs those updates I believe - PR: https://git.openjdk.java.net/jdk/pull/1597
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow [v2]
> This adds the cross-compiled build only, as no Windows+Arm64 machines are > available on GitHub Action that we could use to run the tests. > > Due to cross-compilation a build JDK is required. Initially I added EA builds > to be downloaded from https://jdk.java.net/16/ and used for that, but then I > saw how @shipiliv attempted it for the linux cross-compilation builds in > https://github.com/openjdk/jdk/pull/1147. That is using the JDK image > produced by the x64 variant. This however add more stress to the "critical > path", as now two more jobs depend on the x64 build first. > > Let's see how it works out in the long-run. A Windows+AArch64 build takes > 40-50min. Bernhard Urban-Forster has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - todo note for caching devkit - remove release build - move windows_aarch64_build next to other builds - Merge remote-tracking branch 'upstream/master' into 8256657-win-arm64-gh-submit-workflow - remove fixpath.exe workaround - 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow This adds the cross-compiled build only, as no Windows+Arm64 machines are available on GitHub Action that we could use to run the tests. Due to cross-compilation a build JDK is required. Initially I added EA builds to be downloaded from https://jdk.java.net/16/ and used for that, but then I saw how @shipiliv attempted it for the linux cross-compilation builds in https://github.com/openjdk/jdk/pull/1147. That is using the JDK image produced by the x64 variant. This however add more stress to the "critical path", as now two more jobs depend on the x64 build first. Let's see how it works out in the long-run. A Windows+AArch64 build takes 40-50min. - Changes: https://git.openjdk.java.net/jdk/pull/1379/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1379=01 Stats: 155 lines in 1 file changed: 153 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow
On Mon, 23 Nov 2020 12:37:47 GMT, Magnus Ihse Bursie wrote: >> .github/workflows/submit.yml line 1367: >> >>> 1365: git apply p.patch >>> 1366: working-directory: jdk >>> 1367: shell: bash >> >> This should be in the mainline repo instead. Yes, the absence of this patch >> blocks this PR, but getting that patch into the mainline first is the right >> thing to do. > > I agree. We should not have a build action that requires a patch. That patch in its current form is not suitable for upstream. The initial plan was to wait until the "new WINENV" patch lands [1], which would resolve the problem we face with `fixpath.exe`. But maybe I can come up with a solution for the current situation, given that I've a better understanding of the build system by now. [1] https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html - PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow
On Mon, 23 Nov 2020 12:01:14 GMT, Aleksey Shipilev wrote: >> This adds the cross-compiled build only, as no Windows+Arm64 machines are >> available on GitHub Action that we could use to run the tests. >> >> Due to cross-compilation a build JDK is required. Initially I added EA >> builds to be downloaded from https://jdk.java.net/16/ and used for that, but >> then I saw how @shipiliv attempted it for the linux cross-compilation builds >> in https://github.com/openjdk/jdk/pull/1147. That is using the JDK image >> produced by the x64 variant. This however add more stress to the "critical >> path", as now two more jobs depend on the x64 build first. >> >> Let's see how it works out in the long-run. A Windows+AArch64 build takes >> 40-50min. > > I think you need to wait for #1350, and then reconcile this patch with that > refactoring. Thanks for all the comments. > - 40-50 min builds seem to be excessive for just the hotspot build, do > youknow what exactly takes that long? Is this for release and debug each or > both combined? It's for each of them. Installing a specific version of MSVC and creating the devkit take each ~10min. Thinking about it, there is a opportunity to cache the devkit and do the MSVC installer invocation only if no cached devkit is available. Here is an example run if you want to have a closer look: https://github.com/lewurm/openjdk/runs/1434118318?check_suite_focus=true > - Is it worth the cycles to build both release *and* debug? How probable is it that a non-win-aarch-dev will break one but not the other? I presume developers on the windows aarch project will have tested the build locally before pushing. Fair, I'll remove one of them (as suggested by Aleksey in another comment, I'll keep the debug one). - PR: https://git.openjdk.java.net/jdk/pull/1379
RFR: 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow
This adds the cross-compiled build only, as no Windows+Arm64 machines are available on GitHub Action that we could use to run the tests. Due to cross-compilation a build JDK is required. Initially I added EA builds to be downloaded from https://jdk.java.net/16/ and used for that, but then I saw how @shipiliv attempted it for the linux cross-compilation builds in https://github.com/openjdk/jdk/pull/1147. That is using the JDK image produced by the x64 variant. This however add more stress to the "critical path", as now two more jobs depend on the x64 build first. Let's see how it works out in the long-run. A Windows+AArch64 build takes 40-50min. - Commit messages: - 8256657: Add cross-compiled build for Windows+Arm64 to submit workflow Changes: https://git.openjdk.java.net/jdk/pull/1379/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1379=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8256657 Stats: 164 lines in 1 file changed: 163 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1379.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1379/head:pull/1379 PR: https://git.openjdk.java.net/jdk/pull/1379
Re: RFR: 8256538: Fix annoying awk warning in configure for java versions
On Wed, 18 Nov 2020 13:35:14 GMT, Erik Joelsson wrote: >> The change from grep to awk in JDK-8244248 and further bug fixed in >> JDK-8244756 still has invalid syntax. This causes some awk (most notably >> gawk, the most commonly used) to complain: >> >> gawk: cmd. line:1: warning: regexp escape sequence `"' is not a known regexp >> operator >> >> This is annoying and should be fixed. > > Marked as reviewed by erikj (Reviewer). Thank you Magnus, this is something that has bugged me as well. FWIW the same problem exists here with the build JDK detection: https://github.com/openjdk/jdk/blob/9d5fd4fcabf7bdf580e2fff8e12c2cf130ef44c9/make/autoconf/boot-jdk.m4#L519 Would you mind fixing it as part of this PR too? - PR: https://git.openjdk.java.net/jdk/pull/1285
Re: Preliminary review for new WINENV support
Hello Magnus, Sorry for the late reply! This is awesome work and I don't want to see that to bit rot :-) I rebased your changes on top of current master: https://github.com/openjdk/jdk/compare/master...lewurm:winenv-testing This branch is by no means ready to be reviewed, it's just whatever I've used to do my experiments. Also note that it was a bit painful to merge the changes in toolchain_windows.m4 due to the renaming. While I agree that toolchain_microsoft.m4 is the better name, I suggest to defer that change to a later point. I've tested the following scenarios: 1. cygwin on x86_64 to build x86_64 2. cygwin on x86_64 to build arm64 3. WSL1 on x86_64 to build x86_64 4. WSL1 on x86_64 to build arm64 5. WSL2 on x86_64 to build x86_64 6. WSL1 on arm64 to build arm64 Some context: Microsoft doesn't ship native Arm64 binaries for the VS toolchain (yet). Instead one can run x86 binaries via xtajit (a binary translator), but as one can imagine this comes with some performance penalty. Cygwin is also not available on Arm64. Cygwin already being slow, it made sense to avoid running Cygwin x86 on Arm64, and instead go the cross-compilation route in the OpenJDK build for Windows+Arm64. Previously the OpenJDK build did not support cross-compiling on Windows. Even today, on openjdk/jdk:master a dirty workaround for fixpath.exe is needed to build Windows+Arm64 ( https://github.com/openjdk/jdk/pull/212#issuecomment-695024586 ). So we have been very excited to see this WINENV patch :-) Here are comments for each scenario. == ## 1. cygwin on x86_64 to build x86_64 == ``` $ bash configure --with-devkit=/cygdrive/c/work/VS2019-16.6.1-devkit --with-build-devkit=/cygdrive/c/work/VS2019-16.6.1-devkit --with-boot-jdk=/cygdrive/c/work/jdk-16+22 --openjdk-target=aarch64-unknown-cygwin configure: Configuration created at Mon Nov 9 21:33:42 CET 2020. checking for basename... /usr/bin/basename checking for dirname... /usr/bin/dirname checking for file... /usr/bin/file checking for ldd... /usr/bin/ldd checking for bash... /usr/bin/bash checking for cat... /usr/bin/cat checking for chmod... /usr/bin/chmod checking for cp... /usr/bin/cp checking for cut... /usr/bin/cut checking for date... /usr/bin/date checking for gdiff... /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10043: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10047: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10043: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10047: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10043: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10047: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10043: test: /cygdrive/c/Program: binary operator expected /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10047: test: /cygdrive/c/Program: binary operator expected /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10043: test: too many arguments /cygdrive/c/work/winenv-cygwin/build/.configure-support/generated-configure.sh: line 10047: test: too many arguments [not found] [...] ``` I didn't investigate it any further, as this has already been reported on the mailing list. = ## 2. cygwin on x86_64 to build arm64 = ``` $ bash configure --with-devkit=/cygdrive/c/work/VS2019-16.6.1-devkit --with-build-devkit=/cygdrive/c/work/VS2019-16.6.1-devkit --with-boot-jdk=/cygdrive/c/work/jdk-16+22 --openjdk-target=aarch64-unknown-cygwin ``` Same as scenario 1) ## 3. WSL1 on x86_64 to build x86_64 I do get some weird permission errors, so I run everything with sudo (chmod -R $user doesn't help): ``` $ sudo bash configure --with-build-devkit="/mnt/c/work/VS2019-16.6.1-devkit" --with-devkit="/mnt/c/work/VS2019-16.6.1-devkit" --with-boot-jdk="/mnt/c/work/jdk-16+22" [...] Configuration summary: * Debug level:release * HS debug level: product * JVM variants: server * JVM features: server: 'aot cds compiler1 compiler2 epsilongc g1gc graal jfr jni-check jvmci jvmti management nmt parallelgc serialgc services shenandoahgc vm-structs zgc' * OpenJDK target: OS: windows, CPU architecture: x86, address length: 64 * Version string: 16-internal+0-adhoc.root.winenv-wsl1 (16-internal) Tools summary: * Environment:wsl1 version 4.4.0-19041-Microsoft, #488-Microsoft Mon Sep 01 13:43:00 PST 2020 (Ubuntu 20.04.1 LTS);
Integrated: 8254827: JVMCI: Enable it for Windows+AArch64
On Thu, 15 Oct 2020 15:00:47 GMT, Bernhard Urban-Forster wrote: > Use r18 as allocatable register on Linux only. > > A bootstrap works now (it has been crashing before due to r18 being > allocated): > $ ./windows-aarch64-server-fastdebug/bin/java.exe > -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI > -version > Bootstrapping JVMCI. in 17990 ms > (compiled 3330 methods) > openjdk version "16-internal" 2021-03-16 > OpenJDK Runtime Environment (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) > > Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. This pull request has now been integrated. Changeset: 88ee9733 Author:Bernhard Urban-Forster Committer: Tom Rodriguez URL: https://git.openjdk.java.net/jdk/commit/88ee9733 Stats: 24 lines in 4 files changed: 15 ins; 0 del; 9 mod 8254827: JVMCI: Enable it for Windows+AArch64 Reviewed-by: ihse, never, kvn - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
On Mon, 2 Nov 2020 20:22:21 GMT, Vladimir Kozlov wrote: >> Bernhard Urban-Forster has updated the pull request with a new target base >> due to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains five >> additional commits since the last revision: >> >> - add missing precompiled.hpp include >> - Merge remote-tracking branch 'upstream/master' into >> 8254827-enable-jvmci-win-aarch64 >> - rename argument to canUsePlatformRegister >> - comment for platformRegister >> - 8254827: JVMCI: Enable it for Windows+AArch64 >> >>Use r18 as allocatable register on Linux only. >> >>A bootstrap works now (it has been crashing before due to r18 being >> allocated): >>```console >>$ ./windows-aarch64-server-fastdebug/bin/java.exe >> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI >> -version >>Bootstrapping JVMCI. in 17990 ms >>(compiled 3330 methods) >>openjdk version "16-internal" 2021-03-16 >>OpenJDK Runtime Environment (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >>OpenJDK 64-Bit Server VM (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >>``` >> >>Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. > > Marked as reviewed by kvn (Reviewer). Thanks for the review Tom and Magnus, and for the comments to Vladimir and Doug - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
On Mon, 2 Nov 2020 19:33:39 GMT, Vladimir Kozlov wrote: >> Bernhard Urban-Forster has updated the pull request with a new target base >> due to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains five >> additional commits since the last revision: >> >> - add missing precompiled.hpp include >> - Merge remote-tracking branch 'upstream/master' into >> 8254827-enable-jvmci-win-aarch64 >> - rename argument to canUsePlatformRegister >> - comment for platformRegister >> - 8254827: JVMCI: Enable it for Windows+AArch64 >> >>Use r18 as allocatable register on Linux only. >> >>A bootstrap works now (it has been crashing before due to r18 being >> allocated): >>```console >>$ ./windows-aarch64-server-fastdebug/bin/java.exe >> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI >> -version >>Bootstrapping JVMCI. in 17990 ms >>(compiled 3330 methods) >>openjdk version "16-internal" 2021-03-16 >>OpenJDK Runtime Environment (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) >>OpenJDK 64-Bit Server VM (fastdebug build >> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) >>``` >> >>Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. > > make/autoconf/jvm-features.m4 line 309: > >> 307: if test "x$OPENJDK_TARGET_CPU" = "xx86_64"; then >> 308: AC_MSG_RESULT([yes]) >> 309: elif test "x$OPENJDK_TARGET_CPU" = "xaarch64"; then > > You are missing the same change for JVM_FEATURES_CHECK_JVMCI. > Unless it is done intentionally. It's done a couple lines below: https://github.com/openjdk/jdk/pull/685/files#diff-a09b08bcd422d0a8fb32a95ccf85051ac1e69bef2bd420d579f74d8efa286d2fL343 Or do you mean something else? - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Mon, 2 Nov 2020 17:43:31 GMT, Andrew Haley wrote: >> https://github.com/openjdk/jdk/pull/1013 > >> @lewurm >> This patch seems to break on linux-aarch64 with gcc: > > Builds cleanly on Linux/GCC or me. @theRealAph what gcc version? I can reproduce with $ gcc --version gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 which ships in Ubuntu 19.10 as default - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Mon, 2 Nov 2020 16:16:25 GMT, Magnus Ihse Bursie wrote: >> @magicus I did test the initial version of this PR on linux+arm64, but not >> the latest iteration. sorry about that >> >> What is the policy here? Submit a revert right away or investigate a fix? > > @lewurm Open a new JBS issue with the bug. If you can find a fix in a short > amount of time (which I would believe should be possible; probably just need > a proper cast) it's acceptable to fix it directly. What amounts to a "short > amount of time" is left to reasonable judgement; minutes to hours are okay, > days are not. > > Otherwise, create an anti-delta (revert changeset) to back out your changes, > and open yet another JBS issue for re-implementing them correctly. > > In this case, an alternative short-term fix could also be to remove the > assert instead of backing out the entire patch. https://github.com/openjdk/jdk/pull/1013 - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Mon, 2 Nov 2020 15:41:06 GMT, Magnus Ihse Bursie wrote:
>> Thank you Andrew.
>
> @lewurm
> This patch seems to break on linux-aarch64 with gcc:
> open/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp:1501:52: error:
> comparison of integer expressions of different signedness: 'size_t' {aka
> 'long unsigned int'} and 'int' [-Werror=sign-compare]
> 1501 | assert(StackOverflow::stack_shadow_zone_size() ==
> (int)StackOverflow::stack_shadow_zone_size(), "must be same");
> |
> ^~~
>
> Did you test building this on any aarch64 platforms besides Windows?
@magicus I did test the initial version of this PR on linux+arm64, but not the
latest iteration. sorry about that
What is the policy here? Submit a revert right away or investigate a fix?
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Mon, 2 Nov 2020 13:41:53 GMT, Andrew Haley wrote: >> Marked as reviewed by aph (Reviewer). > >> Would you mind to sponsor it @theRealAph or @magicus? > > Hmm, I think you have to integrate it first. > https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/sponsor Thank you Andrew. - PR: https://git.openjdk.java.net/jdk/pull/530
Integrated: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build
On Tue, 6 Oct 2020 18:09:05 GMT, Bernhard Urban-Forster
wrote:
> I organized this PR so that each commit contains the warning emitted by MSVC
> as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors`
> still works. Just mentioning this here, because it's yet another toolchain
> (Xcode / clang) that needs to be kept happy [going
> forward](https://openjdk.java.net/jeps/391).
This pull request has now been integrated.
Changeset: d2812f78
Author:Bernhard Urban-Forster
Committer: Andrew Haley
URL: https://git.openjdk.java.net/jdk/commit/d2812f78
Stats: 23 lines in 8 files changed: 2 ins; 0 del; 21 mod
8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build
Reviewed-by: ihse, aph
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Tue, 27 Oct 2020 14:04:04 GMT, Andrew Haley wrote: >> Bernhard Urban-Forster has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - uppercase suffix >> - add assert > > Marked as reviewed by aph (Reviewer). Would you mind sponsor it @theRealAph or @magicus? - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Tue, 27 Oct 2020 14:04:04 GMT, Andrew Haley wrote: >> Bernhard Urban-Forster has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - uppercase suffix >> - add assert > > Marked as reviewed by aph (Reviewer). Thank you for the reviews, Magnus and Andrew! - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
On Sun, 18 Oct 2020 09:07:17 GMT, Magnus Ihse Bursie wrote: >> Bernhard Urban-Forster has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - uppercase suffix >> - add assert > > Build changes look fine now. @theRealAph does the PR look okay to you now? - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]
> Use r18 as allocatable register on Linux only. > > A bootstrap works now (it has been crashing before due to r18 being > allocated): > $ > ./windows-aarch64-server-fastdebug/bin/java.exe > -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI > -version > Bootstrapping JVMCI. in 17990 ms (compiled > 3330 methods) > openjdk version "16-internal" 2021-03-16 > OpenJDK Runtime Environment (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) > > Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. Bernhard Urban-Forster has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - add missing precompiled.hpp include - Merge remote-tracking branch 'upstream/master' into 8254827-enable-jvmci-win-aarch64 - rename argument to canUsePlatformRegister - comment for platformRegister - 8254827: JVMCI: Enable it for Windows+AArch64 Use r18 as allocatable register on Linux only. A bootstrap works now (it has been crashing before due to r18 being allocated): ```console $ ./windows-aarch64-server-fastdebug/bin/java.exe -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI -version Bootstrapping JVMCI. in 17990 ms (compiled 3330 methods) openjdk version "16-internal" 2021-03-16 OpenJDK Runtime Environment (fastdebug build 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) OpenJDK 64-Bit Server VM (fastdebug build 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) ``` Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. - Changes: - all: https://git.openjdk.java.net/jdk/pull/685/files - new: https://git.openjdk.java.net/jdk/pull/685/files/28dcf572..7e6cb739 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=685=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=685=01-02 Stats: 29566 lines in 423 files changed: 18920 ins; 8788 del; 1858 mod Patch: https://git.openjdk.java.net/jdk/pull/685.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/685/head:pull/685 PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v2]
On Mon, 19 Oct 2020 11:03:46 GMT, Magnus Ihse Bursie wrote: >> Bernhard Urban-Forster has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - rename argument to canUsePlatformRegister >> - comment for platformRegister > > Build changes look good, but you'll need a review on the hotspot part as well. Thank you for your comments @dougxc - PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v2]
> Use r18 as allocatable register on Linux only. > > A bootstrap works now (it has been crashing before due to r18 being > allocated): > $ > ./windows-aarch64-server-fastdebug/bin/java.exe > -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI > -version > Bootstrapping JVMCI. in 17990 ms (compiled > 3330 methods) > openjdk version "16-internal" 2021-03-16 > OpenJDK Runtime Environment (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) > OpenJDK 64-Bit Server VM (fastdebug build > 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) > > Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. Bernhard Urban-Forster has updated the pull request incrementally with two additional commits since the last revision: - rename argument to canUsePlatformRegister - comment for platformRegister - Changes: - all: https://git.openjdk.java.net/jdk/pull/685/files - new: https://git.openjdk.java.net/jdk/pull/685/files/593dfdd6..28dcf572 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=685=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=685=00-01 Stats: 18 lines in 2 files changed: 9 ins; 3 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/685.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/685/head:pull/685 PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v3]
On Thu, 15 Oct 2020 17:24:56 GMT, Stuart Monteith wrote:
>> Bernhard Urban-Forster has updated the pull request with a new target base
>> due to a merge or a rebase. The pull request
>> now contains 20 commits:
>> - disable warning only for hotspot
>> - Merge remote-tracking branch 'upstream/master' into
>> 8254072-fix-windows-arm64-warnings
>> - Merge remote-tracking branch 'upstream/master' into
>> 8254072-fix-windows-arm64-warnings
>> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267:
>> 'argument':
>>conversion from 'size_t' to 'int', possible loss of data
>> - Revert changes for "warning C4146: unary minus operator applied to
>> unsigned type, result still unsigned"
>> - msvc: disable unary minus warning for unsigned types
>> - ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
>> warning C4267: 'initializing': conversion
>>from 'size_t' to 'int', possible loss of data
>>./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
>> warning C4267: 'initializing': conversion
>>from 'size_t' to 'const int', possible loss of data
>> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'unsigned int', possible loss of data
>>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146:
>> unary minus
>>operator applied to unsigned type, result still unsigned
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441):
>>warning C4267: 'argument': conversion from 'size_t' to 'int', possible
>> loss of data
>> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312:
>> 'type cast': conversion from 'unsigned int'
>>to 'address' of greater size
>> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> - ... and 10 more:
>> https://git.openjdk.java.net/jdk/compare/9359ff03...32e922da
>
> src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp line 658:
>
>> 656: }
>> 657: }
>> 658: size_t size_in_bytes() { return 1ull << size(); }
>
> Capital ULL - I find that easer to search for and it is more consistent.
Thank you! Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v2]
On Thu, 15 Oct 2020 09:57:14 GMT, Andrew Haley wrote:
> Fine, but please assert JavaThread::stack_shadow_zone_size() ==
> (int)JavaThread::stack_shadow_zone_size().
Done.
> Adding casts to shut up compilers is a very risky business, because often (if
> not in this case) the programmer doesn't
> understand the code well, and sprinkles casts everywhere. But casts disable
> compile-time type checking, and this leads
> to risks for future maintainability.
Full ACK and I appreciate your comments on this!
> I wonder if we should fix it in a better way, and use something like
> this in the future for all compiler warnings:
>
> ```
> template
> T1 checked_cast(T2 thing) {
> T1 result = static_cast(thing);
> guarantee(static_cast(result) == thing, "must be");
> return result;
> }
> ```
>
> I know this is additional work, but I promise we'll never need to have this
> conversation again.
This sounds like a great idea to me. I assume it doesn't fit into the scope of
this PR, therefore I've created
[JDK-8254856](https://bugs.openjdk.java.net/browse/JDK-8254856) to track it.
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v4]
> I organized this PR so that each commit contains the warning emitted by MSVC
> as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors`
> still works. Just mentioning this here, because
> it's yet another toolchain (Xcode / clang) that needs to be kept happy
> [going
> forward](https://openjdk.java.net/jeps/391).
Bernhard Urban-Forster has updated the pull request incrementally with two
additional commits since the last revision:
- uppercase suffix
- add assert
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/530/files
- new: https://git.openjdk.java.net/jdk/pull/530/files/32e922da..901bbd48
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=530=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=530=02-03
Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/530.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/530/head:pull/530
PR: https://git.openjdk.java.net/jdk/pull/530
RFR: 8254827: JVMCI: Enable it for Windows+AArch64
Use r18 as allocatable register on Linux only. A bootstrap works now (it has been crashing before due to r18 being allocated): $ ./windows-aarch64-server-fastdebug/bin/java.exe -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI -version Bootstrapping JVMCI. in 17990 ms (compiled 3330 methods) openjdk version "16-internal" 2021-03-16 OpenJDK Runtime Environment (fastdebug build 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk) OpenJDK 64-Bit Server VM (fastdebug build 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode) Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well. - Commit messages: - 8254827: JVMCI: Enable it for Windows+AArch64 Changes: https://git.openjdk.java.net/jdk/pull/685/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=685=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254827 Stats: 15 lines in 3 files changed: 8 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/685.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/685/head:pull/685 PR: https://git.openjdk.java.net/jdk/pull/685
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v2]
On Mon, 12 Oct 2020 10:29:23 GMT, Magnus Ihse Bursie wrote:
>> Bernhard Urban-Forster has updated the pull request with a new target base
>> due to a merge or a rebase. The pull request
>> now contains 18 commits:
>> - Merge remote-tracking branch 'upstream/master' into
>> 8254072-fix-windows-arm64-warnings
>> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267:
>> 'argument':
>>conversion from 'size_t' to 'int', possible loss of data
>> - Revert changes for "warning C4146: unary minus operator applied to
>> unsigned type, result still unsigned"
>> - msvc: disable unary minus warning for unsigned types
>> - ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
>> warning C4267: 'initializing': conversion
>>from 'size_t' to 'int', possible loss of data
>>./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
>> warning C4267: 'initializing': conversion
>>from 'size_t' to 'const int', possible loss of data
>> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'unsigned int', possible loss of data
>>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146:
>> unary minus
>>operator applied to unsigned type, result still unsigned
>> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441):
>>warning C4267: 'argument': conversion from 'size_t' to 'int', possible
>> loss of data
>> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312:
>> 'type cast': conversion from 'unsigned int'
>>to 'address' of greater size
>> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267:
>> 'argument': conversion from 'size_t' to
>>'int', possible loss of data
>> - ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning
>> C4267: 'initializing': conversion from 'size_t' to
>>'int', possible loss of data
>>./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning
>> C4267: 'initializing': conversion from 'size_t' to
>>'const int', possible loss of data
>> - ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2756): warning
>> C4146: unary minus operator applied to unsigned
>>type, result still unsigned
>> - ... and 8 more:
>> https://git.openjdk.java.net/jdk/compare/5351ba6c...a081dfb4
>
> Changes requested by ihse (Reviewer).
@theRealAph I prototyped changing the argument of `bang_stack_with_offset()`
from `int` to `size_t` here:
https://github.com/lewurm/openjdk/commit/85a8f655aa0cb69ef13a2de44dd64c60caf19852.
In that approach casting is
essentially pushed down to `bang_stack_with_offset` because the assembler
instruction of most (all) architectures that
is eventually consuming that offset needs a signed integer anyway. Doesn't seem
like a win to me to be honest.
I would rather prefer to go with what we have in this patch (similar to what
x86 is doing today):
--- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
@@ -1524,7 +1524,7 @@ nmethod*
SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
// Generate stack overflow check
if (UseStackBanging) {
-__ bang_stack_with_offset(JavaThread::stack_shadow_zone_size());
+__ bang_stack_with_offset((int)JavaThread::stack_shadow_zone_size());
} else {
Unimplemented();
}
and leave it with that. What do you think?
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v2]
On Mon, 12 Oct 2020 10:29:11 GMT, Magnus Ihse Bursie wrote: >> Bernhard Urban-Forster has updated the pull request with a new target base >> due to a merge or a rebase. The pull request >> now contains 18 commits: >> - Merge remote-tracking branch 'upstream/master' into >> 8254072-fix-windows-arm64-warnings >> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267: >> 'argument': conversion from 'size_t' to >>'int', possible loss of data >>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267: >> 'argument': conversion from 'size_t' to >>'int', possible loss of data >> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267: >> 'argument': >>conversion from 'size_t' to 'int', possible loss of data >> - Revert changes for "warning C4146: unary minus operator applied to >> unsigned type, result still unsigned" >> - msvc: disable unary minus warning for unsigned types >> - ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123): >> warning C4267: 'initializing': conversion >>from 'size_t' to 'int', possible loss of data >>./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123): >> warning C4267: 'initializing': conversion >>from 'size_t' to 'const int', possible loss of data >> - ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267: >> 'argument': conversion from 'size_t' to >>'unsigned int', possible loss of data >>./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267: >> 'argument': conversion from 'size_t' to >>'int', possible loss of data >> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146: >> unary minus >>operator applied to unsigned type, result still unsigned >> ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): >>warning C4267: 'argument': conversion from 'size_t' to 'int', possible >> loss of data >> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312: >> 'type cast': conversion from 'unsigned int' >>to 'address' of greater size >> - ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267: >> 'argument': conversion from 'size_t' to >>'int', possible loss of data >> - ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning >> C4267: 'initializing': conversion from 'size_t' to >>'int', possible loss of data >>./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning >> C4267: 'initializing': conversion from 'size_t' to >>'const int', possible loss of data >> - ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2756): warning >> C4146: unary minus operator applied to unsigned >>type, result still unsigned >> - ... and 8 more: >> https://git.openjdk.java.net/jdk/compare/5351ba6c...a081dfb4 > > make/autoconf/flags-cflags.m4 line 137: > >> 135: WARNINGS_ENABLE_ALL="-W3" >> 136: DISABLED_WARNINGS="4800" >> 137: DISABLED_WARNINGS+=" 4146" # unary minus operator applied to >> unsigned type, result still unsigned > > This change will affect *all* JDK code. I'm not sure this was intended? > > If it was intended, I think you need to motivate this more explicitly. > > If you only wanted to disable the warning for hotspot, the proper solution > would be to add it to > DISABLED_WARNINGS_microsoft in make/hotspot/lib/CompileJvm.gmk. Thank you @magicus! It was indeed meant only for the hotspot part. - PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v3]
> I organized this PR so that each commit contains the warning emitted by MSVC
> as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors`
> still works. Just mentioning this here, because
> it's yet another toolchain (Xcode / clang) that needs to be kept happy
> [going
> forward](https://openjdk.java.net/jeps/391).
Bernhard Urban-Forster has updated the pull request with a new target base due
to a merge or a rebase. The pull request
now contains 20 commits:
- disable warning only for hotspot
- Merge remote-tracking branch 'upstream/master' into
8254072-fix-windows-arm64-warnings
- Merge remote-tracking branch 'upstream/master' into
8254072-fix-windows-arm64-warnings
- ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267:
'argument':
conversion from 'size_t' to 'int', possible loss of data
- Revert changes for "warning C4146: unary minus operator applied to unsigned
type, result still unsigned"
- msvc: disable unary minus warning for unsigned types
- ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
warning C4267: 'initializing': conversion
from 'size_t' to 'int', possible loss of data
./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
warning C4267: 'initializing': conversion
from 'size_t' to 'const int', possible loss of data
- ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267:
'argument': conversion from 'size_t' to
'unsigned int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146: unary
minus
operator applied to unsigned type, result still unsigned
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441):
warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss
of data
- ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312:
'type cast': conversion from 'unsigned int'
to 'address' of greater size
- ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
- ... and 10 more: https://git.openjdk.java.net/jdk/compare/9359ff03...32e922da
-
Changes: https://git.openjdk.java.net/jdk/pull/530/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=530=02
Stats: 22 lines in 8 files changed: 1 ins; 0 del; 21 mod
Patch: https://git.openjdk.java.net/jdk/pull/530.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/530/head:pull/530
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build [v2]
> I organized this PR so that each commit contains the warning emitted by MSVC
> as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors`
> still works. Just mentioning this here, because
> it's yet another toolchain (Xcode / clang) that needs to be kept happy
> [going
> forward](https://openjdk.java.net/jeps/391).
Bernhard Urban-Forster has updated the pull request with a new target base due
to a merge or a rebase. The pull request
now contains 18 commits:
- Merge remote-tracking branch 'upstream/master' into
8254072-fix-windows-arm64-warnings
- ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1446): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1654): warning C4267:
'argument':
conversion from 'size_t' to 'int', possible loss of data
- Revert changes for "warning C4146: unary minus operator applied to unsigned
type, result still unsigned"
- msvc: disable unary minus warning for unsigned types
- ./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
warning C4267: 'initializing': conversion
from 'size_t' to 'int', possible loss of data
./src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp(1123):
warning C4267: 'initializing': conversion
from 'size_t' to 'const int', possible loss of data
- ./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1312): warning C4267:
'argument': conversion from 'size_t' to
'unsigned int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1370): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441): warning C4146: unary
minus
operator applied to unsigned type, result still unsigned
./src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp(1441):
warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss
of data
- ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(2472): warning C4312:
'type cast': conversion from 'unsigned int'
to 'address' of greater size
- ./src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp(1527): warning C4267:
'argument': conversion from 'size_t' to
'int', possible loss of data
- ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning C4267:
'initializing': conversion from 'size_t' to
'int', possible loss of data
./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2901): warning C4267:
'initializing': conversion from 'size_t' to
'const int', possible loss of data
- ./src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp(2756): warning C4146:
unary minus operator applied to unsigned
type, result still unsigned
- ... and 8 more: https://git.openjdk.java.net/jdk/compare/5351ba6c...a081dfb4
-
Changes: https://git.openjdk.java.net/jdk/pull/530/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=530=01
Stats: 22 lines in 8 files changed: 2 ins; 0 del; 20 mod
Patch: https://git.openjdk.java.net/jdk/pull/530.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/530/head:pull/530
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build
On Tue, 6 Oct 2020 18:09:05 GMT, Bernhard Urban-Forster
wrote:
> I organized this PR so that each commit contains the warning emitted by MSVC
> as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors`
> still works. Just mentioning this here, because
> it's yet another toolchain (Xcode / clang) that needs to be kept happy
> [going
> forward](https://openjdk.java.net/jeps/391).
Thank you Andrew for your comments!
> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on
> [hotspot-dev](mailto:hotspot-...@openjdk.java.net):_
> IMO this warning:
>
> warning C4146: unary minus operator applied to unsigned type, result still
> unsigned
>
> should not be used.
Okay, added to the Makefile and reverted those changes.
> // Generate stack overflow check
> if (UseStackBanging) {
> - __ bang_stack_with_offset(JavaThread::stack_shadow_zone_size());
> + __ bang_stack_with_offset((int)JavaThread::stack_shadow_zone_size());
> } else {
> Unimplemented();
>
> Could this one be fixed by changing stack_shadow_zone_size() or
> bang_stack_with_offset() ? I would have thought that whatever type
> stack_shadow_zone_size() returns should be compatible with
> bang_stack_with_offset().
The x86_64 backend and others do the same:
https://github.com/openjdk/jdk/blob/5351ba6cfa8078f503f1cf0c375b692905c607ff/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L2176-L2178
So should we (1) do the same, (2) diverge or (3) fix all of them?
For the remaining comments, I've updated the PR, please have another look.
-
PR: https://git.openjdk.java.net/jdk/pull/530
Re: RFR: 8253757: Add LLVM-based backend for hsdis
On Wed, 7 Oct 2020 08:02:59 GMT, Xin Liu wrote: >> Can you separate LLVM and binutils from hsdis.cpp? >> >> I guess you say that the problem is both GCC and binutils are not available >> on Windows AArch64. Is it right? >> 1 question: binutils seems to support Windows AArch64. Did you try recently >> binutils? If we can use binutils on Windows >> AArch64, you can fix makefile only. >> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=binutils/dlltool.c;h=ed016b97dc38cdb1b85d2f6df676b9c9750f0d41;hb=HEAD#l248 > > IMHO, it's great to have an alternative disassembler. I personally had > better experience using llvm MC when I decoded > aarch64 and AVX instructions than BFD. Another argument is that LLVM > toolchain is supposed to provide the premium > experience on non-gnu platforms such as FreeBSD.@luhenry I tried to > build it with LLVM10.0.1 > on my x86_64, ubuntu, I ran into a small problem. here is how I build. > `$make ARCH=amd64 CC=/opt/llvm/bin/clang CXX=/opt/llvm/bin/clang++ > LLVM=/opt/llvm/` > > I can't meet this condition because Makefile defines LIBOS_linux. > #elif defined(LIBOS_Linux) && defined(LIBARCH_amd64) > return "x86_64-pc-linux-gnu"; > > Actually, Makefile assigns OS to windows/linux/aix/macosx (all lower case)and > then > `CPPFLAGS+= -DLIBOS_$(OS) -DLIBOS="$(OS)" -DLIBARCH_$(LIBARCH) > -DLIBARCH="$(LIBARCH)" -DLIB_EXT="$(LIB_EXT)"` > > In hsdis.cpp, `native_target_triple` needs to match whatever Makefile > defined. With that fix, I generate llvm version > hsdis-amd64.so and it works flawlessly > 1 question: binutils seems to support Windows AArch64. Did you try recently > binutils? If we can use binutils on Windows > AArch64, you can fix makefile only. > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=binutils/dlltool.c;h=ed016b97dc38cdb1b85d2f6df676b9c9750f0d41;hb=HEAD#l248 This is armv7, I don't see any support for armv8/AArch64 in `dlltool.c`. - PR: https://git.openjdk.java.net/jdk/pull/392
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v7]
On Fri, 25 Sep 2020 12:44:37 GMT, Andrew Haley wrote:
>> Monica Beckwith has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - os_windows: remove duplicated UMA handling
>> - test_safefetch{32,N} works fine on win+aarch64
>
> Marked as reviewed by aph (Reviewer).
@theRealAph okay, I've changed the string representation of `r18` to
`"r18_tls"` on every platform.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 19:28:10 GMT, Vladimir Kempik wrote:
>> The idea is that the naming should suggest that `r18` shouldn't be used on
>> that particular platform. Same is true for
>> macOS, but the ABI docs suggest a different usage, hence we have something
>> like that in our internal branch for macOS:
>> Suggestion:
>> "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls")
>> MACOS_ONLY("rplatform"), "r19",
>> Are you suggesting it should rather be something like this eventually?
>> Suggestion:
>>
>> "r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
>> "r19",
>
> this looks better I think, if it's done right from beginning, we won't have
> to modify it later.
> The Question is, can we do it ahead of JEP-391 ?
> If we can't then maybe better to leave it this way for now:
> WIN64_ONLY("rtls") NOT_WIN64("r18")
>
> as r18 is supposed to be reserved register on aarch64, linux is just an
> exception where it's allowed.
Let us go with the following in this PR as that's the extend of this PR:
Suggestion:
"r17", LINUX_ONLY("r18") WIN64_ONLY("rtls"), "r19",
We can update it accordingly in a PR for JEP-391.
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v10]
On Mon, 28 Sep 2020 17:37:32 GMT, Vladimir Kempik wrote:
>> Monica Beckwith has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now
>> contains 24 commits:
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - SA: update copyright
>> - Fix graal codestyle
>> - Reduce includes
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - os_windows: remove duplicated UMA handling
>> - test_safefetch{32,N} works fine on win+aarch64
>> - cleanup for 8253539: Remove unused JavaThread functions for
>> set_last_Java_fp/pc
>> - cleanup for 8253457: Remove unimplemented register stack functions
>> - Merge remote-tracking branch 'upstream/master' into jdk-windows
>> - ... and 14 more:
>> https://git.openjdk.java.net/jdk/compare/ec9bee68...a7cdaad6
>
> src/hotspot/cpu/aarch64/register_aarch64.cpp line 44:
>
>> 42: "rscratch1", "rscratch2",
>> 43: "r10", "r11", "r12", "r13", "r14", "r15", "r16",
>> 44: "r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls"), "r19",
>
> For me this line doesn't look good in case of expanding this functionality to
> macos-aarch64
> as it's just the name of register and it's r18 on every platform except WIN64
> and has nothing to do with reserving r18.
The idea is that the naming should suggest that `r18` shouldn't be used on that
particular platform. Same is true for
macOS, but the ABI docs suggest a different usage, hence we have something like
that in our internal branch for macOS:
Suggestion:
"r17", NOT_R18_RESERVED("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"),
"r19",
Are you suggesting it should rather be something like this eventually?
Suggestion:
"r17", LINUX_ONLY("r18") WIN64_ONLY("rtls") MACOS_ONLY("rplatform"), "r19",
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v6]
On Thu, 24 Sep 2020 15:43:10 GMT, Chris Plummer wrote: >> Monica Beckwith has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev >> excludes the unrelated changes brought in by the merge/rebase. The pull >> request contains 17 additional commits since >> the last revision: >> - cleanup for 8253539: Remove unused JavaThread functions for >> set_last_Java_fp/pc >> - cleanup for 8253457: Remove unimplemented register stack functions >> - Merge remote-tracking branch 'upstream/master' into jdk-windows >> - Update orderAccess_windows_aarch64.hpp >> >>changing from Acq-reL to Sequential Consistency to avoid compiler >> reordering when no ordering hints are provided >> - 8248787: G1: Workaround MSVC bug >>Reviewed-by: >>Contributed-by: mbeckwit, luhenry, burban >> - 8248670: Windows: Exception handling support on AArch64 >>Reviewed-by: >>Contributed-by: mbeckwit, luhenry, burban >> - 8248660: AArch64: Make _clear_cache and _nop portable >>Summary: __builtin___clear_cache, etc. >>Contributed-by: mbeckwit, luhenry, burban >> - 8248659: AArch64: Extend CPU Feature detection >>Reviewed-by: >>Contributed-by: mbeckwit, luhenry, burban >> - 8248656: Add Windows AArch64 platform support code >>Reviewed-by: >>Contributed-by: mbeckwit, luhenry, burban >> - 8248498: Add build system support for Windows AArch64 >>Reviewed-by: >>Contributed-by: mbeckwit, luhenry, burban >> - ... and 7 more: >> https://git.openjdk.java.net/jdk/compare/451890eb...2b662010 > > I looked at changes to existing SA files. These changes look fine. > > I did not look at the new aarch64 SA files other than the copyright section. > I assume they are clones of the x64 > versions with some symbolic renaming. If there is any more than that and > you'd like me to have a look, let me know. > As for the copyright in the new SA files, I believe it is incorrect and needs > to include Oracle. There are a number of > other non-SA files that are new and also have the same copyright issue. > I also looked at > src/jdk.attach/windows/classes/sun/tools/attach/AttachProviderImpl.java. It > looks fine except it needs > a copyright date update. @plummercj thank you for your feedback. I've updated the copyright in mentioned files. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v5]
On Thu, 24 Sep 2020 04:52:22 GMT, David Holmes wrote: >> Monica Beckwith has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update orderAccess_windows_aarch64.hpp >> >> changing from Acq-reL to Sequential Consistency to avoid compiler >> reordering when no ordering hints are provided > > src/hotspot/share/runtime/stubRoutines.cpp line 397: > >> 395: // test safefetch routines >> 396: // Not on Windows 32bit until 8074860 is fixed >> 397: #if ! (defined(_WIN32) && defined(_M_IX86)) && !defined(_M_ARM64) > > The comment needs updating to refer to Aarch64. This is actually not needed anymore, as it does work just fine on Windows+Aarch64. Presumably we have added it in the early days of the port when we haven't figured out exception handling quite yet. Thanks for catching David. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v5]
On Thu, 24 Sep 2020 04:45:16 GMT, David Holmes wrote: >> Monica Beckwith has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update orderAccess_windows_aarch64.hpp >> >> changing from Acq-reL to Sequential Consistency to avoid compiler >> reordering when no ordering hints are provided > > src/hotspot/os/windows/os_windows.cpp line 2546: > >> 2544: >> 2545: #ifdef _M_ARM64 >> 2546: // Unsafe memory access > > I'm not at all clear why this unsafe memory access handling is for Aarch64 > only? Hum, this was already part of [JDK-8250810](https://github.com/openjdk/jdk/commit/a4eaf9536c272862b5ec856bf263679be09bddc9) / [JDK-8248817](https://github.com/openjdk/jdk/commit/257809d7440e87ac595d03b6c9a98d8f457f314c) (see a couple lines below in this PR) without the `ifdef` for Aarch64, but I messed up rebasing on top of it. I removed it now, thanks for catching! - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v3]
On Mon, 21 Sep 2020 08:15:20 GMT, Bernhard Urban-Forster
wrote:
>> Hey @erikj79, thank you so much for giving it a try!
>>
>>> Our linux-aarch64 build fails with this:
>>> cc: error: unrecognized command line option '-std=c++14'
>>> when compiling
>>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
>>
>> Hmm, that's interesting. What environment is that exactly? What `configure`
>> line are you using there? We have tested on
>> such a system: $ cat /etc/issue
>> Ubuntu 19.10 \n \l
>> $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg
>> $ make clean CONF=linux-aarch64-server-release
>> $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release
>> $ ./build/linux-aarch64-server-release/images/jdk/bin/java
>> -XshowSettings:properties -version 2>&1 | grep aarch64
>> java.home =
>> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk
>> os.arch = aarch64
>> sun.boot.library.path =
>> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib
>>
>>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath.
>>> Is this something you are also experiencing,
>>> and if so, how are you addressing it?
>>
>> Yes. As far as I understand, the problem is that `fixpath.exe` isn't built
>> properly when doing cross-compiling on
>> Windows targets (as it hasn't been a thing so far). We use a workaround
>> internally
>> https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a
>> proper solution is under discussion on
>> build-dev:
>> https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html
>
>> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on
>> [build-dev](mailto:build-dev@openjdk.java.net):_
>>
>> On 18/09/2020 11:14, Monica Beckwith wrote:
>>
>> > This is a continuation of
>> > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>>
>> The diffs in assembler_aarch64.cpp are mostly spurious. Please try this.
>
>
> Thank you Andrew. Is the goal to reduce the patch diff in
> `assembler_aarch64.cpp`? If so, we need to get rid of the
> retry in your patch to avoid additional calls to `random`, e.g. something
> like this (the diff for the generated part
> would look indeed nicer with that:
> https://gist.github.com/lewurm/2e7b0e00447696c75e00febb83034ba1 ):
> --- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> +++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
> @@ -13,6 +13,8 @@ class Register(Operand):
>
> def generate(self):
> self.number = random.randint(0, 30)
> +if self.number == 18:
> +self.number = 17
> return self
>
> def astr(self, prefix):
> @@ -37,6 +39,8 @@ class GeneralRegisterOrZr(Register):
>
> def generate(self):
> self.number = random.randint(0, 31)
> +if self.number == 18:
> +self.number = 16
> return self
>
> def astr(self, prefix = ""):
> @@ -54,6 +58,8 @@ class GeneralRegisterOrZr(Register):
> class GeneralRegisterOrSp(Register):
> def generate(self):
> self.number = random.randint(0, 31)
> +if self.number == 18:
> +self.number = 15
> return self
>
> def astr(self, prefix = ""):
> @@ -1331,7 +1337,7 @@ generate(SpecialCases, [["ccmn", "__ ccmn(zr, zr, 3u,
> Assembler::LE);",
> ["st1w", "__ sve_st1w(z0, __ S, p1, Address(r0,
> 7));", "st1w\t{z0.s}, p1, [x0, #7, MUL VL]"],
> ["st1b", "__ sve_st1b(z0, __ B, p2, Address(sp,
> r1));","st1b\t{z0.b}, p2, [sp, x1]"],
> ["st1h", "__ sve_st1h(z0, __ H, p3, Address(sp,
> r8));","st1h\t{z0.h}, p3, [sp, x8, LSL #1]"],
> -["st1d", "__ sve_st1d(z0, __ D, p4, Address(r0,
> r18));", "st1d\t{z0.d}, p4, [x0, x18, LSL #3]"],
> +["st1d", "__ sve_st1d(z0, __ D, p4, Address(r0,
> r17));", "st1d\t{z0.d}, p4, [x0, x17,
> LSL #3]"],
> ["ldr","__ sve_ldr(z0, Address(sp));",
> "ldr\tz0, [sp]"],
> ["ldr","__ sve_ldr(z
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]
On Fri, 18 Sep 2020 18:38:34 GMT, Bernhard Urban-Forster
wrote:
>> Our linux-aarch64 build fails with this:
>> cc: error: unrecognized command line option '-std=c++14'
>> when compiling
>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
>>
>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is
>> this something you are also experiencing,
>> and if so, how are you addressing it?
>
> Hey @erikj79, thank you so much for giving it a try!
>
>> Our linux-aarch64 build fails with this:
>> cc: error: unrecognized command line option '-std=c++14'
>> when compiling
>> build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch
>
> Hmm, that's interesting. What environment is that exactly? What `configure`
> line are you using there? We have tested on
> such a system: $ cat /etc/issue
> Ubuntu 19.10 \n \l
> $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg
> $ make clean CONF=linux-aarch64-server-release
> $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release
> $ ./build/linux-aarch64-server-release/images/jdk/bin/java
> -XshowSettings:properties -version 2>&1 | grep aarch64
> java.home =
> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk
> os.arch = aarch64
> sun.boot.library.path =
> /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib
>
>> I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is
>> this something you are also experiencing,
>> and if so, how are you addressing it?
>
> Yes. As far as I understand, the problem is that `fixpath.exe` isn't built
> properly when doing cross-compiling on
> Windows targets (as it hasn't been a thing so far). We use a workaround
> internally
> https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a
> proper solution is under discussion on
> build-dev:
> https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html
> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on
> [build-dev](mailto:build-dev@openjdk.java.net):_
>
> On 18/09/2020 11:14, Monica Beckwith wrote:
>
> > This is a continuation of
> > https://mail.openjdk.java.net/pipermail/aarch64-port-dev/2020-August/009566.html
>
> The diffs in assembler_aarch64.cpp are mostly spurious. Please try this.
Thank you Andrew. Is the goal to reduce the patch diff in
`assembler_aarch64.cpp`? If so, we need to get rid of the
retry in your patch to avoid additional calls to `random`, e.g. something like
this (the diff for the generated part
would look indeed nicer with that:
https://gist.github.com/lewurm/2e7b0e00447696c75e00febb83034ba1 ):
--- a/src/hotspot/cpu/aarch64/aarch64-asmtest.py
+++ b/src/hotspot/cpu/aarch64/aarch64-asmtest.py
@@ -13,6 +13,8 @@ class Register(Operand):
def generate(self):
self.number = random.randint(0, 30)
+if self.number == 18:
+self.number = 17
return self
def astr(self, prefix):
@@ -37,6 +39,8 @@ class GeneralRegisterOrZr(Register):
def generate(self):
self.number = random.randint(0, 31)
+if self.number == 18:
+self.number = 16
return self
def astr(self, prefix = ""):
@@ -54,6 +58,8 @@ class GeneralRegisterOrZr(Register):
class GeneralRegisterOrSp(Register):
def generate(self):
self.number = random.randint(0, 31)
+if self.number == 18:
+self.number = 15
return self
def astr(self, prefix = ""):
@@ -1331,7 +1337,7 @@ generate(SpecialCases, [["ccmn", "__ ccmn(zr, zr, 3u,
Assembler::LE);",
["st1w", "__ sve_st1w(z0, __ S, p1, Address(r0,
7));", "st1w\t{z0.s}, p1, [x0, #7, MUL VL]"],
["st1b", "__ sve_st1b(z0, __ B, p2, Address(sp,
r1));","st1b\t{z0.b}, p2, [sp, x1]"],
["st1h", "__ sve_st1h(z0, __ H, p3, Address(sp,
r8));","st1h\t{z0.h}, p3, [sp, x8, LSL #1]"],
-["st1d", "__ sve_st1d(z0, __ D, p4, Address(r0,
r18));", "st1d\t{z0.d}, p4, [x0, x18, LSL #3]"],
+["st1d", "__ sve_st1d(z0, __ D, p4, Address(r0,
r17));", "st1d\t{z0.d}, p4, [x0, x17,
LSL #3]"],
["ldr","__ sve_ldr(z0, Address(sp));",
"ldr\tz0, [sp]"],
["ldr","__ sve_ldr(z31, Address(sp, -256));",
"ldr\tz31, [sp, #-256, MUL VL]"],
["str","__ sve_str(z8, Address(r8, 255));",
"str\tz8, [x8, #255, MUL VL]"],
-
PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v2]
On Fri, 18 Sep 2020 20:34:55 GMT, Erik Joelsson wrote: > I assume you need the rest of the PATH on Windows. Doesn't look like it actually. I've reverted it, thanks for catching it. - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support
On Fri, 18 Sep 2020 15:34:26 GMT, Erik Joelsson wrote: >> Build changes look good to me. I will take this branch for a spin. > > Our linux-aarch64 build fails with this: > cc: error: unrecognized command line option '-std=c++14' > when compiling > build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch > > I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is > this something you are also experiencing, > and if so, how are you addressing it? Hey @erikj79, thank you so much for giving it a try! > Our linux-aarch64 build fails with this: > cc: error: unrecognized command line option '-std=c++14' > when compiling > build/linux-aarch64/buildjdk/hotspot/variant-server/libjvm/objs/precompiled/precompiled.hpp.gch Hmm, that's interesting. What environment is that exactly? What `configure` line are you using there? We have tested on such a system: $ cat /etc/issue Ubuntu 19.10 \n \l $ bash configure --with-boot-jdk=/home/beurba/work/jdk-16+13 --with-jtreg $ make clean CONF=linux-aarch64-server-release $ make images JOBS=255 LOG=info CONF=linux-aarch64-server-release $ ./build/linux-aarch64-server-release/images/jdk/bin/java -XshowSettings:properties -version 2>&1 | grep aarch64 java.home = /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk os.arch = aarch64 sun.boot.library.path = /home/beurba/work/jdk/build/linux-aarch64-server-release/images/jdk/lib > I'm trying to configure a windows-aarch64 build, but it fails on fixpath. Is > this something you are also experiencing, > and if so, how are you addressing it? Yes. As far as I understand, the problem is that `fixpath.exe` isn't built properly when doing cross-compiling on Windows targets (as it hasn't been a thing so far). We use a workaround internally https://gist.github.com/lewurm/c099a4b5fcd8a182510cbdeebcb41f77 , but a proper solution is under discussion on build-dev: https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html - PR: https://git.openjdk.java.net/jdk/pull/212
Re: RFR: 8250876: Build system preparation to macos on aarch64
Good observation David, the change in adlc is just fixing a symptom. The difference to a regular macOS build is that technically, despite running on the same machine, it's actually cross compiling due to Rosetta being the --build=x86_64 system. Being a cross-compile, we therefore hit this case: https://github.com/openjdk/jdk/blob/b0ceab23dd4176329cbf3a95f21e8e9ac2d8723f/make/autoconf/toolchain.m4#L905-L921 And thus infers `/usr/bin/CC` for CXX. I guess cross compiling hasn't been a thing on macOS yet. I tried the following and it passes the adlc build: --- a/make/autoconf/toolchain.m4 +++ b/make/autoconf/toolchain.m4 @@ -917,7 +917,7 @@ AC_DEFUN_ONCE([TOOLCHAIN_SETUP_BUILD_COMPILERS], # find the build compilers in the tools dir, if needed. UTIL_REQUIRE_PROGS(BUILD_CC, [cl cc gcc]) UTIL_FIXUP_EXECUTABLE(BUILD_CC) -UTIL_REQUIRE_PROGS(BUILD_CXX, [cl CC g++]) +UTIL_REQUIRE_PROGS(BUILD_CXX, [clang++ cl CC g++]) UTIL_FIXUP_EXECUTABLE(BUILD_CXX) UTIL_PATH_PROGS(BUILD_NM, nm gcc-nm) UTIL_FIXUP_EXECUTABLE(BUILD_NM) Although I'm not sure about its cleanliness :-) -Bernhard From: build-dev on behalf of David Holmes Sent: Tuesday, August 4, 2020 00:46 To: Erik Joelsson; Vladimir Kempik; build-dev Cc: Anton Kozlov; Alexander Ioffe; Andrew Brygin; Andrey Petushkov Subject: Re: RFR: 8250876: Build system preparation to macos on aarch64 On 3/08/2020 10:57 pm, Erik Joelsson wrote: > Hello Vladimir, > > These changes look innocent enough to me. They aren't actually adding > macosx-aarch64 support, they are just removing two minor (and more > likely OS version related) hurdles from the build. You still have to > provide the actual configuration on the configure command line as is > shown in your example. Before we can call build system support, we would > need configure to automatically setup those flags and add a separate > parameter for the JNF framework. So, given that, I don't think this > change warrants a JEP in itself. Of course this change doesn't warrant a JEP in itself :) My point is that until we have a JEP for the platform that is being targeted then we shouldn't be making changes to provide support for that platform. That said I didn't actually look at the changes but focused on the larger stated aim, so apologies for that. The actual changes here are small and not obviously related to supporting macOS-Aarch64. But I'm unclear on this change as it affects all macOS builds: 42 else ifeq ($(call isBuildOs, macosx), true) 43 ADLC_LDFLAGS := -lc++ if this was fixing a bug as indicated, why do we not see this bug in regular builds? Thanks, David - > My only complaint is that you revert jib-profiles.js. That file is only > used internally at Oracle. If/when we need it to support macosx-aarch64, > we will provide those changes. > > I must say I'm happy to see you managed to get a working build > configuration with just this though! > > /Erik > > On 2020-08-01 00:24, Vladimir Kempik wrote: >> Hello >> >> Please review this change for JDK-8250876 >> >> This changeset adds support for macos/aarch64 into build system. >> It will allow to crosscompile for macos/aarch64 using intel mac as well. >> >> This changeset does NOT address some arm specific issues in the macos >> related code, we plan to do that in s separate commit. >> >> An example of configure to cross-compile for macos/arm64: >> >> --with-boot-jdk=/path/to/java/ >> --with-build-jdk=/path/to/same/java/as/compiled >> --disable-warnings-as-errors --with-jvm-variants=zero >> --openjdk-target=aarch64-apple-darwin --with-extra-cflags='-arch >> arm64' --with-extra-ldflags='-arch arm64 >> -F/Path/To/Folder/Containing/JNF_framework/' >> —with-extra-cxxflags='-arch arm64’ >> >> JNF.framework is missing arm64 part as of next macos release, but >> Apple has opensourced it. >> >> Fix to adlc were needed due to it using symbols from stdc++ and not >> linking to it, so it fails when doing make images. >> >> The webrev: >> https://nam06.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~vkempik%2F8250876%2Fwebrev.00%2Fdata=02%7C01%7Cbeurba%40microsoft.com%7C0c8d58d5eb9144e8717f08d837ff3736%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637320916565796801sdata=HpXJmHXbuawTdExWESK9ssesYTuPTj7N6inXjaHfVaM%3Dreserved=0 >> The bug: >> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8250876data=02%7C01%7Cbeurba%40microsoft.com%7C0c8d58d5eb9144e8717f08d837ff3736%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637320916565796801sdata=9z2Nw8d0pa5huxUKOYorMOVy6SBo7o%2FhDT1EmgOhxQ8%3Dreserved=0 >> >> Testing: jdk/submit. >> >> Thanks, Vladimir.
