Skip to content

Commit 2fff774

Browse files
JornVerneeJornVernee
committed
7903585: Revisit variadic support
Reviewed-by: mcimadamore
1 parent f388d6d commit 2fff774

File tree

39 files changed

+260
-222
lines changed

39 files changed

+260
-222
lines changed

‎build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ task jtreg(type: JavaExec) {
210210
"-javaoption:--enable-preview",
211211
"-javaoption:--enable-native-access=org.openjdk.jextract,ALL-UNNAMED",
212212
"-avm", "-conc:auto", "-verbose:summary",
213+
"-retain:fail,error",
213214
"../test"
214215
]
215216
}

‎src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,15 @@ private void emitFunctionWrapper(String javaName, String nativeName, FunctionDes
150150
returnExpr = STR."return (\{retType}) ";
151151
}
152152
String getterName = mangleName(javaName, MethodHandle.class);
153-
String factoryName = isVarArg ?
154-
"downcallHandleVariadic" :
155-
"downcallHandle";
156153
incrAlign();
154+
if (!isVarArg) {
157155
emitDocComment(decl);
158156
appendLines(STR."""
159157
\{MEMBER_MODS} MethodHandle \{getterName}() {
160158
class Holder {
161159
static final FunctionDescriptor DESC = \{descriptorString(2, descriptor)};
162160
163-
static final MethodHandle MH = RuntimeHelper.\{factoryName}(\"\{nativeName}\", DESC);
161+
static final MethodHandle MH = RuntimeHelper.downcallHandle(\"\{nativeName}\", DESC);
164162
}
165163
return RuntimeHelper.requireNonNull(Holder.MH, \"\{javaName}\");
166164
}
@@ -174,6 +172,42 @@ class Holder {
174172
}
175173
}
176174
""");
175+
} else {
176+
String invokerName = javaName + "$invoker";
177+
String invokerFactoryName = javaName + "$makeInvoker";
178+
String paramExprs = paramExprs(declType, finalParamNames, isVarArg);
179+
appendLines(STR."""
180+
public interface \{invokerName} {
181+
\{retType} \{javaName}(\{paramExprs});
182+
}
183+
184+
""");
185+
emitDocComment(decl);
186+
appendLines(STR."""
187+
public static \{invokerName} \{invokerFactoryName}(MemoryLayout... layouts) {
188+
FunctionDescriptor baseDesc$ = \{descriptorString(2, descriptor)};
189+
var mh$ = RuntimeHelper.downcallHandleVariadic("\{nativeName}", baseDesc$, layouts);
190+
return (\{paramExprs}) -> {
191+
try {
192+
\{returnExpr}mh$.invokeExact(\{String.join(", ", finalParamNames)});
193+
} catch(IllegalArgumentException ex$) {
194+
throw ex$; // rethrow IAE from passing wrong number/type of args
195+
} catch (Throwable ex$) {
196+
throw new AssertionError("should not reach here", ex$);
197+
}
198+
};
199+
}
200+
201+
""");
202+
emitDocComment(decl);
203+
String varargsParam = finalParamNames.get(finalParamNames.size() - 1);
204+
appendLines(STR."""
205+
public static \{retType} \{javaName}(\{paramExprs}) {
206+
MemoryLayout[] inferredLayouts$ = RuntimeHelper.inferVariadicLayouts(\{varargsParam});
207+
\{returnExpr}\{invokerFactoryName}(inferredLayouts$).\{javaName}(\{String.join(", ", finalParamNames)});
208+
}
209+
""");
210+
}
177211
decrAlign();
178212
}
179213

