RFR: 8314249: Refactor handling of invokedynamic in JVMCI ConstantPool [v2]

[Doug Simon]

On Wed, 16 Aug 2023 19:34:32 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This PR is part of the clean up JVMCI to track [JDK-8301993](https://bugs.openjdk.org/browse/JDK-8301993), where the  constant pool cache is being removed (as of now, only method references use the CpCache).
>> 
>> 1. `rawIndexToConstantPoolIndex()` is used only for the `invokedynamic` bytecode. It should be renamed to `indyIndexConstantPoolIndex()`
>> 
>> 2. `rawIndexToConstantPoolCacheIndex()` should not be called for the `invokedynamic` bytecode, which doesn't use cpCache entries after [JDK-8301995](https://bugs.openjdk.org/browse/JDK-8301995).
>> 
>> 3. Some `cpi` parameters should be renamed to `rawIndex` or `which`
>> 
>> 4. Added a test case for `ConstantPool.lookupAppendix()`, which was not tested in the JDK repo.
>> 
>> I added comments about the 4 types of indices used in HotSpotConstantPool.java: `cpi`, `rawIndex`, `cpci` and `which`. The latter two types will be removed after [JDK-8301993](https://bugs.openjdk.org/browse/JDK-8301993) is complete.
>> 
>> Note that there are still some incorrect use of `cpi` in the implementation and test cases. Those will be cleaned up in [JDK-8314172](https://bugs.openjdk.org/browse/JDK-8314172)
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @dougxc comments - fixed typos

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

> 720:     @Override
> 721:     public JavaMethod lookupMethod(int rawIndex, int opcode, ResolvedJavaMethod caller) {
> 722:         final int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);

@iklam we need to support `opcode == INVOKEDYNAMIC` here. After this change, we are seeing:

Caused by: java.lang.InternalError: java.lang.IllegalArgumentException: unexpected opcode 186
        at jdk.internal.vm.compiler/org.graalvm.compiler.serviceprovider.GraalServices.lookupMethodWithCaller(GraalServices.java:464)
        at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.lookupMethodInPool(BytecodeParser.java:4274)
        at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.phases.SharedGraphBuilderPhase$SharedBytecodeParser.lookupMethodInPool(SharedGraphBuilderPhase.java:209)
        at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.lookupMethod(BytecodeParser.java:4266)
        at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.genInvokeDynamic(BytecodeParser.java:1725)
        at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.processBytecode(BytecodeParser.java:5363)
        at jdk.internal.vm.compiler/org.graalvm.compiler.java.BytecodeParser.iterateBytecodesForBlock(BytecodeParser.java:3431)
        ... 116 more
Caused by: java.lang.IllegalArgumentException: unexpected opcode 186
        at jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotConstantPool.rawIndexToConstantPoolCacheIndex(HotSpotConstantPool.java:275)
        at jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotConstantPool.lookupMethod(HotSpotConstantPool.java:722)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at jdk.internal.vm.compiler/org.graalvm.compiler.serviceprovider.GraalServices.lookupMethodWithCaller(GraalServices.java:457)
        ... 122 more


when building libgraal. The following patch seems to fix it:


diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java
index d4bb97f03c4..1d32211f1a0 100644
--- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java
+++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java
@@ -719,7 +719,15 @@ private static JavaType getJavaType(final Object type) {
 
     @Override
     public JavaMethod lookupMethod(int rawIndex, int opcode, ResolvedJavaMethod caller) {
-        final int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
+        final int cpci;
+        if (opcode == jdk.vm.ci.hotspot.HotSpotConstantPool.Bytecodes.INVOKEDYNAMIC) {
+            if (!isInvokedynamicIndex(rawIndex)) {
+                throw new IllegalArgumentException("expected a raw index for INVOKEDYNAMIC but got " + rawIndex);
+            }
+            cpci = rawIndex;
+        } else {
+            cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
+        }
         final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, cpci, (byte) opcode, (HotSpotResolvedJavaMethodImpl) caller);
         if (method != null) {
             return method;

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15297#discussion_r1314713759