RFR: 8357660: [JVMCI] Add Support for Retrieving All Indy BootstrapMethodInvocations directly from the ConstantPool

[Doug Simon]

On Fri, 23 May 2025 17:37:14 GMT, Tom Shull <duke at openjdk.org> wrote:

> This PR adds support for directly retrieving all invokedynamic BootstrapMethodInvocations from a ConstantPool.
> 
> In addition, two methods are added to the BootstrapMethodInvocations:
> 1. `void resolveInvokeDynamic()`
> 2. `JavaConstant lookupInvokeDynamicAppendix()`
> 
> The combination of these two features allows one to directly interact with all invokedynamic information of a given ConstantPool without having to iterate through all of the Classfile's methods to find all invokedynamic bytecodes

Please add some tests for the new methods to `test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestDynamicConstant.java`.

I also updated the title of https://bugs.openjdk.org/browse/JDK-8357660 to Not Be All Capitalized so you'll need to fix the title of this PR.
Also, please update both titles and descriptions further to reflect the final changes (i.e. lookupBootstrapMethodInvocations instead of lookupIndyBootstrapMethodInvocations).

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java line 476:

> 474: 
> 475:     /**
> 476:      * Returns the number of {@code ResolvedIndyEntry} present within this constant

`{@code ResolvedIndyEntry}` -> `{@code ResolvedIndyEntry}s`

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

> 538:         private final JavaConstant type;
> 539:         private final List<JavaConstant> staticArguments;
> 540:         private final int index;

index -> cpiOrIndyIndex

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

> 649:                 return List.of();
> 650:             }
> 651:             return IntStream.range(0, numIndys).mapToObj(i -> lookupBootstrapMethodInvocation(i, Bytecodes.INVOKEDYNAMIC))

Suggestion:

            return IntStream.range(0, numIndys)
                            .mapToObj(i -> lookupBootstrapMethodInvocation(i, Bytecodes.INVOKEDYNAMIC))
                            .toList();

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

> 652:                     .toList();
> 653:         } else {
> 654:             return IntStream.range(1, length()).filter(cpi -> {

Suggestion:

            return IntStream.range(1, length())
                            .filter(this::isDynamicEntry)
                            .mapToObj(...);


and:

    private boolean isDynamicEntry(int cpi) {
        JvmConstant tagAt = getTagAt(cpi);
        return tagAt != null && tagAt.name.equals("Dynamic");
    }

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

> 655:         } else {
> 656:             return IntStream.range(1, length())
> 657:                             .filter(this::isDynamicEntry)

Looks like you forgot to add the definition of `isDynamicEntry` that I suggested:

    private boolean isDynamicEntry(int cpi) {
        JvmConstant tagAt = getTagAt(cpi);
        return tagAt != null && tagAt.name.equals("Dynamic");
    }

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java line 198:

> 196:          * If this bootstrap method invocation is for a {@code
> 197:          * CONSTANTAdd_InvokeDynamic_info} pool entry, then this method ensures the
> 198:          * invoke dynamic is resolved. This can be used to compile time resolve the

What exactly does resolving an invoke dynamic mean?
Also I would leave out the sentence about "compile time" unless you clarify exactly what that means.

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java line 233:

> 231: 
> 232:     /**
> 233:      * Returns the BootstrapMethodInvocation instances for all invokedynamic

Point out that the returned list is unmodifiable (like the API for `Stream.toList()` does).

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java line 237:

> 235:      * is returned.
> 236:      */
> 237:     BootstrapMethodInvocation[] lookupAllIndyBootstrapMethodInvocations();

Why not make this return all `BootstrapMethodInvocation`s? The caller can then filter out the indy ones with `isInvokeDynamic`. Also, please return a `List<BootstrapMethodInvocation>` instead of an array - we should never return arrays from JVMCI (see #23159 as an example of addressing existing API). Lastly, return `List.of()` instead of null.

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

PR Comment: https://git.openjdk.org/jdk/pull/25420#issuecomment-2906643446
PR Comment: https://git.openjdk.org/jdk/pull/25420#issuecomment-2921667337
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2107428322
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2115447272
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2114177826
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2114187417
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2114737379
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2107430633
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2112429562
PR Review Comment: https://git.openjdk.org/jdk/pull/25420#discussion_r2107441215