Skip to content

Commit

Permalink
Simplify logic.
Browse files Browse the repository at this point in the history 
Revert most of the code, but use a configurable limit to keep generated code under control.
Add comments
  • Loading branch information
@mcimadamore
mcimadamore committed Oct 25, 2024
1 parent c92e45e commit e0371b2
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 66 deletions.
102 changes: 42 additions & 60 deletions src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import jdk.internal.foreign.abi.Binding.VMStore;
import jdk.internal.vm.annotation.ForceInline;
import sun.security.action.GetBooleanAction;
import sun.security.action.GetIntegerAction;
import sun.security.action.GetPropertyAction;

import java.io.IOException;
Expand Down Expand Up @@ -80,6 +81,8 @@ public class BindingSpecializer {
= GetPropertyAction.privilegedGetProperty("jdk.internal.foreign.abi.Specializer.DUMP_CLASSES_DIR");
private static final boolean PERFORM_VERIFICATION
= GetBooleanAction.privilegedGetProperty("jdk.internal.foreign.abi.Specializer.PERFORM_VERIFICATION");
private static final int SCOPE_DEDUP_DEPTH
= GetIntegerAction.privilegedGetProperty("jdk.internal.foreign.abi.Specializer.SCOPE_DEDUP_DEPTH", 2);

// Bunch of helper constants
private static final int CLASSFILE_VERSION = ClassFileFormatVersion.latest().major();
Expand Down Expand Up @@ -271,11 +274,19 @@ private void specialize() {
if (callingSequence.forDowncall()) {
returnAllocatorIdx = 0; // first param

// set up scope cache (for acquire/release)
scopeSlots = new int[1];
int scopeLocal = cb.allocateLocal(REFERENCE);
scopeSlots[0] = scopeLocal;
cb.aconst_null().astore(scopeLocal);
// for downcalls we also acquire/release scoped parameters before/after the call
// create a bunch of locals here to keep track of their scopes (to release later)
int[] initialScopeSlots = new int[callerMethodType.parameterCount()];
int numScopes = 0;
for (int i = 0; i < callerMethodType.parameterCount(); i++) {
if (shouldAcquire(i)) {
int scopeLocal = cb.allocateLocal(REFERENCE);
initialScopeSlots[numScopes++] = scopeLocal;
cb.aconst_null()
.astore(scopeLocal); // need to initialize all scope locals here in case an exception occurs
}
}
scopeSlots = Arrays.copyOf(initialScopeSlots, numScopes); // fit to size
curScopeLocalIdx = 0; // used from emitGetInput
}

Expand Down Expand Up @@ -493,63 +504,35 @@ private void emitGetInput() {

private void emitAcquireScope() {
cb.checkcast(CD_AbstractMemorySegmentImpl)
.invokevirtual(CD_AbstractMemorySegmentImpl, "sessionImpl", MTD_SESSION_IMPL);
.invokevirtual(CD_AbstractMemorySegmentImpl, "sessionImpl", MTD_SESSION_IMPL);
Label skipAcquire = cb.newLabel();
Label end = cb.newLabel();

// start with 1 scope to maybe acquire on the stack
assert curScopeLocalIdx != -1;
boolean addressParam = curScopeLocalIdx == 0;
boolean firstParam = curScopeLocalIdx == 1;

if (!addressParam) {
cb.dup()
.aload(scopeSlots[0])
boolean hasLookup = false;

// Here we check if the current scope has not been already acquired.
// To do that, we generate many comparisons (one per cached scope).
// Note that we always skip comparisons against the very first cached scope
// (as that is the function address, which typically belongs to another scope).
// We also stop the comparisons at SCOPE_DEDUP_DEPTH, to keep a lid on the size
// of the generated code.
for (int i = 1; i < curScopeLocalIdx && i <= SCOPE_DEDUP_DEPTH; i++) {
cb.dup() // dup for comparison
.aload(scopeSlots[i])
.if_acmpeq(skipAcquire);
hasLookup = true;
}

if (firstParam) { // only cache by-ref params after the first
// call acquire first here. So that if it fails, we don't call release
cb.dup()
.invokevirtual(CD_MemorySessionImpl, "acquire0", MTD_ACQUIRE0) // call acquire on the other
.astore(scopeSlots[0]);
} else {
cb.invokevirtual(CD_MemorySessionImpl, "acquire0", MTD_ACQUIRE0); // call acquire on the other
}

curScopeLocalIdx++;

if (!addressParam) { // avoid ASM generating a bunch of nops for the dead code
cb.goto_(end)
.labelBinding(skipAcquire)
.pop(); // drop scope
}

cb.labelBinding(end);
}

private void emitReleaseScope() {
cb.checkcast(CD_AbstractMemorySegmentImpl)
.invokevirtual(CD_AbstractMemorySegmentImpl, "sessionImpl", MTD_SESSION_IMPL);
Label skipAcquire = cb.newLabel();
Label end = cb.newLabel();

// start with 1 scope to maybe acquire on the stack
assert curScopeLocalIdx != -1;
boolean addressParam = curScopeLocalIdx == 0;
boolean firstParam = curScopeLocalIdx == 1;
boolean useCache = !addressParam && !firstParam;

if (useCache) {
cb.dup()
.aload(scopeSlots[0])
.if_acmpeq(skipAcquire);
}

cb.invokevirtual(CD_MemorySessionImpl, "release0", MTD_RELEASE0); // call acquire on the other
curScopeLocalIdx++;
// 1 scope to acquire on the stack
cb.dup();
int nextScopeLocal = scopeSlots[curScopeLocalIdx++];
// call acquire first here. So that if it fails, we don't call release
cb.invokevirtual(CD_MemorySessionImpl, "acquire0", MTD_ACQUIRE0) // call acquire on the other
.astore(nextScopeLocal); // store off one to release later

if (useCache) { // avoid ASM generating a bunch of nops for the dead code
if (hasLookup) { // avoid ASM generating a bunch of nops for the dead code
cb.goto_(end)
.labelBinding(skipAcquire)
.pop(); // drop scope
Expand All @@ -559,13 +542,12 @@ private void emitReleaseScope() {
}

private void emitReleaseScopes() {
curScopeLocalIdx = 0; // reset
for (int paramIndex = 0 ; paramIndex < callerMethodType.parameterCount() ; paramIndex++) {
if (shouldAcquire(paramIndex)) {
Class<?> highLevelType = callerMethodType.parameterType(paramIndex);
cb.loadLocal(TypeKind.from(highLevelType), cb.parameterSlot(paramIndex));
emitReleaseScope();
}
for (int scopeLocal : scopeSlots) {
cb.aload(scopeLocal)
.ifThen(Opcode.IFNONNULL, ifCb -> {
ifCb.aload(scopeLocal)
.invokevirtual(CD_MemorySessionImpl, "release0", MTD_RELEASE0);
});
}
}

Expand Down
12 changes: 6 additions & 6 deletions test/micro/org/openjdk/bench/java/lang/foreign/CallByRefHighArity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,22 +86,22 @@ public void tearDown() {

// void noop_params0() {}
private static final MethodHandle MH_NOOP_PARAMS0 = Linker.nativeLinker()
.downcallHandle(FunctionDescriptor.ofVoid())
.downcallHandle(FunctionDescriptor.ofVoid(), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params0").orElseThrow());

// void noop_params1(void *param0) {}
private static final MethodHandle MH_NOOP_PARAMS1 = Linker.nativeLinker()
.downcallHandle(FunctionDescriptor.ofVoid(
ValueLayout.ADDRESS
))
), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params1").orElseThrow());

// void noop_params2(void *param0, void *param1) {}
private static final MethodHandle MH_NOOP_PARAMS2 = Linker.nativeLinker()
.downcallHandle(FunctionDescriptor.ofVoid(
ValueLayout.ADDRESS,
ValueLayout.ADDRESS
))
), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params2").orElseThrow());

// void noop_params3(void *param0, void *param1, void *param2) {}
Expand All @@ -110,7 +110,7 @@ public void tearDown() {
ValueLayout.ADDRESS,
ValueLayout.ADDRESS,
ValueLayout.ADDRESS
))
), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params3").orElseThrow());

// void noop_params4(void *param0, void *param1, void *param2, void *param3) {}
Expand All @@ -120,7 +120,7 @@ public void tearDown() {
ValueLayout.ADDRESS,
ValueLayout.ADDRESS,
ValueLayout.ADDRESS
))
), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params4").orElseThrow());

// void noop_params5(int param0, int param1, void *param2, void *param3, void *param4) {}
Expand All @@ -131,7 +131,7 @@ public void tearDown() {
ValueLayout.ADDRESS,
ValueLayout.ADDRESS,
ValueLayout.ADDRESS
))
), Linker.Option.critical(true))
.bindTo(LOOKUP.find("noop_params5").orElseThrow());

// void noop_params10(int param0, int param1, void *param2, void *param3, void *param4,
Expand Down

0 comments on commit e0371b2

Please sign in to comment.