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

teshull · 2025-05-23T17:37:14Z

This PR adds support for directly retrieving all invokedynamic BootstrapMethodInvocations from a ConstantPool.

In addition, two methods are added to the BootstrapMethodInvocations:

void resolveInvokeDynamic()
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

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Integration blocker

⚠️ Title mismatch between PR and JBS for issue JDK-8357660

Issue

JDK-8357660: [JVMCI] Add support for retrieving all indy BootstrapMethodInvocations directly from ConstantPool (Bug - P4) ⚠️ Title mismatch between PR and JBS.

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25420/head:pull/25420
$ git checkout pull/25420

Update a local copy of the PR:
$ git checkout pull/25420
$ git pull https://git.openjdk.org/jdk.git pull/25420/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25420

View PR using the GUI difftool:
$ git pr show -t 25420

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25420.diff

Using Webrev

Link to Webrev Comment

…onstant Pool.

bridgekeeper · 2025-05-23T17:38:03Z

Hi @teshull, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user teshull" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

openjdk · 2025-05-23T17:38:35Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-05-23T17:39:04Z

@teshull The following labels will be automatically applied to this pull request:

graal
hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

teshull · 2025-05-23T17:52:02Z

/covered

bridgekeeper · 2025-05-23T17:52:49Z

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

dougxc · 2025-05-24T08:49:54Z

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.

teshull · 2025-05-26T13:39:06Z

@dougxc I integrated testing for the new methods into TestDynamicConstant.java now

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

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

dougxc · 2025-05-26T14:14:33Z

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

+     * constant pool contains no invokedynamic BootstrapMethodInvocations, then null
+     * is returned.
+     */
+    BootstrapMethodInvocation[] lookupAllIndyBootstrapMethodInvocations();


Why not make this return all BootstrapMethodInvocations? 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.

Changed to return a list.

Why not make this return all BootstrapMethodInvocations

Within HotSpot it is very easy to pick off all indy BootstrapMethodInvocations via the ConstantPoolCache

Each invokedynamic bytecode location has a unique BootstrapMethodInvocation instance, but they may share the same constant pool entry, so it's not trivial to find all BootstrapMethodInvocations. One would have to iterate both all method bytecodes and constant pool slots, and do some additional filtering.

How about List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean indy)? That is, it either gets the indy or the condy BSM invocations. I can imagine SVM wanting the latter at some point right?

BTW, I noticed that the javadoc for ConstantPool.lookupBootstrapMethodInvocation is somewhat incorrect. Please check and apply these corrections in this PR:

diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java index 2273b256f03..3519af4bcbb 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java @@ -199,12 +199,12 @@ interface BootstrapMethodInvocation { * in the constant pool. * * @param index if {@code opcode} is -1, {@code index} is a constant pool index. Otherwise {@code opcode} - * must be {@code Bytecodes.INVOKEDYNAMIC}, and {@code index} must be the operand of that - * opcode in the bytecode stream (i.e., a {@code rawIndex}). - * @param opcode must be {@code Bytecodes.INVOKEDYNAMIC}, or -1 if + * must be {@code Bytecodes.INVOKEDYNAMIC} or {@code CONSTANT_Dynamic_info}, and {@code index} + * must be the operand of that opcode in the bytecode stream (i.e., a {@code rawIndex}). + * @param opcode must be {@code Bytecodes.INVOKEDYNAMIC}, {@code CONSTANT_Dynamic_info}, or -1 if * {@code index} was not decoded from a bytecode stream * @return the bootstrap method invocation details or {@code null} if the entry specified by {@code index} - * is not a {@code CONSTANT_Dynamic_info} or @{code CONSTANT_InvokeDynamic_info} + * is not a {@code CONSTANT_Dynamic_info} or {@code CONSTANT_InvokeDynamic_info} * @jvms 4.7.23 The {@code BootstrapMethods} Attribute */ default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) {

I prototyped the option List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean indy) here: master...teshull:jdk:jvmci_bootstrap_alternative

As part of this I also prototyped generic BSM resolution / lookup logic

From the SVM perspective, retrieving condys via this new support isn't a big win. It's easy enough already to walk the ConstantPool. However, for symmetry purposes, it is reasonable to have this method (along with the resolve / lookup). What's your preference: this new version or the original?

I like the symmetry of the new version. Also, I think you can simplify things by replacing use of flatMap here with filter.

