On Mon, 20 Mar 2023 14:29:35 GMT, Matias Saavedra Silva <[email protected]>
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
>
> Matias Saavedra Silva has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix riscv interpreter mistake and acquire semantics
src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 491:
> 489: } else {
> 490: // Pop N words from the stack
> 491: __ get_cache_and_index_at_bcp(r1, r2, 1, index_size);
This aliasing of `r1` and `cache` is very confusing. Please decide whether to
use the name `cache` or `r1` and do so consistently.
src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 2337:
> 2335: // Load-acquire the adapter method
> 2336: __ lea(method, Address(cache,
> in_bytes(ResolvedIndyEntry::method_offset())));
> 2337: __ ldar(method, method);
What reordering are we trying to prevent here?
src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 2399:
> 2397: bool is_invokevirtual,
> 2398: bool is_invokevfinal,
> /*unused*/
> 2399: bool is_invokedynamic
> /*unused*/) {
Suggestion:
bool /*is_invokedynamic*/) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12778#discussion_r1143191698
PR Review Comment: https://git.openjdk.org/jdk/pull/12778#discussion_r1143193859
PR Review Comment: https://git.openjdk.org/jdk/pull/12778#discussion_r1143195547
