On Mon, 27 Feb 2023 21:37:34 GMT, Matias Saavedra Silva <matsa...@openjdk.org>
wrote:
> The current structure used to store the resolution information for
> invokedynamic, ConstantPoolCacheEntry, is difficult to interpret due to its
> ambigious fields f1 and f2. This structure can hold information for fields,
> methods, and invokedynamics and each of its fields can hold different types
> of values depending on the entry.
>
> This enhancement proposes a new structure to exclusively contain
> invokedynamic information in a manner that is easy to interpret and easy to
> extend. Resolved invokedynamic entries will be stored in an array in the
> constant pool cache and the operand of the invokedynamic bytecode will be
> rewritten to be the index into this array.
>
> Any areas that previously accessed invokedynamic data from
> ConstantPoolCacheEntry will be replaced with accesses to this new array and
> structure. Verified with tier1-9 tests.
>
> The PPC was provided by @reinrich and the RISCV port was provided by
> @DingliZhang and @zifeihan.
>
> This change supports the following platforms: x86, aarch64, PPC, and RISCV
src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1844:
> 1842: // Scale the index to be the entry index *
> sizeof(ResolvedInvokeDynamicInfo)
> 1843: mov(tmp, sizeof(ResolvedIndyEntry));
> 1844: mul(index, index, tmp);
On 64bits platform, sizeof(ResolvedIndyEntry) is 16, a power of two, so shift
instruction could be used instead of a multiply instructions (with an assert in
case the size of ResolvedIndyEntry is changed).
src/hotspot/cpu/x86/interp_masm_x86.cpp line 2075:
> 2073: movptr(cache, Address(rbp, frame::interpreter_frame_cache_offset *
> wordSize));
> 2074: movptr(cache, Address(cache,
> in_bytes(ConstantPoolCache::invokedynamic_entries_offset())));
> 2075: imull(index, index, sizeof(ResolvedIndyEntry)); // Scale the index to
> be the entry index * sizeof(ResolvedInvokeDynamicInfo)
A shift instruction could be used when sizeof(ResolvedIndyEntry) is a power of
two. It is on x86_64 platforms but not on x86_32 platforms (both are using this
file).
Suggested change:
if (is_power_of_2(sizeof(ResolvedIndyEntry))) {
shll(index, log2i(sizeof(ResolvedIndyEntry)));
} else {
imull(index, index, sizeof(ResolvedIndyEntry)); // Scale the index to be
the entry index * sizeof(ResolvedInvokeDynamicInfo)
}
src/hotspot/cpu/x86/templateTable_x86.cpp line 2747:
> 2745: address entry = CAST_FROM_FN_PTR(address,
> InterpreterRuntime::resolve_from_cache);
> 2746: __ movl(method, code); // this is essentially
> Bytecodes::_invokedynamic
> 2747: __ call_VM(noreg, entry, method); // Example uses temp = rbx. In this
> case rbx is method
The comment is confusing and seems to need an update. The register 'method' is
used, but its content is not the method anymore, it is the bytecode.
src/hotspot/cpu/x86/templateTable_x86.cpp line 2770:
> 2768: // since the parameter_size includes it.
> 2769: __ push(rbx);
> 2770: __ mov(rbx, index);
Why is the index (rdx) copied to rbx instead of using the index (rdx) register
directly to call load_resolved_reference_at_index() ? The method doesn't modify
the content of the register.
src/hotspot/share/interpreter/bootstrapInfo.cpp line 67:
> 65: assert(_indy_index != -1, "");
> 66: // Check if method is not null
> 67: if ( _pool->resolved_indy_entry_at(_indy_index)->method() != nullptr) {
_pool->resolved_reference_from_indy(_indy_index) is repeated 5 times. Using a
local variable would make the code easier to read.
-------------
PR: https://git.openjdk.org/jdk/pull/12778
