Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion src/hotspot/cpu/aarch64/aarch64.ad
Original file line number Diff line number Diff line change
Expand Up @@ -2221,7 +2221,7 @@ void MachUEPNode::emit(CodeBuffer& cbuf, PhaseRegAlloc* ra_) const
{
// This is the unverified entry point.
C2_MacroAssembler _masm(&cbuf);
__ ic_check(CodeEntryAlignment);
__ ic_check(InteriorEntryAlignment);
}

uint MachUEPNode::size(PhaseRegAlloc* ra_) const
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,10 @@ int MacroAssembler::ic_check(int end_alignment) {
Register tmp1 = rscratch1;
Register tmp2 = r10;

Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we could still call verify_oop(receiver) here, but I see that would complicate ic_check_size().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I thought the same. As it's important for correctness that ic_check_size is accurate, I was hoping to have as few different modes in it as possible.

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, offset() + ic_check_size());

int uep_offset = offset();
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,9 @@ AdapterHandlerEntry* SharedRuntime::generate_i2c2i_adapters(MacroAssembler *masm

{
__ block_comment("c2i_unverified_entry {");
// Method might have been compiled since the call site was patched to
// interpreted; if that is the case treat it as a miss so we can get
// the call site corrected.
__ ic_check(1 /* end_alignment */);
__ ldr(rmethod, Address(data, CompiledICData::speculated_method_offset()));

Expand Down Expand Up @@ -1548,6 +1551,8 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
__ clinit_barrier(rscratch2, rscratch1, &L_skip_barrier);
__ far_jump(RuntimeAddress(SharedRuntime::get_handle_wrong_method_stub()));

// Verified entry point must be aligned
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment should come around line 1539.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will fix it.


__ bind(L_skip_barrier);
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/arm/arm.ad
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ void MachUEPNode::format( PhaseRegAlloc *ra_, outputStream *st ) const {

void MachUEPNode::emit(CodeBuffer &cbuf, PhaseRegAlloc *ra_) const {
C2_MacroAssembler _masm(&cbuf);
__ ic_check(CodeEntryAlignment);
__ ic_check(InteriorEntryAlignment);
}

uint MachUEPNode::size(PhaseRegAlloc *ra_) const {
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/cpu/arm/macroAssembler_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,10 @@ int MacroAssembler::ic_check(int end_alignment) {
Register tmp1 = R4;
Register tmp2 = R5;

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, offset() + ic_check_size());

int uep_offset = offset();
Expand All @@ -1886,4 +1890,4 @@ int MacroAssembler::ic_check(int end_alignment) {
jump(SharedRuntime::get_ic_miss_stub(), relocInfo::runtime_call_type);
bind(dont);
return uep_offset;
}
}
3 changes: 2 additions & 1 deletion src/hotspot/cpu/arm/sharedRuntime_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,8 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,

const Register receiver = R0; // see receiverOpr()
__ verify_oop(receiver);
__ ic_check(1 /* end_alignment */);
// Inline cache check
__ ic_check(CodeEntryAlignment /* end_alignment */);

int vep_offset = __ pc() - start;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add back
// Verified entry point
above this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.


Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,10 @@ int MacroAssembler::ic_check(int end_alignment) {
Register tmp1 = R11_scratch1;
Register tmp2 = R12_scratch2;

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, end_alignment, end_alignment - ic_check_size());

int uep_offset = offset();
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3559,6 +3559,10 @@ int MacroAssembler::ic_check(int end_alignment) {
// Hence we can clobber it.
Register tmp2 = t2;

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, ic_check_size());
int uep_offset = offset();

Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/cpu/s390/macroAssembler_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2167,6 +2167,11 @@ int MacroAssembler::ic_check(int end_alignment) {
Register R1_scratch = Z_R1_scratch;
Register R9_data = Z_inline_cache;
Label success, failure;

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, offset() + ic_check_size());

int uep_offset = offset();
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/cpu/x86/macroAssembler_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,10 @@ int MacroAssembler::ic_check(int end_alignment) {
Register data = rax;
Register temp = LP64_ONLY(rscratch1) NOT_LP64(rbx);

Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if VerifyOops check could be added back, maybe as a follow-up RFE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.

// The UEP of a code blob ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately. That's why we align
// before the inline cache check here, and not after
align(end_alignment, offset() + ic_check_size());

int uep_offset = offset();
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
Label exception_pending;

__ verify_oop(receiver);
// verified entry must be aligned for code patching.
__ ic_check(8 /* end_alignment */);

int vep_offset = ((intptr_t)__ pc()) - start;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/x86_64.ad
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,7 @@ void MachUEPNode::format(PhaseRegAlloc* ra_, outputStream* st) const
void MachUEPNode::emit(CodeBuffer& cbuf, PhaseRegAlloc* ra_) const
{
MacroAssembler masm(&cbuf);
masm.ic_check(CodeEntryAlignment);
masm.ic_check(InteriorEntryAlignment);
}

uint MachUEPNode::size(PhaseRegAlloc* ra_) const
Expand Down
19 changes: 4 additions & 15 deletions src/hotspot/share/code/compiledMethod.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,27 +433,16 @@ static void clean_ic_if_metadata_is_dead(CompiledIC *ic) {
}

// Clean references to unloaded nmethods at addr from this one, which is not unloaded.
static void clean_if_nmethod_is_unloaded(CompiledIC *ic, CompiledMethod* from,
template <typename CallsiteT>
static void clean_if_nmethod_is_unloaded(CallsiteT* callsite, CompiledMethod* from,
bool clean_all) {
CodeBlob* cb = CodeCache::find_blob(ic->destination());
CodeBlob* cb = CodeCache::find_blob(callsite->destination());
if (!cb->is_compiled()) {
return;
}
CompiledMethod* cm = cb->as_compiled_method();
if (clean_all || !cm->is_in_use() || cm->is_unloading() || cm->method()->code() != cm) {
ic->set_to_clean();
}
}

static void clean_if_nmethod_is_unloaded(CompiledDirectCall *cdc, CompiledMethod* from,
bool clean_all) {
CodeBlob* cb = CodeCache::find_blob(cdc->destination());
if (!cb->is_compiled()) {
return;
}
CompiledMethod* cm = cb->as_compiled_method();
if (clean_all || !cm->is_in_use() || cm->is_unloading() || cm->method()->code() != cm) {
cdc->set_to_clean();
callsite->set_to_clean();
}
}

Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/oops/oopsHierarchy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ class MethodData;
// class Metadata
class Method;
class ConstantPool;
// class CHeapObj

// The klass hierarchy is separate from the oop hierarchy.

Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/opto/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3413,6 +3413,9 @@ void PhaseOutput::install_code(ciMethod* target,
_code_offsets.set_value(CodeOffsets::OSR_Entry, _first_block_size);
} else {
if (!target->is_static()) {
// The UEP of an nmethod ensures that the VEP is padded. However, the padding of the UEP is placed
// before the inline cache check, so we don't have to execute any nop instructions when dispatching
// through the UEP, yet we can ensure that the VEP is aligned appropriately.
_code_offsets.set_value(CodeOffsets::Entry, _first_block_size - MacroAssembler::ic_check_size());
Copy link
Member

Choose a reason for hiding this comment

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

This looks tricky. I think it means CodeOffsets::Entry starts after the alignment padding NOPs. If that's true then the ic_check functions could use a comment explaining that alignment needs to come first, not last. A comment here wouldn't hurt either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's exactly it. I found that I got a less fortunate nop encoding that actually did show up as a tiny regression. It was fixed by not running the nops. I'll write a comment.

}
_code_offsets.set_value(CodeOffsets::Verified_Entry, _first_block_size);
Expand Down
9 changes: 1 addition & 8 deletions src/hotspot/share/runtime/sharedRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1360,13 +1360,12 @@ methodHandle SharedRuntime::resolve_helper(bool is_virtual, bool is_optimized, T
// we are done patching the code.


CompiledICLocker ml(caller_nm);
if (is_virtual && !is_optimized) {
CompiledICLocker ml(caller_nm);
CompiledIC* inline_cache = CompiledIC_before(caller_nm, caller_frame.pc());
inline_cache->update(&call_info, receiver->klass());
} else {
// Callsite is a direct call - set it to the destination method
CompiledICLocker ml(caller_nm);
CompiledDirectCall* callsite = CompiledDirectCall::before(caller_frame.pc());
callsite->set(callee_method);
}
Expand Down Expand Up @@ -1594,9 +1593,6 @@ methodHandle SharedRuntime::handle_ic_miss_helper(TRAPS) {
JvmtiDynamicCodeEventCollector event_collector;

// Update inline cache to megamorphic. Skip update if we are called from interpreted.
// Transitioning IC caches may require transition stubs. If we run out
// of transition stubs, we have to drop locks and perform a safepoint
// that refills them.
RegisterMap reg_map(current,
RegisterMap::UpdateMap::skip,
RegisterMap::ProcessFrames::include,
Expand Down Expand Up @@ -1661,8 +1657,6 @@ methodHandle SharedRuntime::reresolve_call_site(TRAPS) {
CompiledICLocker ml(caller_nm);
address call_addr = caller_nm->call_instruction_address(pc);

// Check relocations for the matching call to 1) avoid false positives,
// and 2) determine the type.
if (call_addr != nullptr) {
// On x86 the logic for finding a call instruction is blindly checking for a call opcode 5
// bytes back in the instruction stream so we must also check for reloc info.
Expand Down Expand Up @@ -1804,7 +1798,6 @@ JRT_LEAF(void, SharedRuntime::fixup_callers_callsite(Method* method, address cal
return;
}

assert(iter.has_current(), "must have a reloc at java call site");
relocInfo::relocType type = iter.reloc()->type();
if (type != relocInfo::static_call_type &&
type != relocInfo::opt_virtual_call_type) {
Expand Down
5 changes: 0 additions & 5 deletions src/hotspot/share/runtime/vmStructs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,6 @@
nonstatic_field(ArrayKlass, _dimension, int) \
volatile_nonstatic_field(ArrayKlass, _higher_dimension, ObjArrayKlass*) \
volatile_nonstatic_field(ArrayKlass, _lower_dimension, ArrayKlass*) \
volatile_nonstatic_field(CompiledICData, _speculated_method, Method*) \
volatile_nonstatic_field(CompiledICData, _speculated_klass, uintptr_t) \
nonstatic_field(CompiledICData, _itable_defc_klass, Klass*) \
nonstatic_field(CompiledICData, _itable_refc_klass, Klass*) \
nonstatic_field(ConstantPool, _tags, Array<u1>*) \
nonstatic_field(ConstantPool, _cache, ConstantPoolCache*) \
nonstatic_field(ConstantPool, _pool_holder, InstanceKlass*) \
Expand Down Expand Up @@ -1164,7 +1160,6 @@
/* MetadataOopDesc hierarchy (NOTE: some missing) */ \
/**************************************************/ \
\
declare_toplevel_type(CompiledICData) \
declare_toplevel_type(MetaspaceObj) \
declare_type(Metadata, MetaspaceObj) \
declare_type(Klass, Metadata) \
Expand Down