On Mon, 12 Oct 2020 10:29:23 GMT, Magnus Ihse Bursie <i...@openjdk.org> 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