Skip to content

Commit

Permalink
8312235: [JVMCI] ConstantPool should not force eager resolution
Browse files Browse the repository at this point in the history
Reviewed-by: never, matsaave
  • Loading branch information
Doug Simon committed Jul 27, 2023
1 parent 7cbab1f commit 86821a7
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 58 deletions.
35 changes: 23 additions & 12 deletions src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,15 +697,24 @@ C2V_VMENTRY_NULL(jobject, getUncachedStringInPool, (JNIEnv* env, jobject, ARGUME
return JVMCIENV->get_jobject(JVMCIENV->get_object_constant(obj));
C2V_END

C2V_VMENTRY_NULL(jobject, resolvePossiblyCachedConstantInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index))
C2V_VMENTRY_NULL(jobject, lookupConstantInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint cp_index, bool resolve))
constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
oop obj = cp->resolve_possibly_cached_constant_at(index, CHECK_NULL);
constantTag tag = cp->tag_at(index);
oop obj;
if (!resolve) {
bool found_it;
obj = cp->find_cached_constant_at(cp_index, found_it, CHECK_NULL);
if (!found_it) {
return nullptr;
}
} else {
obj = cp->resolve_possibly_cached_constant_at(cp_index, CHECK_NULL);
}
constantTag tag = cp->tag_at(cp_index);
if (tag.is_dynamic_constant()) {
if (obj == nullptr) {
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_NULL_POINTER());
}
BasicType bt = Signature::basic_type(cp->uncached_signature_ref_at(index));
BasicType bt = Signature::basic_type(cp->uncached_signature_ref_at(cp_index));
if (!is_reference_type(bt)) {
if (!is_java_primitive(bt)) {
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_ILLEGAL());
Expand Down Expand Up @@ -1578,16 +1587,18 @@ C2V_VMENTRY_NULL(jobject, iterateFrames, (JNIEnv* env, jobject compilerToVM, job
return nullptr;
C2V_END

C2V_VMENTRY_0(int, resolveInvokeDynamicInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index))
if (!ConstantPool::is_invokedynamic_index(index)) {
JVMCI_THROW_MSG_0(IllegalStateException, err_msg("not an invokedynamic index %d", index));
C2V_VMENTRY_0(int, decodeIndyIndexToCPIndex, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint encoded_indy_index, jboolean resolve))
if (!ConstantPool::is_invokedynamic_index(encoded_indy_index)) {
JVMCI_THROW_MSG_0(IllegalStateException, err_msg("not an encoded indy index %d", encoded_indy_index));
}

constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
CallInfo callInfo;
LinkResolver::resolve_invoke(callInfo, Handle(), cp, index, Bytecodes::_invokedynamic, CHECK_0);
int indy_index = cp->decode_invokedynamic_index(index);
cp->cache()->set_dynamic_call(callInfo, indy_index);
int indy_index = cp->decode_invokedynamic_index(encoded_indy_index);
if (resolve) {
LinkResolver::resolve_invoke(callInfo, Handle(), cp, encoded_indy_index, Bytecodes::_invokedynamic, CHECK_0);
cp->cache()->set_dynamic_call(callInfo, indy_index);
}
return cp->resolved_indy_entry_at(indy_index)->constant_pool_index();
C2V_END

Expand Down Expand Up @@ -3108,13 +3119,13 @@ JNINativeMethod CompilerToVM::methods[] = {
{CC "lookupKlassInPool", CC "(" HS_CONSTANT_POOL2 "I)Ljava/lang/Object;", FN_PTR(lookupKlassInPool)},
{CC "lookupAppendixInPool", CC "(" HS_CONSTANT_POOL2 "I)" OBJECTCONSTANT, FN_PTR(lookupAppendixInPool)},
{CC "lookupMethodInPool", CC "(" HS_CONSTANT_POOL2 "IB" HS_METHOD2 ")" HS_METHOD, FN_PTR(lookupMethodInPool)},
{CC "lookupConstantInPool", CC "(" HS_CONSTANT_POOL2 "IZ)" JAVACONSTANT, FN_PTR(lookupConstantInPool)},
{CC "constantPoolRemapInstructionOperandFromCache", CC "(" HS_CONSTANT_POOL2 "I)I", FN_PTR(constantPoolRemapInstructionOperandFromCache)},
{CC "resolveBootstrapMethod", CC "(" HS_CONSTANT_POOL2 "I)[" OBJECT, FN_PTR(resolveBootstrapMethod)},
{CC "getUncachedStringInPool", CC "(" HS_CONSTANT_POOL2 "I)" JAVACONSTANT, FN_PTR(getUncachedStringInPool)},
{CC "resolvePossiblyCachedConstantInPool", CC "(" HS_CONSTANT_POOL2 "I)" JAVACONSTANT, FN_PTR(resolvePossiblyCachedConstantInPool)},
{CC "resolveTypeInPool", CC "(" HS_CONSTANT_POOL2 "I)" HS_KLASS, FN_PTR(resolveTypeInPool)},
{CC "resolveFieldInPool", CC "(" HS_CONSTANT_POOL2 "I" HS_METHOD2 "B[I)" HS_KLASS, FN_PTR(resolveFieldInPool)},
{CC "resolveInvokeDynamicInPool", CC "(" HS_CONSTANT_POOL2 "I)I", FN_PTR(resolveInvokeDynamicInPool)},
{CC "decodeIndyIndexToCPIndex", CC "(" HS_CONSTANT_POOL2 "IZ)I", FN_PTR(decodeIndyIndexToCPIndex)},
{CC "resolveInvokeHandleInPool", CC "(" HS_CONSTANT_POOL2 "I)V", FN_PTR(resolveInvokeHandleInPool)},
{CC "isResolvedInvokeHandleInPool", CC "(" HS_CONSTANT_POOL2 "I)I", FN_PTR(isResolvedInvokeHandleInPool)},
{CC "resolveMethod", CC "(" HS_KLASS2 HS_METHOD2 HS_KLASS2 ")" HS_METHOD, FN_PTR(resolveMethod)},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,19 +271,22 @@ JavaConstant getUncachedStringInPool(HotSpotConstantPool constantPool, int cpi)
private native JavaConstant getUncachedStringInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi);

/**
* Resolves the entry at index {@code cpi} in {@code constantPool} to an object, looking in the
* Gets the entry at index {@code cpi} in {@code constantPool}, looking in the
* constant pool cache first.
*
* The behavior of this method is undefined if {@code cpi} does not denote one of the following
* entry types: {@code JVM_CONSTANT_Dynamic}, {@code JVM_CONSTANT_String},
* {@code JVM_CONSTANT_MethodHandle}, {@code JVM_CONSTANT_MethodHandleInError},
* {@code JVM_CONSTANT_MethodType} and {@code JVM_CONSTANT_MethodTypeInError}.
*
* @param resolve specifies if a resolved entry is expected. If {@code false},
* {@code null} is returned for an unresolved entry.
*/
JavaConstant resolvePossiblyCachedConstantInPool(HotSpotConstantPool constantPool, int cpi) {
return resolvePossiblyCachedConstantInPool(constantPool, constantPool.getConstantPoolPointer(), cpi);
JavaConstant lookupConstantInPool(HotSpotConstantPool constantPool, int cpi, boolean resolve) {
return lookupConstantInPool(constantPool, constantPool.getConstantPoolPointer(), cpi, resolve);
}

private native JavaConstant resolvePossiblyCachedConstantInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi);
private native JavaConstant lookupConstantInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi, boolean resolve);

/**
* Gets the {@code JVM_CONSTANT_NameAndType} index referenced by the {@code rawIndex}.
Expand Down Expand Up @@ -387,17 +390,18 @@ private native HotSpotResolvedJavaMethodImpl lookupMethodInPool(HotSpotConstantP
long callerMethodPointer);

/**
* Ensures that the type referenced by the specified {@code JVM_CONSTANT_InvokeDynamic} entry at
* index {@code cpi} in {@code constantPool} is loaded and initialized.
* Converts the encoded indy index operand of an invokedynamic instruction
* to an index directly into {@code constantPool}.
*
* @throws IllegalArgumentException if {@code cpi} is not an invokedynamic index
* @return the invokedynamic index
* @param resolve if {@true}, then resolve the entry (which may call a bootstrap method)
* @throws IllegalArgumentException if {@code encoded_indy_index} is not an encoded indy index
* @return {@code JVM_CONSTANT_InvokeDynamic} constant pool entry index for the invokedynamic
*/
int resolveInvokeDynamicInPool(HotSpotConstantPool constantPool, int cpi) {
return resolveInvokeDynamicInPool(constantPool, constantPool.getConstantPoolPointer(), cpi);
int decodeIndyIndexToCPIndex(HotSpotConstantPool constantPool, int encoded_indy_index, boolean resolve) {
return decodeIndyIndexToCPIndex(constantPool, constantPool.getConstantPoolPointer(), encoded_indy_index, resolve);
}

private native int resolveInvokeDynamicInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi);
private native int decodeIndyIndexToCPIndex(HotSpotConstantPool constantPool, long constantPoolPointer, int encoded_indy_index, boolean resolve);

/**
* Resolves the details for invoking the bootstrap method associated with the
Expand Down Expand Up @@ -440,7 +444,7 @@ void resolveInvokeHandleInPool(HotSpotConstantPool constantPool, int cpi) {

/**
* If {@code cpi} denotes an entry representing a resolved dynamic adapter (see
* {@link #resolveInvokeDynamicInPool} and {@link #resolveInvokeHandleInPool}), return the
* {@link #decodeIndyIndexToCPIndex} and {@link #resolveInvokeHandleInPool}), return the
* opcode of the instruction for which the resolution was performed ({@code invokedynamic} or
* {@code invokevirtual}), or {@code -1} otherwise.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,11 @@ JavaConstant getStaticFieldConstantValue(int cpi) {

@Override
public Object lookupConstant(int cpi) {
return lookupConstant(cpi, true);
}

@Override
public Object lookupConstant(int cpi, boolean resolve) {
final JvmConstant tag = getTagAt(cpi);
switch (tag.name) {
case "Integer":
Expand All @@ -658,17 +663,18 @@ public Object lookupConstant(int cpi) {
* "pseudo strings" (arbitrary live objects) patched into a String entry. Such
* entries do not have a symbol in the constant pool slot.
*/
return compilerToVM().resolvePossiblyCachedConstantInPool(this, cpi);
return compilerToVM().lookupConstantInPool(this, cpi, true);
case "MethodHandle":
case "MethodHandleInError":
case "MethodType":
case "MethodTypeInError":
case "Dynamic":
case "DynamicInError":
return compilerToVM().resolvePossiblyCachedConstantInPool(this, cpi);
return compilerToVM().lookupConstantInPool(this, cpi, resolve);
default:
throw new JVMCIError("Unknown constant pool tag %s", tag);
}

}

