Skip to content

Commit 9692124

Browse files
committed
Revert "[vm/ffi] Optimize @Native calls"
This reverts commit e16bb21. Reason for revert: Indication this caused engine test failures of the kind: ``` �[0;32m[ RUN ] �[mEmbedderA11yTest.A11yTreeIsConsistentUsingV1Callbacks ../../flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc:639: Failure Expected equality of these values: std::strncmp(kTooltip, node->tooltip, sizeof(kTooltip) - 1) Which is: 116 0 ``` Original change's description: > [vm/ffi] Optimize `@Native` calls > > This CL removes static fields for storing the `@Native`'s function > addresses. Instead, the function addresses are stored in the object > pool for all archs except for ia32. ia32 has no AOT and no AppJit > snapshots, so the addresses are directly embedded in the code. > > This CL removes the closure wrapping for `@Native`s. Instead of > `pointer.asFunctionInternal()()` where `asFunction` returns a closure > which contains the trampoline, the function is compiled to a body > which contains the trampoline `Native()`. This is possible for > `@Native`s because the dylib and symbol names are known statically. > > Doing the compilation in kernel_to_il instead of a CFE transform > enables supporting static linking later. (The alternative would have > been to transform in the cfe to a `@pragma('vm:cachable-idempotent')` > instead of constructing the IL in kernel_to_il. > > To enable running resolution in ia32 in kernel_to_il.cc, the > resolution function has been made available via > `runtime/lib/ffi_dynamic_library.h`. > > Because the new calls are simply static calls, the TFA can figure > out const arguments flowing to these calls. This leads to constant > locations in the parameters to FfiCalls. So, this CL also introduces > logic to move constants into `NativeLocation`s. > > TEST=runtime/vm/compiler/backend/il_test.cc > TEST=tests/ffi/function_*_native_(leaf_)test.dart > TEST=pkg/vm/testcases/transformations/ffi/ffinative_compound_return.dart > > Closes: #47625 > Closes: #51618 > Change-Id: Ic5d017005dedcedea40c455c4d24dbe774f91603 > CoreLibraryReviewExempt: Internal FFI implementation changes > Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64c-try,vm-aot-win-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284300 > Commit-Queue: Daco Harkes <[email protected]> > Reviewed-by: Alexander Markov <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> Change-Id: Icc87a6ca33bffecabb15c6b168a06ccc38c2fe5b Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64c-try,vm-aot-win-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333840 Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
1 parent 85e1bbf commit 9692124

34 files changed

+889
-1336
lines changed

pkg/vm/lib/transformations/ffi/common.dart

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,31 +1160,19 @@ class FfiTransformer extends Transformer {
11601160
]))
11611161
..fileOffset = fileOffset;
11621162

1163-
final possibleCompoundReturn = findCompoundReturnType(dartSignature);
1164-
if (possibleCompoundReturn != null) {
1165-
return invokeCompoundConstructor(
1166-
asFunctionInternalInvocation, possibleCompoundReturn);
1163+
if (dartSignature is FunctionType) {
1164+
final returnType = dartSignature.returnType;
1165+
if (returnType is InterfaceType) {
1166+
final clazz = returnType.classNode;
1167+
if (clazz.superclass == structClass || clazz.superclass == unionClass) {
1168+
return invokeCompoundConstructor(asFunctionInternalInvocation, clazz);
1169+
}
1170+
}
11671171
}
11681172

11691173
return asFunctionInternalInvocation;
11701174
}
11711175

1172-
/// Returns the compound [Class] if a compound is returned, otherwise `null`.
1173-
Class? findCompoundReturnType(DartType dartSignature) {
1174-
if (dartSignature is! FunctionType) {
1175-
return null;
1176-
}
1177-
final returnType = dartSignature.returnType;
1178-
if (returnType is! InterfaceType) {
1179-
return null;
1180-
}
1181-
final clazz = returnType.classNode;
1182-
if (clazz.superclass == structClass || clazz.superclass == unionClass) {
1183-
return clazz;
1184-
}
1185-
return null;
1186-
}
1187-
11881176
/// Returns
11891177
/// - `true` if leaf
11901178
/// - `false` if not leaf

pkg/vm/lib/transformations/ffi/native.dart

Lines changed: 119 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,81 @@ class FfiNativeTransformer extends FfiTransformer {
183183
);
184184
}
185185

