RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry

[Frederic Parain]

On Mon, 27 Feb 2023 21:37:34 GMT, Matias Saavedra Silva <matsaave at 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