Skip to content

Commit

Permalink
Reland "[vm/ffi] Optimize Pointer operations for statically known types"
Browse files Browse the repository at this point in the history 
Original CL in patchset 1.
Fix for simdbc in patchset 4.
Fix for arm32 precompiled in: https://dart-review.googlesource.com/c/sdk/+/120660/

This CL optimizes Pointer operations in hot loops for Pointer<NativeInteger/NativeDouble/Pointer> (not for structs).

Design: go/dart-ffi-pointers-il

It provides roughly a 100x speedup for the FfiMemory benchmark. The next 5x speedup is to get rid of allocations due to `load` and `store` not being inlined.

FFI API is changed to enable optimizations:

* Disable dynamic invocations of Pointer.load / Pointer.store.
* Disallow implicit downcast of argument passed to Pointer.store.
* Stop zeroing out Pointer.address on Pointer.free().

Issue: #38172

Related issues:

Closes: #35902 (Disallowing dynamic invocations of Pointer ops.)
Closes: #37385 (Function variance checking)
Change-Id: I3921a595fd05026d6ca565ace496771d7c1d877b
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-dartkb-linux-debug-simarm64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-dartkb-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-mac-debug-simdbc64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-reload-mac-release-simdbc64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-precomp-mac-release-simarm_x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/120661
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
  • Loading branch information
@dcharkes
dcharkes authored and commit-bot@chromium.org committed Oct 8, 2019
1 parent 15e8c12 commit d23c824
Show file tree
Hide file tree
Showing 30 changed files with 1,094 additions and 327 deletions.
37 changes: 36 additions & 1 deletion pkg/vm/lib/transformations/ffi.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,22 @@ const nonSizeAlignment = <Abi, Map<NativeType, int>>{
Abi.wordSize32Align64: {},
};

/// Load, store, and elementAt are rewired to their static type for these types.
const List<NativeType> optimizedTypes = [
NativeType.kInt8,
NativeType.kInt16,
NativeType.kInt32,
NativeType.kInt64,
NativeType.kUint8,
NativeType.kUint16,
NativeType.kUint32,
NativeType.kUnit64,
NativeType.kIntptr,
NativeType.kFloat,
NativeType.kDouble,
NativeType.kPointer,
];