I updated the javadoc misplaced @ in {@code}. However, the opcode doc changes look wrong to me; the opcode must be -1 or INVOKEDYNAMIC (

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

Line 592 in 04e0fe0

int cpi = opcode == -1 ? index : indyIndexConstantPoolIndex(index, opcode);

)

yeah, looks like you're right. I was basing my assumption on case "Dynamic" in:

@Override public BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) { int cpi = opcode == -1 ? index : indyIndexConstantPoolIndex(index, opcode); final JvmConstant tag = getTagAt(cpi); switch (tag.name) { case "InvokeDynamic": case "Dynamic":

I guess it's possible for an INVOKEDYNAMIC to resolve it's cpi to a CONSTANT_Dynamic entry.

I think INVOKEDYNAMIC should always point to a CONSTANT_InvokeDynamic entry

dougxc · 2025-05-28T17:38:15Z

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

@@ -211,6 +229,12 @@ default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int
        throw new UnsupportedOperationException();
    }

+    /**
+     * Returns the BootstrapMethodInvocation instances for all invokedynamic


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

teshull · 2025-05-29T06:19:39Z

@dougxc I cleaned up the PR to now have the symmetric lookup option and updated the tests

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

Co-authored-by: Douglas Simon <doug.simon@oracle.com>

dougxc · 2025-05-29T20:55:25Z

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

@@ -653,7 +653,9 @@ public List<BootstrapMethodInvocation> lookupBootstrapMethodInvocations(boolean
                            .toList();
                    .toList();
        } else {
-            return IntStream.range(1, length()).filter(cpi -> {
+            return IntStream.range(1, length())
+                            .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"); }

Yes, I applied the suggested change via github, and am just validating it works now (which of course it doesn't). I'll fix it

dougxc · 2025-05-30T08:54:02Z

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).

dougxc · 2025-05-30T08:57:14Z

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

        private final boolean indy;
        private final ResolvedJavaMethod method;
        private final String name;
        private final JavaConstant type;
        private final List<JavaConstant> staticArguments;
+        private final int index;


index -> cpiOrIndyIndex

mlbridge · 2025-05-30T15:06:28Z

Webrevs

00: Full (519be178)

Add support for retrieving all Indy BootstrapMethodInvocations from C…

072467d

…onstant Pool.

bridgekeeper bot added the oca Needs verification of OCA signatory status label May 23, 2025

openjdk bot added graal graal-dev@openjdk.org hotspot-compiler hotspot-compiler-dev@openjdk.org labels May 23, 2025

bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label May 23, 2025

style fixes and add testing to TestDynamicConstants.

5d868f7

dougxc reviewed May 26, 2025

View reviewed changes

address reviewer feedback

99d785d

dougxc reviewed May 28, 2025

View reviewed changes

change to allow both indys and condys to be looked up all at once

fa98e7a

dougxc reviewed May 29, 2025

View reviewed changes

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java Outdated Show resolved Hide resolved

dougxc reviewed May 29, 2025

View reviewed changes

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java Outdated Show resolved Hide resolved

teshull and others added 2 commits May 29, 2025 21:20

commit review suggestion

2557614

Co-authored-by: Douglas Simon <doug.simon@oracle.com>

commit review suggestion

b8123ba

Co-authored-by: Douglas Simon <doug.simon@oracle.com>

dougxc reviewed May 29, 2025

View reviewed changes

complete changes

519be17

dougxc reviewed May 30, 2025

View reviewed changes

bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels May 30, 2025

openjdk bot added the rfr Pull request is ready for review label May 30, 2025

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

Are you sure you want to change the base?

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

Conversation

teshull commented May 23, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Integration blocker

Issue

Reviewing

Uh oh!

bridgekeeper bot commented May 23, 2025

Uh oh!

openjdk bot commented May 23, 2025

Uh oh!

openjdk bot commented May 23, 2025

Uh oh!

teshull commented May 23, 2025

Uh oh!

bridgekeeper bot commented May 23, 2025

Uh oh!

dougxc commented May 24, 2025

Uh oh!

teshull commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teshull May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougxc commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented May 30, 2025

Webrevs

Uh oh!

Uh oh!

teshull commented May 23, 2025 •

edited by openjdk bot

Loading

teshull May 27, 2025 •

edited

Loading

teshull May 27, 2025 •

edited

Loading

teshull May 29, 2025 •

edited

Loading

dougxc commented May 30, 2025 •

edited

Loading