@@ -206,7 +240,7 @@ void emitPointerTypedef(Declaration.Typedef typedefTree, String name) {
206240

207241
private boolean primitiveKindSupported(Type.Primitive.Kind kind) {
208242
return switch(kind) {
209-
case Short, Int, Long, LongLong, Float, Double, Char -> true;
243+
case Bool, Short, Int, Long, LongLong, Float, Double, Char -> true;
210244
default -> false;
211245
};
212246
}

‎src/main/resources/org/openjdk/jextract/impl/resources/RuntimeHelper.java.template

Lines changed: 27 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,21 @@ final class RuntimeHelper {
7070
return LINKER.downcallHandle(fdesc);
7171
}
7272

73-
static MethodHandle downcallHandleVariadic(String name, FunctionDescriptor fdesc) {
74-
return SYMBOL_LOOKUP.find(name).
75-
map(addr -> VarargsInvoker.make(addr, fdesc)).
76-
orElse(null);
73+
static MethodHandle downcallHandleVariadic(String name, FunctionDescriptor baseDesc, MemoryLayout[] variadicLayouts) {
74+
FunctionDescriptor variadicDesc = baseDesc.appendArgumentLayouts(variadicLayouts);
75+
Linker.Option fva = Linker.Option.firstVariadicArg(baseDesc.argumentLayouts().size());
76+
return SYMBOL_LOOKUP.find(name)
77+
.map(addr -> LINKER.downcallHandle(addr, variadicDesc, fva)
78+
.asSpreader(Object[].class, variadicLayouts.length))
79+
.orElse(null);
80+
}
81+
82+
static MemoryLayout[] inferVariadicLayouts(Object[] varargs) {
83+
MemoryLayout[] result = new MemoryLayout[varargs.length];
84+
for (int i = 0; i < varargs.length; i++) {
85+
result[i] = variadicLayout(varargs[i].getClass());
86+
}
87+
return result;
7788
}
7889

7990
static MethodHandle upcallHandle(Class<?> fi, String name, FunctionDescriptor fdesc) {
@@ -99,147 +110,18 @@ final class RuntimeHelper {
99110

100111
// Internals only below this point
101112

102-
private static final class VarargsInvoker {
103-
private static final MethodHandle INVOKE_MH;
104-
private final MemorySegment symbol;
105-
private final FunctionDescriptor function;
106-
107-
private VarargsInvoker(MemorySegment symbol, FunctionDescriptor function) {
108-
this.symbol = symbol;
109-
this.function = function;
110-
}
111-
112-
static {
113-
try {
114-
INVOKE_MH = MethodHandles.lookup().findVirtual(VarargsInvoker.class, "invoke", MethodType.methodType(Object.class, SegmentAllocator.class, Object[].class));
115-
} catch (ReflectiveOperationException e) {
116-
throw new RuntimeException(e);
117-
}
118-
}
119-
120-
static MethodHandle make(MemorySegment symbol, FunctionDescriptor function) {
121-
VarargsInvoker invoker = new VarargsInvoker(symbol, function);
122-
MethodHandle handle = INVOKE_MH.bindTo(invoker).asCollector(Object[].class, function.argumentLayouts().size() + 1);
123-
MethodType mtype = MethodType.methodType(function.returnLayout().isPresent() ? carrier(function.returnLayout().get(), true) : void.class);
124-
for (MemoryLayout layout : function.argumentLayouts()) {
125-
mtype = mtype.appendParameterTypes(carrier(layout, false));
126-
}
127-
mtype = mtype.appendParameterTypes(Object[].class);
128-
boolean needsAllocator = function.returnLayout().isPresent() &&
129-
function.returnLayout().get() instanceof GroupLayout;
130-
if (needsAllocator) {
131-
mtype = mtype.insertParameterTypes(0, SegmentAllocator.class);
132-
} else {
133-
handle = MethodHandles.insertArguments(handle, 0, THROWING_ALLOCATOR);
134-
}
135-
return handle.asType(mtype);
136-
}
137-
138-
static Class<?> carrier(MemoryLayout layout, boolean ret) {
139-
if (layout instanceof ValueLayout valueLayout) {
140-
return valueLayout.carrier();
141-
} else if (layout instanceof GroupLayout) {
142-
return MemorySegment.class;
143-
} else {
144-
throw new AssertionError("Cannot get here!");
145-
}
146-
}
147-
148-
private Object invoke(SegmentAllocator allocator, Object[] args) throws Throwable {
149-
// one trailing Object[]
150-
int nNamedArgs = function.argumentLayouts().size();
151-
assert(args.length == nNamedArgs + 1);
152-
// The last argument is the array of vararg collector
153-
Object[] unnamedArgs = (Object[]) args[args.length - 1];
154-
155-
int argsCount = nNamedArgs + unnamedArgs.length;
156-
Class<?>[] argTypes = new Class<?>[argsCount];
157-
MemoryLayout[] argLayouts = new MemoryLayout[nNamedArgs + unnamedArgs.length];
158-
159-
int pos = 0;
160-
for (pos = 0; pos < nNamedArgs; pos++) {
161-
argLayouts[pos] = function.argumentLayouts().get(pos);
162-
}
163-
164-
assert pos == nNamedArgs;
165-
for (Object o: unnamedArgs) {
166-
argLayouts[pos] = variadicLayout(normalize(o.getClass()));
167-
pos++;
168-
}
169-
assert pos == argsCount;
170-
171-
FunctionDescriptor f = (function.returnLayout().isEmpty()) ?
172-
FunctionDescriptor.ofVoid(argLayouts) :
173-
FunctionDescriptor.of(function.returnLayout().get(), argLayouts);
174-
MethodHandle mh = LINKER.downcallHandle(symbol, f);
175-
boolean needsAllocator = function.returnLayout().isPresent() &&
176-
function.returnLayout().get() instanceof GroupLayout;
177-
if (needsAllocator) {
178-
mh = mh.bindTo(allocator);
179-
}
180-
// flatten argument list so that it can be passed to an asSpreader MH
181-
Object[] allArgs = new Object[nNamedArgs + unnamedArgs.length];
182-
System.arraycopy(args, 0, allArgs, 0, nNamedArgs);
183-
System.arraycopy(unnamedArgs, 0, allArgs, nNamedArgs, unnamedArgs.length);
184-
185-
return mh.asSpreader(Object[].class, argsCount).invoke(allArgs);
186-
}
187-
188-
private static Class<?> unboxIfNeeded(Class<?> clazz) {
189-
if (clazz == Boolean.class) {
190-
return boolean.class;
191-
} else if (clazz == Void.class) {
192-
return void.class;
193-
} else if (clazz == Byte.class) {
194-
return byte.class;
195-
} else if (clazz == Character.class) {
196-
return char.class;
197-
} else if (clazz == Short.class) {
198-
return short.class;
199-
} else if (clazz == Integer.class) {
200-
return int.class;
201-
} else if (clazz == Long.class) {
202-
return long.class;
203-
} else if (clazz == Float.class) {
204-
return float.class;
205-
} else if (clazz == Double.class) {
206-
return double.class;
207-
} else {
208-
return clazz;
209-
}
210-
}
211-
212-
private Class<?> promote(Class<?> c) {
213-
if (c == byte.class || c == char.class || c == short.class || c == int.class) {
214-
return long.class;
215-
} else if (c == float.class) {
216-
return double.class;
217-
} else {
218-
return c;
219-
}
220-
}
221-
222-
private Class<?> normalize(Class<?> c) {
223-
c = unboxIfNeeded(c);
224-
if (c.isPrimitive()) {
225-
return promote(c);
226-
}
227-
if (MemorySegment.class.isAssignableFrom(c)) {
228-
return MemorySegment.class;
229-
}
230-
throw new IllegalArgumentException("Invalid type for ABI: " + c.getTypeName());
231-
}
232-
233-
private MemoryLayout variadicLayout(Class<?> c) {
234-
if (c == long.class) {
235-
return JAVA_LONG;
236-
} else if (c == double.class) {
237-
return JAVA_DOUBLE;
238-
} else if (c == MemorySegment.class) {
239-
return ADDRESS;
240-
} else {
241-
throw new IllegalArgumentException("Unhandled variadic argument class: " + c);
242-
}
113+
private static MemoryLayout variadicLayout(Class<?> c) {
114+
// apply default argument promotions per C spec
115+
// note that all primitives are boxed, since they are passed through an Object[]
116+
if (c == Boolean.class || c == Byte.class || c == Character.class || c == Short.class || c == Integer.class) {
117+
return JAVA_INT;
118+
} else if (c == Long.class) {
119+
return JAVA_LONG;
120+
} else if (c == Float.class || c == Double.class) {
121+
return JAVA_DOUBLE;
122+
} else if (MemorySegment.class.isAssignableFrom(c)) {
123+
return ADDRESS;
243124
}
125+
throw new IllegalArgumentException("Invalid type for ABI: " + c.getTypeName());
244126
}
245127
}

‎test.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#include <stdarg.h>
22

3-
int getpid();
3+
int getpid(void);

‎test/TEST.groups

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ native_tests = \
33
jtreg/generator/funcPointerInvokers/TestFuncPointerInvokers.java \
44
jtreg/generator/test8239918/LibTest8239918Test.java \
55
jtreg/generator/test8244938/Test8244938.java \
6-
jtreg/generator/test8244959/Test8244959.java \
6+
jtreg/generator/testPrintf/TestPrintf.java \
77
jtreg/generator/test8245003/Test8245003.java \
88
jtreg/generator/test8246341/LibTest8246341Test.java \
99
jtreg/generator/test8246341/LibTest8246341Test.java \

‎test/jtreg/generator/funcPointerInvokers/TestFuncPointerInvokers.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@
3636
* @test id=classes
3737
* @library /lib
3838
* @run main/othervm JtregJextract -l Func -t test.jextract.funcpointers func.h
39+
* @build TestFuncPointerInvokers
3940
* @run testng/othervm --enable-native-access=ALL-UNNAMED TestFuncPointerInvokers
4041
*/
4142
/*
4243
* @test id=sources
4344
* @library /lib
44-
* @run main/othervm JtregJextract -l Func -t test.jextract.funcpointers func.h
45+
* @run main/othervm JtregJextractSources -l Func -t test.jextract.funcpointers func.h
46+
* @build TestFuncPointerInvokers
4547
* @run testng/othervm --enable-native-access=ALL-UNNAMED TestFuncPointerInvokers
4648
*/
4749
public class TestFuncPointerInvokers {

‎test/jtreg/generator/test7903347/LibTest7903347Test.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
* @summary add long name option for all single letter options and expand help on default values for various options
3131
* @library /lib
3232
* @run main/othervm JtregJextract --library Test7903347 -t test.jextract.test7903347 test7903347.h
33+
* @build LibTest7903347Test
3334
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest7903347Test
3435
*/
3536
/*
@@ -38,6 +39,7 @@
3839
* @summary add long name option for all single letter options and expand help on default values for various options
3940
* @library /lib
4041
* @run main/othervm JtregJextractSources --library Test7903347 -t test.jextract.test7903347 test7903347.h
42+
* @build LibTest7903347Test
4143
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest7903347Test
4244
*/
4345
public class LibTest7903347Test {

‎test/jtreg/generator/test8239918/LibTest8239918Test.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,18 @@
2929
* @test id=classes
3030
* @bug 8239918
3131
* @summary jextract generates uncompilable code for no argument C function
32-
* @library /lib
32+
* @library /lib
3333
* @run main/othervm JtregJextract -l Test8239918 -t test.jextract.test8239918 test8239918.h
34+
* @build LibTest8239918Test
3435
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest8239918Test
3536
*/
3637
/*
3738
* @test id=sources
3839
* @bug 8239918
3940
* @summary jextract generates uncompilable code for no argument C function
40-
* @library /lib
41+
* @library /lib
4142
* @run main/othervm JtregJextractSources -l Test8239918 -t test.jextract.test8239918 test8239918.h
43+
* @build LibTest8239918Test
4244
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest8239918Test
4345
*/
4446
public class LibTest8239918Test {

‎test/jtreg/generator/test8240373/Lib8240373Test.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
* @summary Jextract assigns type "Void" to enum macros
3232
* @library /lib
3333
* @run main/othervm JtregJextract -t test.jextract.test8240373 test8240373.h
34+
* @build Lib8240373Test
3435
* @run testng/othervm --enable-native-access=ALL-UNNAMED Lib8240373Test
3536
*/
3637

@@ -41,6 +42,7 @@
4142
* @library /lib
4243
*
4344
* @run main/othervm JtregJextractSources -t test.jextract.test8240373 test8240373.h
45+
* @build Lib8240373Test
4446
* @run testng/othervm --enable-native-access=ALL-UNNAMED Lib8240373Test
4547
*/
4648

‎test/jtreg/generator/test8244412/LibTest8244412Test.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
* @bug 8244412
3939
* @summary jextract should generate static utils class for primitive typedefs
4040
* @run main/othervm JtregJextract -t test.jextract.test8244412 test8244412.h
41+
* @build LibTest8244412Test
4142
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest8244412Test
4243
*/
4344
/*
@@ -46,6 +47,7 @@
4647
* @bug 8244412
4748
* @summary jextract should generate static utils class for primitive typedefs
4849
* @run main/othervm JtregJextractSources -t test.jextract.test8244412 test8244412.h
50+
* @build LibTest8244412Test
4951
* @run testng/othervm --enable-native-access=ALL-UNNAMED LibTest8244412Test
5052
*/
5153
public class LibTest8244412Test {

0 commit comments

Comments
 (0)