On Fri, 19 Apr 2024 20:13:03 GMT, Scott Gibbons <sgibb...@openjdk.org> wrote:
>> This code makes an intrinsic stub for `Unsafe::setMemory` for x86_64. See >> [this PR](https://github.com/openjdk/jdk/pull/16760) for discussion around >> this change. >> >> Overall, making this an intrinsic improves overall performance of >> `Unsafe::setMemory` by up to 4x for all buffer sizes. >> >> Tested with tier-1 (and full CI). I've added a table of the before and >> after numbers for the JMH I ran (`MemorySegmentZeroUnsafe`). >> >> [setMemoryBM.txt](https://github.com/openjdk/jdk/files/14808974/setMemoryBM.txt) > > Scott Gibbons has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments This looks good. I only have question about long vs short jumps in stub's code. src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 2550: > 2548: > 2549: // If zero, then we're done > 2550: __ jccb(Assembler::zero, L_exit); Code in `generate_unsafe_setmemory()` uses long jumps to `L_exit` but here you use short. Why? src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 2638: > 2636: L_exit, _masm); > 2637: } > 2638: __ jmp(L_exit); Here is long jump to `L_exit` after `do_setmemory_atomic_loop()` call. Should this be also short jump? src/hotspot/share/opto/runtime.cpp line 785: > 783: fields[argp++] = TypePtr::NOTNULL; // dest > 784: fields[argp++] = TypeX_X; // size > 785: LP64_ONLY(fields[argp++] = Type::HALF); // size Nit: align `/` src/hotspot/share/utilities/copy.hpp line 2: > 1: /* > 2: * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights > reserved. You forgot to undo year change in this file. ------------- PR Review: https://git.openjdk.org/jdk/pull/18555#pullrequestreview-2012400269 PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572947954 PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572948693 PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572955327 PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572960023