186+
// Create field holding the resolved native function pointer.
187+
//
188+
// For:
189+
// @FfiNative<IntPtr Function(Pointer<Void>)>('DoXYZ', isLeaf:true)
190+
// external int doXyz(NativeFieldWrapperClass1 obj);
191+
//
192+
// Create:
193+
// static final _doXyz$FfiNative$ptr =
194+
// Pointer<NativeFunction<IntPtr Function(Pointer<Void>)>>
195+
// .fromAddress(_ffi_resolver('..', 'DoXYZ', 1))
196+
// .asFunction<int Function(Pointer<Void>)>(isLeaf:true);
197+
Field _createResolvedFfiNativeField(
198+
String dartFunctionName,
199+
StringConstant nativeFunctionName,
200+
StringConstant? assetName,
201+
bool isLeaf,
202+
FunctionType dartFunctionType,
203+
FunctionType ffiFunctionType,
204+
int fileOffset,
205+
Uri fileUri,
206+
) {
207+
// Derive number of arguments from the native function signature.
208+
final numberNativeArgs = ffiFunctionType.positionalParameters.length;
209+
210+
final nativeFunctionType = InterfaceType(
211+
nativeFunctionClass,
212+
Nullability.legacy,
213+
<DartType>[ffiFunctionType],
214+
);
215+
216+
// _ffi_resolver('...', 'DoXYZ', 1)
217+
final resolverInvocation = FunctionInvocation(
218+
FunctionAccessKind.FunctionType,
219+
StaticGet(resolverField),
220+
Arguments(<Expression>[
221+
ConstantExpression(
222+
assetName ?? StringConstant(currentLibrary.importUri.toString())),
223+
ConstantExpression(nativeFunctionName),
224+
ConstantExpression(IntConstant(numberNativeArgs)),
225+
]),
226+
functionType: resolverField.type as FunctionType)
227+
..fileOffset = fileOffset;
228+
229+
// _fromAddress<NativeFunction<Double Function(Double)>>(...)
230+
final functionPointerExpression = StaticInvocation(
231+
fromAddressInternal,
232+
Arguments(
233+
<Expression>[resolverInvocation],
234+
types: [nativeFunctionType],
235+
))
236+
..fileOffset = fileOffset;
237+
238+
final asFunctionInvocation = buildAsFunctionInternal(
239+
functionPointer: functionPointerExpression,
240+
dartSignature: dartFunctionType,
241+
nativeSignature: ffiFunctionType,
242+
isLeaf: isLeaf,
243+
fileOffset: fileOffset,
244+
);
245+
246+
// static final _doXyz$FfiNative$Ptr = ...
247+
final fieldName =
248+
Name('_$dartFunctionName\$FfiNative\$Ptr', currentLibrary);
249+
final functionPointerField = Field.immutable(fieldName,
250+
type: dartFunctionType,
251+
initializer: asFunctionInvocation,
252+
isStatic: true,
253+
isFinal: true,
254+
fileUri: fileUri,
255+
getterReference: currentLibraryIndex?.lookupGetterReference(fieldName))
256+
..fileOffset = fileOffset;
257+
258+
return functionPointerField;
259+
}
260+
186261
// Whether a parameter of [dartParameterType], passed as [ffiParameterType],
187262
// needs to be converted to Pointer.
188263
bool _requiresPointerConversion(
@@ -275,7 +350,7 @@ class FfiNativeTransformer extends FfiTransformer {
275350
// reachabilityFence(#t0);
276351
// } => #t1
277352
Expression _wrapArgumentsAndReturn({
278-
required StaticInvocation invocation,
353+
required FunctionInvocation invocation,
279354
required FunctionType dartFunctionType,
280355
required FunctionType ffiFunctionType,
281356
bool checkReceiverForNullptr = false,
@@ -412,8 +487,6 @@ class FfiNativeTransformer extends FfiTransformer {
412487
return validSignature;
413488
}
414489

415-
static const vmFfiNative = 'vm:ffi:native';
416-
417490
Procedure _transformProcedure(
418491
Procedure node,
419492
StringConstant nativeFunctionName,
@@ -435,135 +508,74 @@ class FfiNativeTransformer extends FfiTransformer {
435508
return node;
436509
}
437510

438-
final pragmaConstant = ConstantExpression(
439-
InstanceConstant(pragmaClass.reference, [], {
440-
pragmaName.fieldReference: StringConstant(vmFfiNative),
441-
pragmaOptions.fieldReference: InstanceConstant(
442-
nativeClass.reference,
443-
[ffiFunctionType],
444-
{
445-
nativeSymbolField.fieldReference: nativeFunctionName,
446-
nativeAssetField.fieldReference: assetName ??
447-
StringConstant(currentLibrary.importUri.toString()),
448-
nativeIsLeafField.fieldReference: BoolConstant(isLeaf),
449-
},
450-
)
451-
}),
452-
InterfaceType(
453-
pragmaClass,
454-
Nullability.nonNullable,
455-
[],
456-
),
457-
);
458-
459-
final possibleCompoundReturn = findCompoundReturnType(dartFunctionType);
460-
if (dartFunctionType == wrappedDartFunctionType &&
461-
node.isStatic &&
462-
possibleCompoundReturn == null) {
463-
// We are not wrapping/unwrapping arguments or return value.
464-
node.addAnnotation(pragmaConstant);
465-
return node;
466-
}
467-
468-
// Introduce a new function as external with the annotation and give the
469-
// current function a body that does the wrapping/unwrapping.
470-
node.isExternal = false;
471-
472-
node.addAnnotation(ConstantExpression(
473-
InstanceConstant(pragmaClass.reference, [], {
474-
pragmaName.fieldReference: StringConstant("vm:prefer-inline"),
475-
pragmaOptions.fieldReference: NullConstant(),
476-
}),
477-
));
478-
479511
final parent = node.parent;
480-
var fileUri = currentLibrary.fileUri;
481-
if (parent is Class) {
482-
fileUri = parent.fileUri;
483-
} else if (parent is Library) {
484-
fileUri = parent.fileUri;
485-
}
486512

487-
int varCounter = 0;
488-
final nonWrappedFfiNative = Procedure(
489-
Name('_${node.name.text}\$${node.kind.name}\$FfiNative', currentLibrary),
490-
ProcedureKind.Method,
491-
FunctionNode(
492-
/*body=*/ null,
493-
requiredParameterCount: wrappedDartFunctionType.requiredParameterCount,
494-
positionalParameters: [
495-
for (final positionalParameter
496-
in wrappedDartFunctionType.positionalParameters)
497-
VariableDeclaration(
498-
/*name=*/ '#t${varCounter++}',
499-
type: positionalParameter,
500-
)..fileOffset = node.fileOffset,
501-
],
502-
returnType: wrappedDartFunctionType.returnType,
503-
)..fileOffset = node.fileOffset,
504-
fileUri: fileUri,
505-
isStatic: true,
506-
isExternal: true,
507-
)
508-
..isNonNullableByDefault = node.isNonNullableByDefault
509-
..fileOffset = node.fileOffset;
510-
nonWrappedFfiNative.addAnnotation(pragmaConstant);
513+
// static final _myMethod$FfiNative$Ptr = ..
514+
final resolvedField = _createResolvedFfiNativeField(
515+
'${node.name.text}\$${node.kind.name}',
516+
nativeFunctionName,
517+
assetName,
518+
isLeaf,
519+
wrappedDartFunctionType,
520+
ffiFunctionType,
521+
node.fileOffset,
522+
node.fileUri,
523+
);
511524

512-
// Add procedure to the parent the FfiNative function belongs to.
525+
// Add field to the parent the FfiNative function belongs to.
513526
if (parent is Class) {
514-
parent.addProcedure(nonWrappedFfiNative);
527+
parent.addField(resolvedField);
515528
} else if (parent is Library) {
516-
parent.addProcedure(nonWrappedFfiNative);
529+
parent.addField(resolvedField);
517530
} else {
518-
throw 'Unexpected parent of @Native function. '
531+
throw 'Unexpected parent of @FfiNative function. '
519532
'Expected Class or Library, but found ${parent}.';
520533
}
521534

522-
final nonWrappedInvocation = StaticInvocation(
523-
nonWrappedFfiNative,
524-
Arguments(argumentList),
525-
)..fileOffset = node.fileOffset;
535+
// _myFunction$FfiNative$Ptr(obj, x)
536+
final functionPointerInvocation = FunctionInvocation(
537+
FunctionAccessKind.FunctionType,
538+
StaticGet(resolvedField),
539+
Arguments(argumentList),
540+
functionType: wrappedDartFunctionType)
541+
..fileOffset = node.fileOffset;
526542

527543
Expression result = (wrappedDartFunctionType == dartFunctionType
528-
? nonWrappedInvocation
544+
? functionPointerInvocation
529545
: _wrapArgumentsAndReturn(
530-
invocation: nonWrappedInvocation,
546+
invocation: functionPointerInvocation,
531547
dartFunctionType: dartFunctionType,
532548
ffiFunctionType: ffiFunctionType,
533549
checkReceiverForNullptr: checkReceiverForNullptr,
534550
));
535-
if (possibleCompoundReturn != null) {
536-
result = invokeCompoundConstructor(result, possibleCompoundReturn);
537-
}
538551

552+
// => _myFunction$FfiNative$Ptr(
553+
// Pointer<Void>.fromAddress(_getNativeField(obj)), x)
539554
node.function.body = ReturnStatement(result)..parent = node.function;
540555

541556
return node;
542557
}
543558

544559
// Transform FfiNative instance methods.
545-
//
546560
// Example:
547-
//
548561
// class MyNativeClass extends NativeFieldWrapperClass1 {
549-
// @Native<IntPtr Function(Pointer<Void>, IntPtr)>(symbol: 'MyClass_MyMethod')
562+
// @FfiNative<IntPtr Function(Pointer<Void>, IntPtr)>('MyClass_MyMethod')
550563
// external int myMethod(int x);
551564
// }
552-
//
553565
// Becomes, roughly:
554-
//
555566
// ... {
556-
// @pragma(
557-
// 'vm:ffi:native',
558-
// Native<IntPtr Function(Pointer<Void>, IntPtr)>(
559-
// symbol: 'MyClass_MyMethod',
560-
// assetId: '<library uri>',
561-
// ),
562-
// )
563-
// external int _myFunction$FfiNative(Pointer<Void> self, int x);
567+
// static final _myMethod$FfiNative$Ptr = ...
568+
// static _myMethod$FfiNative(MyNativeClass self, int x)
569+
// => _myMethod$FfiNative$Ptr(
570+
// Pointer<Void>.fromAddress(_getNativeField(self)), x);
571+
// int myMethod(int x) => _myMethod$FfiNative(this, x);
572+
// }
564573
//
565-
// @pragma('vm:prefer-inline')
566-
// int myMethod(int x) => _myMethod$FfiNative(_getNativeField(this), x);
574+
// ... {
575+
// static final _myMethod$FfiNative$Ptr = ...
576+
// int myMethod(int x)
577+
// => _myMethod$FfiNative$Ptr(
578+
// Pointer<Void>.fromAddress(_getNativeField(this)), x);
567579
// }
568580
Procedure _transformInstanceMethod(
569581
Procedure node,
@@ -597,26 +609,13 @@ class FfiNativeTransformer extends FfiTransformer {
597609
}
598610

599611
// Transform FfiNative static functions.
600-
//
601612
// Example:
602-
//
603-
// @Native<IntPtr Function(Pointer<Void>, IntPtr)>(symbol: 'MyFunction')
613+
// @FfiNative<IntPtr Function(Pointer<Void>, IntPtr)>('MyFunction')
604614
// external int myFunction(MyNativeClass obj, int x);
605-
//
606615
// Becomes, roughly:
607-
//
608-
// @pragma(
609-
// 'vm:ffi:native',
610-
// Native<IntPtr Function(Pointer<Void>, IntPtr)>(
611-
// symbol: 'MyFunction',
612-
// assetId: '<library uri>',
613-
// ),
614-
// )
615-
// external int _myFunction$FfiNative(Pointer<Void> self, int x);
616-
//
617-
// @pragma('vm:prefer-inline')
616+
// static final _myFunction$FfiNative$Ptr = ...
618617
// int myFunction(MyNativeClass obj, int x)
619-
// => _myFunction$FfiNative(
618+
// => myFunction$FfiNative$Ptr(
620619
// Pointer<Void>.fromAddress(_getNativeField(obj)), x);
621620
Procedure _transformStaticFunction(
622621
Procedure node,
@@ -649,7 +648,7 @@ class FfiNativeTransformer extends FfiTransformer {
649648
@override
650649
visitProcedure(Procedure node) {
651650
// Only transform functions that are external and have FfiNative annotation:
652-
// @Native<Double Function(Double)>(symbol: 'Math_sqrt')
651+
// @FfiNative<Double Function(Double)>('Math_sqrt')
653652
// external double _square_root(double x);
654653
final ffiNativeAnnotation =
655654
tryGetNativeAnnotation(node) ?? tryGetFfiNativeAnnotation(node);
@@ -662,6 +661,7 @@ class FfiNativeTransformer extends FfiTransformer {
662661
1, node.location?.file);
663662
return node;
664663
}
664+
node.isExternal = false;
665665

666666
node.annotations.remove(ffiNativeAnnotation);
667667

pkg/vm/lib/transformations/ffi/use_sites.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
352352
.where((c) =>
353353
c.superclass == structClass || c.superclass == unionClass)
354354
.toList();
355-
return invokeCompoundConstructors(replacement, compoundClasses);
355+
return _invokeCompoundConstructors(replacement, compoundClasses);
356356
} else if (target == allocateMethod) {
357357
final DartType nativeType = node.arguments.types[0];
358358

@@ -387,7 +387,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
387387
return node;
388388
}
389389

390-
Expression invokeCompoundConstructors(
390+
Expression _invokeCompoundConstructors(
391391
Expression nestedExpression, List<Class> compoundClasses) =>
392392
compoundClasses
393393
.distinct()
@@ -748,7 +748,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer {
748748
.map((t) => t.classNode)
749749
.where((c) => c.superclass == structClass || c.superclass == unionClass)
750750
.toList();
751-
return invokeCompoundConstructors(replacement, compoundClasses);
751+
return _invokeCompoundConstructors(replacement, compoundClasses);
752752
}
753753

754754
Expression _replaceGetRef(StaticInvocation node) {

0 commit comments

Comments
 (0)