RFR: 8314078: HotSpotConstantPool.lookupField() asserts due to field changes in ConstantPool.cpp [v2]

Aleksey Shipilev shade at openjdk.org
Tue Aug 15 08:57:16 UTC 2023


On Fri, 11 Aug 2023 20:28:58 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This PR updates Java code in JVMCI to match the C code changes in [JDK-8301996](https://bugs.openjdk.java.net/browse/JDK-8301996) that modified the constant pool layout. Essentially, the indices after a getfield/putfield/getstatic/putstatic bytecode is no longer a CpCacheIndex, but an index for `ConstantPool::resolved_field_entry_at(int field_index)`.
>> 
>> The assertion (and subsequent crash) happen only in debug builds. Out of pure luck, in product build JVMCI produces the correct output even after [JDK-8301996](https://bugs.openjdk.java.net/browse/JDK-8301996), although the code was doing the wrong thing.
>> 
>> This PR has (so far) two commits:
>> 
>> - 6527e67b1832087d180d2b50b65aaeca2e244c28 The actual functional change to use the `rawIndex` that follows a field bytecode.
>> - c322b8e71d4d9e33bd065e64420101010f9127fc Fixed incorrectly named parameters and variables in the JVMCI code and JavaDoc. In most cases, `cpi` needs to be changed to `rawIndex` to reflect the correct type of the index.
>> 
>> To help reviewing, I am limiting the renaming to just those affected by the field changes (without the renames, it's hard to validate that I am actually doing the right thing).
>> 
>> There are still some cases of `cpi` that need to be changed to `rawIndex`. I will fix those in a separate RFE. E.g. in ConstantPool.java:
>> 
>> 
>> default JavaMethod lookupMethod(int cpi, int opcode) {
>>     return lookupMethod(cpi, opcode, null);
>> }
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @dougxc review: Added comments about rawIndex vs cpi

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java line 783:

> 781:     @Override
> 782:     public JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode) {
> 783:         final int cpi = compilerToVM().decodeFieldIndexToCPIndex(this, rawIndex);

I have the new warning here: `cpi` is not used. I guess it is correct, seeing how methods are accepting `rawIndex` now, right?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15237#discussion_r1294346394


More information about the graal-dev mailing list