Skip to content

8315771: [JVMCI] Resolution of bootstrap methods with int[] static arguments #15588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed

8315771: [JVMCI] Resolution of bootstrap methods with int[] static arguments #15588

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b0940de
Implement lookup of bootstrap methods with static arguments of type i…
Zeavee Sep 6, 2023
2cdc0b4
Rewrite doc for bootstrapArgumentIndexAt and add second argument to a…
Zeavee Sep 6, 2023
b33ffed
Add more details to the documentation and add a pseudo code example f…
Zeavee Sep 7, 2023
093d0ce
Add missing line of pseudo code and applied modification suggestion
Zeavee Sep 7, 2023
4ec0a7b
Add CachedBSMArgs to resolve arguments lazily instead of letting the …
Zeavee Sep 7, 2023
079969c
Fix BootstrapMethodInvocation.getStaticArguments documentation and ad…
Zeavee Sep 8, 2023
d6aa593
Use lookupConstant instead of lookupConstantInPool in CachedBSMArgs.get
Zeavee Sep 13, 2023
d75e092
Add undefined behavior documentation for bootstrapArgumentIndexAt and…
Zeavee Sep 13, 2023
f4377fc
Update getStaticArguments documentation example and update TestDynami…
Zeavee Sep 14, 2023
9e0627a
Add -XX:UseBootstrapCallInfo=3 to TestDynamicConstant
Zeavee Sep 19, 2023
ea4aa98
Add an extra run configuration for TestDynamicConstant instead of rep…
Zeavee Sep 19, 2023
7c27481
Update BootstrapMethodInvocation.getStaticArguments documentation
Zeavee Sep 20, 2023
7a8d9f2
Iterate over all static arguments in TestDynamicConstant
Zeavee Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,21 +549,27 @@ static class CachedBSMArgs extends AbstractList<JavaConstant> {
/**
* Lazily resolves and caches the argument at the given index and returns it. The method
* {@link CompilerToVM#bootstrapArgumentIndexAt} is used to obtain the constant pool
* index of the entry and the method {@link CompilerToVM#lookupConstantInPool} is used
* to resolve it. If the resolution failed, the index is returned as a
* index of the entry and the method {@link ConstantPool#lookupConstant} is used to
* resolve it. If the resolution failed, the index is returned as a
* {@link PrimitiveConstant}.
*
* @param index index of the element to return
* @return A {@link PrimitiveConstant} representing a {@code CONSTANT_Dynamic_info}
* entry or a {@link JavaConstant} representing the static argument requested
* @return A {@link PrimitiveConstant} representing an unresolved constant pool entry
* or a {@link JavaConstant} representing the static argument requested
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or a {@link JavaConstant}
a PrimitiveConstant is a JavaConstant isn't it?

Copy link
Contributor Author

@Zeavee Zeavee Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a PrimitiveConstant is a JavaConstant isn't it?

Yes, it is, but since the primitives are returned in their boxed form, a PrimitiveConstant can only be a constant pool index, so I think it is better to specify the difference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is that a PrimitiveConstant is a subtype of JavaConstant so the "or" does not make sense. It's like saying "an int or a number".

*/
@Override
public JavaConstant get(int index) {
JavaConstant res = cache[index];
if (res == null) {
int argCpi = compilerToVM().bootstrapArgumentIndexAt(cp, bssIndex, index);
res = compilerToVM().lookupConstantInPool(cp, argCpi, false);
if (res == null) {
Object object = cp.lookupConstant(argCpi, false);
if (object instanceof PrimitiveConstant primitiveConstant) {
res = runtime().getReflection().boxPrimitive(primitiveConstant);
} else if (object instanceof JavaConstant javaConstant) {
res = javaConstant;
} else if (object instanceof JavaType type) {
res = runtime().getReflection().forObject(type);
} else {
res = JavaConstant.forInt(argCpi);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my understanding and my testing, this can only be reached for CONSTANT_Dynamic constant pool entries, as other types of entries should resolve even with resolve = false. Am I correct or am I missing some edge cases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's right based on the spec for ConstantPool.lookupConstant.

}
cache[index] = res;
Expand Down