Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v3]

2023-03-15 Thread Matias Saavedra Silva
On Tue, 14 Mar 2023 15:39:39 GMT, Gui Cao  wrote:

>> Matias Saavedra Silva has updated the pull request with a new target base 
>> due to a merge or a rebase. The incremental webrev excludes the unrelated 
>> changes brought in by the merge/rebase. The pull request contains five 
>> additional commits since the last revision:
>> 
>>  - Typo in comment
>>  - Merge branch 'master' into resolvedIndyEntry_8301995
>>  - Interpreter optimization and comments
>>  - PPC and RISCV port
>>  - 8301995: Move invokedynamic resolution information out of the cpCache
>
> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1843:
> 
>> 1841:   ldr(cache, Address(rcpool, 
>> in_bytes(ConstantPoolCache::invokedynamic_entries_offset(;
>> 1842:   // Scale the index to be the entry index * 
>> sizeof(ResolvedInvokeDynamicInfo)
>> 1843:   mov(tmp, sizeof(ResolvedIndyEntry));
> 
> The tmp register is not used here, is it redundant?

Right, the tmp register is not needed anymore thanks to the mul to shift 
optimization. Note that shifting will not be possible on 32-bit systems due to 
the size of ResolvedIndyEntry not being a power of two. This optimization only 
works on 64 bit builds.

-

PR: https://git.openjdk.org/jdk/pull/12778


Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v3]

2023-03-14 Thread Gui Cao
On Tue, 14 Mar 2023 13:59:48 GMT, Matias Saavedra Silva  
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 with a new target base due 
> to a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Typo in comment
>  - Merge branch 'master' into resolvedIndyEntry_8301995
>  - Interpreter optimization and comments
>  - PPC and RISCV port
>  - 8301995: Move invokedynamic resolution information out of the cpCache

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1843:

> 1841:   ldr(cache, Address(rcpool, 
> in_bytes(ConstantPoolCache::invokedynamic_entries_offset(;
> 1842:   // Scale the index to be the entry index * 
> sizeof(ResolvedInvokeDynamicInfo)
> 1843:   mov(tmp, sizeof(ResolvedIndyEntry));

The tmp register is not used here, is it redundant?

-

PR: https://git.openjdk.org/jdk/pull/12778


Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v3]

2023-03-14 Thread Gui Cao
On Tue, 14 Mar 2023 13:59:48 GMT, Matias Saavedra Silva  
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 with a new target base due 
> to a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Typo in comment
>  - Merge branch 'master' into resolvedIndyEntry_8301995
>  - Interpreter optimization and comments
>  - PPC and RISCV port
>  - 8301995: Move invokedynamic resolution information out of the cpCache

Hi, 
I have updated the riscv related code by referring to the latest aarch64 
related changes, please help me to update it.
https://github.com/zifeihan/jdk/commit/ca9f110ca4eb066f828442265f43ed0d9311a9cc
(on this branch: https://github.com/zifeihan/jdk/commits/follow_12778)

@RealFYang @DingliZhang Please help review the RISCV port code.

-

PR: https://git.openjdk.org/jdk/pull/12778


Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v3]

2023-03-14 Thread Matias Saavedra Silva
> 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 with a new target base due 
to a merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains five additional 
commits since the last revision:

 - Typo in comment
 - Merge branch 'master' into resolvedIndyEntry_8301995
 - Interpreter optimization and comments
 - PPC and RISCV port
 - 8301995: Move invokedynamic resolution information out of the cpCache

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12778/files
  - new: https://git.openjdk.org/jdk/pull/12778/files/c2d87e59..a3e7ca02

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=12778=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=12778=01-02

  Stats: 92608 lines in 1481 files changed: 72908 ins; 8825 del; 10875 mod
  Patch: https://git.openjdk.org/jdk/pull/12778.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12778/head:pull/12778

PR: https://git.openjdk.org/jdk/pull/12778