/// [FfiTransformer] contains logic which is shared between
/// _FfiUseSiteTransformer and _FfiDefinitionTransformer.
class FfiTransformer extends Transformer {
Expand All @@ -178,6 +194,7 @@ class FfiTransformer extends Transformer {
final Procedure loadMethod;
final Procedure storeMethod;
final Procedure offsetByMethod;
final Procedure elementAtMethod;
final Procedure asFunctionMethod;
final Procedure asFunctionInternal;
final Procedure lookupFunctionMethod;
Expand All @@ -188,6 +205,10 @@ class FfiTransformer extends Transformer {
final Procedure abiMethod;
final Procedure pointerFromFunctionProcedure;
final Procedure nativeCallbackFunctionProcedure;
final Map<NativeType, Procedure> loadMethods;
final Map<NativeType, Procedure> storeMethods;
final Map<NativeType, Procedure> elementAtMethods;
final Procedure loadStructMethod;

/// Classes corresponding to [NativeType], indexed by [NativeType].
final List<Class> nativeTypesClasses;
Expand All @@ -209,6 +230,7 @@ class FfiTransformer extends Transformer {
loadMethod = index.getMember('dart:ffi', 'Pointer', 'load'),
storeMethod = index.getMember('dart:ffi', 'Pointer', 'store'),
offsetByMethod = index.getMember('dart:ffi', 'Pointer', 'offsetBy'),
elementAtMethod = index.getMember('dart:ffi', 'Pointer', 'elementAt'),
addressOfField = index.getMember('dart:ffi', 'Struct', 'addressOf'),
structFromPointer =
index.getMember('dart:ffi', 'Struct', 'fromPointer'),
Expand All @@ -228,7 +250,20 @@ class FfiTransformer extends Transformer {
index.getTopLevelMember('dart:ffi', '_nativeCallbackFunction'),
nativeTypesClasses = nativeTypeClassNames
.map((name) => index.getClass('dart:ffi', name))
.toList();
.toList(),
loadMethods = Map.fromIterable(optimizedTypes, value: (t) {
final name = nativeTypeClassNames[t.index];
return index.getTopLevelMember('dart:ffi', "_load$name");
}),
storeMethods = Map.fromIterable(optimizedTypes, value: (t) {
final name = nativeTypeClassNames[t.index];
return index.getTopLevelMember('dart:ffi', "_store$name");
}),
elementAtMethods = Map.fromIterable(optimizedTypes, value: (t) {
final name = nativeTypeClassNames[t.index];
return index.getTopLevelMember('dart:ffi', "_elementAt$name");
}),
loadStructMethod = index.getTopLevelMember('dart:ffi', '_loadStruct');

/// Computes the Dart type corresponding to a ffi.[NativeType], returns null
/// if it is not a valid NativeType.
Expand Down
52 changes: 42 additions & 10 deletions pkg/vm/lib/transformations/ffi_use_sites.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import 'ffi.dart'
NativeType,
kNativeTypeIntStart,
kNativeTypeIntEnd,
FfiTransformer;
FfiTransformer,
optimizedTypes;

/// Checks and replaces calls to dart:ffi struct fields and methods.
void transformLibraries(
Expand Down Expand Up @@ -316,8 +317,6 @@ class _FfiUseSiteTransformer extends FfiTransformer {
return StaticInvocation(asFunctionInternal,
Arguments([node.receiver], types: [dartType, nativeSignature]));
} else if (target == loadMethod) {
// TODO(dacoharkes): should load and store be generic?
// https://github.com/dart-lang/sdk/issues/35902
final DartType dartType = node.arguments.types[0];
final DartType pointerType = node.receiver.getStaticType(env);
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
Expand All @@ -327,11 +326,20 @@ class _FfiUseSiteTransformer extends FfiTransformer {
_ensureNativeTypeSized(nativeType, node, target.name);
_ensureNativeTypeToDartType(nativeType, dartType, node,
allowStructs: true);

// TODO(37773): When moving to extension methods we can get rid of
// this rewiring.
final Class nativeClass = (nativeType as InterfaceType).classNode;
final NativeType nt = getType(nativeClass);
final typeArguments = [
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
];
return StaticInvocation(
optimizedTypes.contains(nt) ? loadMethods[nt] : loadStructMethod,
Arguments([node.receiver], types: typeArguments));
} else if (target == storeMethod) {
// TODO(dacoharkes): should load and store permitted to be generic?
// https://github.com/dart-lang/sdk/issues/35902
final DartType dartType =
node.arguments.positional[0].getStaticType(env);
final Expression storeValue = node.arguments.positional.single;
final DartType dartType = storeValue.getStaticType(env);
final DartType pointerType = node.receiver.getStaticType(env);
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);

Expand All @@ -341,6 +349,32 @@ class _FfiUseSiteTransformer extends FfiTransformer {
_ensureNativeTypeValid(nativeType, node);
_ensureNativeTypeSized(nativeType, node, target.name);
_ensureNativeTypeToDartType(nativeType, dartType, node);

// TODO(37773): When moving to extension methods we can get rid of
// this rewiring.
final Class nativeClass = (nativeType as InterfaceType).classNode;
final NativeType nt = getType(nativeClass);
final typeArguments = [
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
];
return StaticInvocation(storeMethods[nt],
Arguments([node.receiver, storeValue], types: typeArguments));
} else if (target == elementAtMethod) {
// TODO(37773): When moving to extension methods we can get rid of
// this rewiring.
final DartType pointerType = node.receiver.getStaticType(env);
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
final Class nativeClass = (nativeType as InterfaceType).classNode;
final NativeType nt = getType(nativeClass);
if (optimizedTypes.contains(nt)) {
final typeArguments = [
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
];
return StaticInvocation(
elementAtMethods[nt],
Arguments([node.receiver, node.arguments.positional[0]],
types: typeArguments));
}
}
} on _FfiStaticTypeError {
// It's OK to swallow the exception because the diagnostics issued will
Expand All @@ -361,9 +395,7 @@ class _FfiUseSiteTransformer extends FfiTransformer {
final DartType shouldBeElementType =
convertNativeTypeToDartType(containerTypeArg, allowStructs);
if (elementType == shouldBeElementType) return;
// Both subtypes and implicit downcasts are allowed statically.
if (env.isSubtypeOf(shouldBeElementType, elementType,
SubtypeCheckMode.ignoringNullabilities)) return;
// We disable implicit downcasts, they will go away when NNBD lands.
if (env.isSubtypeOf(elementType, shouldBeElementType,
SubtypeCheckMode.ignoringNullabilities)) return;
diagnosticReporter.report(
Expand Down
Loading

0 comments on commit d23c824

Please sign in to comment.