Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3e2aab6
Add Unsafe.setMemory as intrinsic
asgibbons Mar 27, 2024
2334b03
Added actual intrinsic
asgibbons Mar 27, 2024
6eebcbd
Removed setMemory1; debugged intrinsic code
asgibbons Mar 28, 2024
7c73856
Test removing intrinsic
asgibbons Mar 28, 2024
74c47e2
Add benchmark
asgibbons Mar 28, 2024
6e283bc
Restore intrinsic
asgibbons Mar 28, 2024
44c24ec
Address review comment
asgibbons Mar 29, 2024
b17a1f4
Fixed bug - incorrect interface to *_fill_entry
asgibbons Mar 29, 2024
401a2a9
Clean up code for PR
asgibbons Mar 29, 2024
c5cb30c
Use non-sse fill (old left in)
asgibbons Apr 1, 2024
6ee69c8
Remove dead code
asgibbons Apr 1, 2024
3aa60a4
Addressing review comments.
asgibbons Apr 2, 2024
8bed156
Fix Windows
asgibbons Apr 3, 2024
b025318
Fixed generate_fill when count > 0x80000000
asgibbons Apr 5, 2024
fd6f04f
Oops
asgibbons Apr 6, 2024
f81aaa9
Add movq to locate_operand
asgibbons Apr 8, 2024
b0ac857
Address review comments (#15)
asgibbons Apr 11, 2024
95230e2
Set memory test (#16)
asgibbons Apr 11, 2024
41ffcc3
Merge master
asgibbons Apr 11, 2024
b99499a
Fix whitespace error.
asgibbons Apr 11, 2024
89db3eb
Addressing more review comments
asgibbons Apr 11, 2024
970c575
Addressing yet more review comments
asgibbons Apr 12, 2024
6e731c8
Even more review comments
asgibbons Apr 12, 2024
405e4e0
Change fill routines
asgibbons Apr 15, 2024
95b0a34
Rename UnsafeCopyMemory{,Mark} to UnsafeMemory{Access,Mark} (#19)
asgibbons Apr 15, 2024
44cc91b
Only add a memory mark for byte unaligned fill
asgibbons Apr 15, 2024
824fb60
Set memory test (#21)
asgibbons Apr 15, 2024
80b5a0c
Set memory test (#22)
asgibbons Apr 15, 2024
856464e
Set memory test (#23)
asgibbons Apr 15, 2024
116d7dd
Merge branch 'openjdk:master' into setMemory
asgibbons Apr 15, 2024
113aa90
Fix memory mark after sync to upstream
asgibbons Apr 15, 2024
7a1d67e
Add enter() and leave(); remove Windows-specific register stuff
asgibbons Apr 16, 2024
dccf6b6
Address review comments; update copyright years
asgibbons Apr 19, 2024
dd0094e
Review comments
asgibbons Apr 19, 2024
1961624
Long to short jmp; other cleanup
asgibbons Apr 19, 2024
c129016
Fix UnsafeCopyMemoryMark scope issue
asgibbons Apr 20, 2024
1122b50
Merge branch 'openjdk:master' into setMemory
asgibbons Apr 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/hotspot/cpu/x86/macroAssembler_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5965,7 +5965,7 @@ void MacroAssembler::generate_fill(BasicType t, bool aligned,
orl(value, rtmp);
}

cmpl(count, 2<<shift); // Short arrays (< 8 bytes) fill by element
cmpptr(count, 2<<shift); // Short arrays (< 8 bytes) fill by element
jcc(Assembler::below, L_fill_4_bytes); // use unsigned cmp
if (!UseUnalignedLoadStores && !aligned && (t == T_BYTE || t == T_SHORT)) {
Label L_skip_align2;
Expand Down Expand Up @@ -6042,7 +6042,7 @@ void MacroAssembler::generate_fill(BasicType t, bool aligned,
Label L_fill_64_bytes_loop_avx3, L_check_fill_64_bytes_avx2;

// If number of bytes to fill < VM_Version::avx3_threshold(), perform fill using AVX2
cmpl(count, VM_Version::avx3_threshold());
cmpptr(count, VM_Version::avx3_threshold());
jccb(Assembler::below, L_check_fill_64_bytes_avx2);

vpbroadcastd(xtmp, xtmp, Assembler::AVX_512bit);
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/cpu/x86/stubGenerator_x86_64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4013,6 +4013,11 @@ void StubGenerator::generate_initial_stubs() {
UnsafeCopyMemory::create_table(16);
}

// Initialize table for unsafe set memeory check.
if (UnsafeSetMemory::_table == nullptr) {
UnsafeSetMemory::create_table(16);
}

// entry points that exist in all platforms Note: This is code
// that could be shared among different platforms - however the
// benefit seems to be smaller than the disadvantage of having a
Expand Down
33 changes: 25 additions & 8 deletions src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ address StubGenerator::generate_disjoint_copy_avx3_masked(address* entry, const
__ ret(0);

if (MaxVectorSize == 64) {
UnsafeCopyMemoryMark ucmm(this, !is_oop && !aligned, false, ucme_exit_pc);
__ BIND(L_copy_large);
arraycopy_avx3_large(to, from, temp1, temp2, temp3, temp4, count, xmm1, xmm2, xmm3, xmm4, shift);
__ jmp(L_finish);
Expand Down Expand Up @@ -2547,6 +2548,7 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
// Fill words
{
Label L_wordsTail, L_wordsLoop, L_wordsTailLoop;
UnsafeSetMemoryMark usmm(this, true, true);
////// Set words
__ leaq(rScratch2, Address(size, 1));
__ movq(rScratch1, rScratch2);
Expand Down Expand Up @@ -2584,14 +2586,15 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
__ incrementq(rScratch1);
__ cmpq(size, rScratch1);
__ jccb(Assembler::notEqual, L_wordsTailLoop);
__ jmp(L_exit);
}
__ jmp(L_exit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is long jump to L_exit after do_setmemory_atomic_loop() call. Should this be also short jump?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have additional code in debug VM wihch increase distance and requires long jump? I don't see it. Usually it something which call __ STOP().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code required a long jump due to the size of do_setmemory_atomic_loop but has since been refactored. The jmp(Label) code will generate a short jump provided the label has been defined and is in range. Otherwise a long jump is generated.

Changed to jmpb


__ BIND(L_fillQuadwords);

// Fill QUADWORDs
{
Label L_qwordLoop, L_qwordsTail, L_qwordsTailLoop;
UnsafeSetMemoryMark usmm(this, true, true);

__ leaq(rScratch2, Address(size, 7));
__ movq(rScratch1, rScratch2);
Expand Down Expand Up @@ -2628,14 +2631,18 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
__ incrementq(rScratch1);
__ cmpq(size, rScratch1);
__ jccb(Assembler::notEqual, L_qwordsTailLoop);
__ jmp(L_exit);
}
__ BIND(L_exit);

restore_arg_regs();
__ ret(0);

__ BIND(L_fillDwords);

// Fill DWORDs
{
Label L_dwordLoop, L_dwordsTail, L_dwordsTailLoop;
UnsafeSetMemoryMark usmm(this, true, true);

__ leaq(rScratch2, Address(size, 3));
__ movq(rScratch1, rScratch2);
Expand Down Expand Up @@ -2675,13 +2682,14 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
__ incrementq(rScratch1);
__ cmpq(size, rScratch1);
__ jccb(Assembler::notEqual, L_dwordsTailLoop);
__ jmpb(L_exit);
}
__ jmpb(L_exit);

__ BIND(L_fillBytes);
#ifdef MUSL_LIBC
{
Label L_byteLoop, L_longByteLoop, L_byteTail, L_byteTailLoop;
UnsafeSetMemoryMark usmm(this, true, true);

#undef wide_value
#define savedSize rax
Expand Down Expand Up @@ -2738,13 +2746,22 @@ address StubGenerator::generate_unsafe_setmemory(const char *name,
__ movq(c_rarg2, rax);

__ xchgq(c_rarg1, c_rarg2);
__ jump(RuntimeAddress(byte_fill_entry));
// generate_unsafe_fill(T_BYTE, false, "unsafe_set_memory");
__ mov(r11, c_rarg2);

__ enter(); // required for proper stackwalking of RuntimeStub frame

{
UnsafeSetMemoryMark usmm(this, true, true);

__ generate_fill(T_BYTE, false, c_rarg0, c_rarg1, r11, rax, xmm0);
}

__ vzeroupper();
__ leave(); // required for proper stackwalking of RuntimeStub frame
__ ret(0);
}
#endif // MUSL_LIBC
__ BIND(L_exit);

restore_arg_regs();
__ ret(0);

#undef dest
#undef size
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/os/windows/os_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2788,12 +2788,16 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
}

bool is_unsafe_arraycopy = (in_native || in_java) && UnsafeCopyMemory::contains_pc(pc);
if (((in_vm || in_native || is_unsafe_arraycopy) && thread->doing_unsafe_access()) ||
bool is_unsafe_setmemory = (in_native || in_java) && UnsafeSetMemory::contains_pc(pc);
if (((in_vm || in_native || is_unsafe_arraycopy || is_unsafe_setmemory) && thread->doing_unsafe_access()) ||
(nm != nullptr && nm->has_unsafe_access())) {
address next_pc = Assembler::locate_next_instruction(pc);
if (is_unsafe_arraycopy) {
next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
}
if (is_unsafe_setmemory) {
next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
}
return Handle_Exception(exceptionInfo, SharedRuntime::handle_unsafe_access(thread, next_pc));
}
}
Expand Down
16 changes: 11 additions & 5 deletions src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,18 +442,21 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
CodeBlob* cb = CodeCache::find_blob(pc);
CompiledMethod* nm = (cb != nullptr) ? cb->as_compiled_method_or_null() : nullptr;
bool is_unsafe_arraycopy = thread->doing_unsafe_access() && UnsafeCopyMemory::contains_pc(pc);
if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy) {
bool is_unsafe_setmemory = thread->doing_unsafe_access() && UnsafeSetMemory::contains_pc(pc);
if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy ||
is_unsafe_setmemory) {
address next_pc = Assembler::locate_next_instruction(pc);
if (is_unsafe_arraycopy) {
next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
}
if (is_unsafe_setmemory) {
next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
}
stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
}
}
else

} else
#ifdef AMD64
if (sig == SIGFPE &&
if (sig == SIGFPE &&
(info->si_code == FPE_INTDIV || info->si_code == FPE_FLTDIV
// Workaround for macOS ARM incorrectly reporting FPE_FLTINV for "div by 0"
// instead of the expected FPE_FLTDIV when running x86_64 binary under Rosetta emulation
Expand Down Expand Up @@ -526,6 +529,9 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
if (UnsafeCopyMemory::contains_pc(pc)) {
next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
}
if (UnsafeSetMemory::contains_pc(pc)) {
next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
}
stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
}

Expand Down
16 changes: 11 additions & 5 deletions src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,21 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
CodeBlob* cb = CodeCache::find_blob(pc);
CompiledMethod* nm = (cb != nullptr) ? cb->as_compiled_method_or_null() : nullptr;
bool is_unsafe_arraycopy = thread->doing_unsafe_access() && UnsafeCopyMemory::contains_pc(pc);
if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy) {
bool is_unsafe_setmemory = thread->doing_unsafe_access() && UnsafeSetMemory::contains_pc(pc);
if ((nm != nullptr && nm->has_unsafe_access()) || is_unsafe_arraycopy ||
is_unsafe_setmemory) {
address next_pc = Assembler::locate_next_instruction(pc);
if (is_unsafe_arraycopy) {
next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
}
if (is_unsafe_setmemory) {
next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
}
stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
}
}
else

} else
#ifdef AMD64
if (sig == SIGFPE &&
if (sig == SIGFPE &&
(info->si_code == FPE_INTDIV || info->si_code == FPE_FLTDIV)) {
stub =
SharedRuntime::
Expand Down Expand Up @@ -320,6 +323,9 @@ bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
if (UnsafeCopyMemory::contains_pc(pc)) {
next_pc = UnsafeCopyMemory::page_error_continue_pc(pc);
}
if (UnsafeSetMemory::contains_pc(pc)) {
next_pc = UnsafeSetMemory::page_error_continue_pc(pc);
}
stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4951,9 +4951,9 @@ bool LibraryCallKit::inline_unsafe_setMemory() {
null_check_receiver(); // null-check receiver
if (stopped()) return true;

C->set_has_unsafe_access(true); // Mark eventual nmethod as "unsafe".
if (StubRoutines::unsafe_setmemory() == nullptr) return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why this check is needed here, since we already check whether the stub is there in is_intrinsic_supported.

Note that inline_unsafe_copyMemory also doesn't have this check. I think it would be good to keep consistency between the two.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.


// printf("In inline_unsafe_setMemory\n");
C->set_has_unsafe_access(true); // Mark eventual nmethod as "unsafe".

Node* dst_base = argument(1); // type: oop
Node* dst_off = ConvL2X(argument(2)); // type: long
Expand Down
52 changes: 52 additions & 0 deletions src/hotspot/share/runtime/stubRoutines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ int UnsafeCopyMemory::_table_length = 0;
int UnsafeCopyMemory::_table_max_length = 0;
address UnsafeCopyMemory::_common_exit_stub_pc = nullptr;

UnsafeSetMemory* UnsafeSetMemory::_table = nullptr;
int UnsafeSetMemory::_table_length = 0;
int UnsafeSetMemory::_table_max_length = 0;
address UnsafeSetMemory::_common_exit_stub_pc = nullptr;

// Implementation of StubRoutines - for a description
// of how to extend it, see the header file.

Expand Down Expand Up @@ -226,6 +231,31 @@ address UnsafeCopyMemory::page_error_continue_pc(address pc) {
return nullptr;
}

void UnsafeSetMemory::create_table(int max_size) {
UnsafeSetMemory::_table = new UnsafeSetMemory[max_size];
UnsafeSetMemory::_table_max_length = max_size;
}

bool UnsafeSetMemory::contains_pc(address pc) {
for (int i = 0; i < UnsafeSetMemory::_table_length; i++) {
UnsafeSetMemory* entry = &UnsafeSetMemory::_table[i];
if (pc >= entry->start_pc() && pc < entry->end_pc()) {
return true;
}
}
return false;
}

address UnsafeSetMemory::page_error_continue_pc(address pc) {
for (int i = 0; i < UnsafeSetMemory::_table_length; i++) {
UnsafeSetMemory* entry = &UnsafeSetMemory::_table[i];
if (pc >= entry->start_pc() && pc < entry->end_pc()) {
return entry->error_exit_pc();
}
}
return nullptr;
}


static BufferBlob* initialize_stubs(StubCodeGenerator::StubsKind kind,
int code_size, int max_aligned_stubs,
Expand Down Expand Up @@ -539,3 +569,25 @@ UnsafeCopyMemoryMark::~UnsafeCopyMemoryMark() {
}
}
}

UnsafeSetMemoryMark::UnsafeSetMemoryMark(StubCodeGenerator* cgen, bool add_entry, bool continue_at_scope_end, address error_exit_pc) {
_cgen = cgen;
_ucm_entry = nullptr;
if (add_entry) {
address err_exit_pc = nullptr;
if (!continue_at_scope_end) {
err_exit_pc = error_exit_pc != nullptr ? error_exit_pc : UnsafeSetMemory::common_exit_stub_pc();
}
assert(err_exit_pc != nullptr || continue_at_scope_end, "error exit not set");
_ucm_entry = UnsafeSetMemory::add_to_table(_cgen->assembler()->pc(), nullptr, err_exit_pc);
}
}

UnsafeSetMemoryMark::~UnsafeSetMemoryMark() {
if (_ucm_entry != nullptr) {
_ucm_entry->set_end_pc(_cgen->assembler()->pc());
if (_ucm_entry->error_exit_pc() == nullptr) {
_ucm_entry->set_error_exit_pc(_cgen->assembler()->pc());
}
}
}
46 changes: 46 additions & 0 deletions src/hotspot/share/runtime/stubRoutines.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,52 @@ class UnsafeCopyMemoryMark : public StackObj {
~UnsafeCopyMemoryMark();
};

class UnsafeSetMemory : public CHeapObj<mtCode> {
private:
address _start_pc;
address _end_pc;
address _error_exit_pc;
public:
static address _common_exit_stub_pc;
static UnsafeSetMemory* _table;
static int _table_length;
static int _table_max_length;
UnsafeSetMemory() : _start_pc(nullptr), _end_pc(nullptr), _error_exit_pc(nullptr) {}
void set_start_pc(address pc) { _start_pc = pc; }
void set_end_pc(address pc) { _end_pc = pc; }
void set_error_exit_pc(address pc) { _error_exit_pc = pc; }
address start_pc() const { return _start_pc; }
address end_pc() const { return _end_pc; }
address error_exit_pc() const { return _error_exit_pc; }

static void set_common_exit_stub_pc(address pc) { _common_exit_stub_pc = pc; }
static address common_exit_stub_pc() { return _common_exit_stub_pc; }

static UnsafeSetMemory* add_to_table(address start_pc, address end_pc, address error_exit_pc) {
guarantee(_table_length < _table_max_length, "Incorrect UnsafeSetMemory::_table_max_length");
UnsafeSetMemory* entry = &_table[_table_length];
entry->set_start_pc(start_pc);
entry->set_end_pc(end_pc);
entry->set_error_exit_pc(error_exit_pc);

_table_length++;
return entry;
}

static bool contains_pc(address pc);
static address page_error_continue_pc(address pc);
static void create_table(int max_size);
};

class UnsafeSetMemoryMark : public StackObj {
private:
UnsafeSetMemory* _ucm_entry;
StubCodeGenerator* _cgen;
public:
UnsafeSetMemoryMark(StubCodeGenerator* cgen, bool add_entry, bool continue_at_scope_end, address error_exit_pc = nullptr);
~UnsafeSetMemoryMark();
};

class StubRoutines: AllStatic {

public:
Expand Down