@Override
Expand Down Expand Up @@ -822,7 +828,7 @@ public int rawIndexToConstantPoolIndex(int rawIndex, int opcode) {
if (opcode != Bytecodes.INVOKEDYNAMIC) {
throw new IllegalArgumentException("expected INVOKEDYNAMIC at " + rawIndex + ", got " + opcode);
}
return compilerToVM().resolveInvokeDynamicInPool(this, rawIndex);
return compilerToVM().decodeIndyIndexToCPIndex(this, rawIndex, false);
}
if (opcode == Bytecodes.INVOKEDYNAMIC) {
throw new IllegalArgumentException("unexpected INVOKEDYNAMIC at " + rawIndex);
Expand Down Expand Up @@ -856,7 +862,7 @@ public void loadReferencedType(int cpi, int opcode, boolean initialize) {
if (!isInvokedynamicIndex(cpi)) {
throw new IllegalArgumentException("must use invokedynamic index but got " + cpi);
}
index = compilerToVM().resolveInvokeDynamicInPool(this, cpi);
index = compilerToVM().decodeIndyIndexToCPIndex(this, cpi, true);
break;
}
case Bytecodes.GETSTATIC:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,20 @@ default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int cpi, int o
*/
Object lookupConstant(int cpi);

/**
* Looks up a constant at the specified index.
*
* If {@code resolve == false} and the denoted constant is of type
* {@code JVM_CONSTANT_Dynamic}, {@code JVM_CONSTANT_MethodHandle} or
* {@code JVM_CONSTANT_MethodType} and it's not yet resolved then
* {@code null} is returned.
*
* @param cpi the constant pool index
* @return the {@code Constant} or {@code JavaType} instance representing the constant pool
* entry
*/
Object lookupConstant(int cpi, boolean resolve);

/**
* Looks up the appendix at the specified index.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ public static HotSpotResolvedObjectType lookupTypeHelper(String name,
}
}

public static Object resolvePossiblyCachedConstantInPool(ConstantPool constantPool, int cpi) {
DirectHotSpotObjectConstantImpl obj = (DirectHotSpotObjectConstantImpl) CTVM.resolvePossiblyCachedConstantInPool((HotSpotConstantPool) constantPool, cpi);
public static Object lookupConstantInPool(ConstantPool constantPool, int cpi, boolean resolve) {
DirectHotSpotObjectConstantImpl obj = (DirectHotSpotObjectConstantImpl) CTVM.lookupConstantInPool((HotSpotConstantPool) constantPool, cpi, resolve);
return obj.object;
}

Expand Down Expand Up @@ -132,11 +132,6 @@ public static HotSpotResolvedJavaMethod lookupMethodInPool(
return CTVM.lookupMethodInPool((HotSpotConstantPool) constantPool, cpi, opcode, null);
}

public static void resolveInvokeDynamicInPool(
ConstantPool constantPool, int cpi) {
CTVM.resolveInvokeDynamicInPool((HotSpotConstantPool) constantPool, cpi);
}

public static void resolveInvokeHandleInPool(
ConstantPool constantPool, int cpi) {
CTVM.resolveInvokeHandleInPool((HotSpotConstantPool) constantPool, cpi);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -41,7 +41,7 @@
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
* -XX:+WhiteBoxAPI -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
* -XX:-UseJVMCICompiler
* compiler.jvmci.compilerToVM.ResolvePossiblyCachedConstantInPoolTest
* compiler.jvmci.compilerToVM.LookupConstantInPoolTest
*/

package compiler.jvmci.compilerToVM;
Expand All @@ -64,15 +64,15 @@
import static compiler.jvmci.compilerToVM.ConstantPoolTestCase.ConstantTypes.CONSTANT_STRING;

/**
* Test for {@code jdk.vm.ci.hotspot.CompilerToVM.resolvePossiblyCachedConstantInPool} method
* Test for {@code jdk.vm.ci.hotspot.CompilerToVM.lookupConstantInPool} method
*/
public class ResolvePossiblyCachedConstantInPoolTest {
public class LookupConstantInPoolTest {

public static void main(String[] args) throws Exception {
Map<ConstantTypes, Validator> typeTests = new HashMap<>();
typeTests.put(CONSTANT_STRING, ResolvePossiblyCachedConstantInPoolTest::validateString);
typeTests.put(CONSTANT_METHODHANDLE, ResolvePossiblyCachedConstantInPoolTest::validateMethodHandle);
typeTests.put(CONSTANT_METHODTYPE, ResolvePossiblyCachedConstantInPoolTest::validateMethodType);
typeTests.put(CONSTANT_STRING, LookupConstantInPoolTest::validateString);
typeTests.put(CONSTANT_METHODHANDLE, LookupConstantInPoolTest::validateMethodHandle);
typeTests.put(CONSTANT_METHODTYPE, LookupConstantInPoolTest::validateMethodType);
ConstantPoolTestCase testCase = new ConstantPoolTestCase(typeTests);
testCase.test();
// The next "Class.forName" and repeating "testCase.test()"
Expand Down Expand Up @@ -103,7 +103,7 @@ private static void validateString(ConstantPool constantPoolCTVM,
index = cpci;
cached = "cached ";
}
Object constantInPool = CompilerToVMHelper.resolvePossiblyCachedConstantInPool(constantPoolCTVM, index);
Object constantInPool = CompilerToVMHelper.lookupConstantInPool(constantPoolCTVM, index, true);
String stringToVerify = (String) constantInPool;
String stringToRefer = entry.name;
if (stringToRefer.equals("") && cpci != ConstantPoolTestsHelper.NO_CP_CACHE_PRESENT) {
Expand All @@ -114,14 +114,14 @@ private static void validateString(ConstantPool constantPoolCTVM,
}

private static final String NOT_NULL_MSG
= "Object returned by resolvePossiblyCachedConstantInPool method should not be null";
= "Object returned by lookupConstantInPool method should not be null";


private static void validateMethodHandle(ConstantPool constantPoolCTVM,
ConstantTypes cpType,
DummyClasses dummyClass,
int index) {
Object constantInPool = CompilerToVMHelper.resolvePossiblyCachedConstantInPool(constantPoolCTVM, index);
Object constantInPool = CompilerToVMHelper.lookupConstantInPool(constantPoolCTVM, index, true);
String msg = String.format("%s for index %d", NOT_NULL_MSG, index);
Asserts.assertNotNull(constantInPool, msg);
if (!(constantInPool instanceof MethodHandle)) {
Expand All @@ -138,7 +138,7 @@ private static void validateMethodType(ConstantPool constantPoolCTVM,
ConstantTypes cpType,
DummyClasses dummyClass,
int index) {
Object constantInPool = CompilerToVMHelper.resolvePossiblyCachedConstantInPool(constantPoolCTVM, index);
Object constantInPool = CompilerToVMHelper.lookupConstantInPool(constantPoolCTVM, index, true);
String msg = String.format("%s for index %d", NOT_NULL_MSG, index);
Asserts.assertNotNull(constantInPool, msg);
Class mtToVerify = constantInPool.getClass();
Expand Down
Loading

1 comment on commit 86821a7

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.