Skip to content

Commit 0eff6b3

Browse files
dcharkescommit-bot@chromium.org
authored andcommitted
Revert "[vm/ffi] Optimize Pointer operations for statically known types"
This reverts commit 3712ed2. Reason for revert: Breaks Arm32 precompiled. Issue: #38737 Original change's description: > [vm/ffi] Optimize Pointer operations for statically known types > > 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: I96058d8b5b49052eb6999f084372e6f08b4f6f17 > 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 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117547 > Commit-Queue: Daco Harkes <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> [email protected],[email protected],[email protected] Change-Id: I3b7923ace45beaa9f99119e9ea20c1e52b429ad8 No-Presubmit: true No-Tree-Checks: true No-Try: true Issue: #38172 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 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/120582 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
1 parent db35230 commit 0eff6b3

30 files changed

+327
-1094
lines changed

pkg/vm/lib/transformations/ffi.dart

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -154,22 +154,6 @@ const nonSizeAlignment = <Abi, Map<NativeType, int>>{
154154
Abi.wordSize32Align64: {},
155155
};
156156

157-
/// Load, store, and elementAt are rewired to their static type for these types.
158-
const List<NativeType> optimizedTypes = [
159-
NativeType.kInt8,
160-
NativeType.kInt16,
161-
NativeType.kInt32,
162-
NativeType.kInt64,
163-
NativeType.kUint8,
164-
NativeType.kUint16,
165-
NativeType.kUint32,
166-
NativeType.kUnit64,
167-
NativeType.kIntptr,
168-
NativeType.kFloat,
169-
NativeType.kDouble,
170-
NativeType.kPointer,
171-
];
172-
173157
/// [FfiTransformer] contains logic which is shared between
174158
/// _FfiUseSiteTransformer and _FfiDefinitionTransformer.
175159
class FfiTransformer extends Transformer {
@@ -194,7 +178,6 @@ class FfiTransformer extends Transformer {
194178
final Procedure loadMethod;
195179
final Procedure storeMethod;
196180
final Procedure offsetByMethod;
197-
final Procedure elementAtMethod;
198181
final Procedure asFunctionMethod;
199182
final Procedure asFunctionInternal;
200183
final Procedure lookupFunctionMethod;
@@ -205,10 +188,6 @@ class FfiTransformer extends Transformer {
205188
final Procedure abiMethod;
206189
final Procedure pointerFromFunctionProcedure;
207190
final Procedure nativeCallbackFunctionProcedure;
208-
final Map<NativeType, Procedure> loadMethods;
209-
final Map<NativeType, Procedure> storeMethods;
210-
final Map<NativeType, Procedure> elementAtMethods;
211-
final Procedure loadStructMethod;
212191

213192
/// Classes corresponding to [NativeType], indexed by [NativeType].
214193
final List<Class> nativeTypesClasses;
@@ -230,7 +209,6 @@ class FfiTransformer extends Transformer {
230209
loadMethod = index.getMember('dart:ffi', 'Pointer', 'load'),
231210
storeMethod = index.getMember('dart:ffi', 'Pointer', 'store'),
232211
offsetByMethod = index.getMember('dart:ffi', 'Pointer', 'offsetBy'),
233-
elementAtMethod = index.getMember('dart:ffi', 'Pointer', 'elementAt'),
234212
addressOfField = index.getMember('dart:ffi', 'Struct', 'addressOf'),
235213
structFromPointer =
236214
index.getMember('dart:ffi', 'Struct', 'fromPointer'),
@@ -250,20 +228,7 @@ class FfiTransformer extends Transformer {
250228
index.getTopLevelMember('dart:ffi', '_nativeCallbackFunction'),
251229
nativeTypesClasses = nativeTypeClassNames
252230
.map((name) => index.getClass('dart:ffi', name))
253-
.toList(),
254-
loadMethods = Map.fromIterable(optimizedTypes, value: (t) {
255-
final name = nativeTypeClassNames[t.index];
256-
return index.getTopLevelMember('dart:ffi', "_load$name");
257-
}),
258-
storeMethods = Map.fromIterable(optimizedTypes, value: (t) {
259-
final name = nativeTypeClassNames[t.index];
260-
return index.getTopLevelMember('dart:ffi', "_store$name");
261-
}),
262-
elementAtMethods = Map.fromIterable(optimizedTypes, value: (t) {
263-
final name = nativeTypeClassNames[t.index];
264-
return index.getTopLevelMember('dart:ffi', "_elementAt$name");
265-
}),
266-
loadStructMethod = index.getTopLevelMember('dart:ffi', '_loadStruct');
231+
.toList();
267232

268233
/// Computes the Dart type corresponding to a ffi.[NativeType], returns null
269234
/// if it is not a valid NativeType.

pkg/vm/lib/transformations/ffi_use_sites.dart

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ import 'ffi.dart'
3030
NativeType,
3131
kNativeTypeIntStart,
3232
kNativeTypeIntEnd,
33-
FfiTransformer,
34-
optimizedTypes;
33+
FfiTransformer;
3534

3635
/// Checks and replaces calls to dart:ffi struct fields and methods.
3736
void transformLibraries(
@@ -317,6 +316,8 @@ class _FfiUseSiteTransformer extends FfiTransformer {
317316
return StaticInvocation(asFunctionInternal,
318317
Arguments([node.receiver], types: [dartType, nativeSignature]));
319318
} else if (target == loadMethod) {
319+
// TODO(dacoharkes): should load and store be generic?
320+
// https://github.com/dart-lang/sdk/issues/35902
320321
final DartType dartType = node.arguments.types[0];
321322
final DartType pointerType = node.receiver.getStaticType(env);
322323
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
@@ -326,20 +327,11 @@ class _FfiUseSiteTransformer extends FfiTransformer {
326327
_ensureNativeTypeSized(nativeType, node, target.name);
327328
_ensureNativeTypeToDartType(nativeType, dartType, node,
328329
allowStructs: true);
329-
330-
// TODO(37773): When moving to extension methods we can get rid of
331-
// this rewiring.
332-
final Class nativeClass = (nativeType as InterfaceType).classNode;
333-
final NativeType nt = getType(nativeClass);
334-
final typeArguments = [
335-
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
336-
];
337-
return StaticInvocation(
338-
optimizedTypes.contains(nt) ? loadMethods[nt] : loadStructMethod,
339-
Arguments([node.receiver], types: typeArguments));
340330
} else if (target == storeMethod) {
341-
final Expression storeValue = node.arguments.positional.single;
342-
final DartType dartType = storeValue.getStaticType(env);
331+
// TODO(dacoharkes): should load and store permitted to be generic?
332+
// https://github.com/dart-lang/sdk/issues/35902
333+
final DartType dartType =
334+
node.arguments.positional[0].getStaticType(env);
343335
final DartType pointerType = node.receiver.getStaticType(env);
344336
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
345337

@@ -349,32 +341,6 @@ class _FfiUseSiteTransformer extends FfiTransformer {
349341
_ensureNativeTypeValid(nativeType, node);
350342
_ensureNativeTypeSized(nativeType, node, target.name);
351343
_ensureNativeTypeToDartType(nativeType, dartType, node);
352-
353-
// TODO(37773): When moving to extension methods we can get rid of
354-
// this rewiring.
355-
final Class nativeClass = (nativeType as InterfaceType).classNode;
356-
final NativeType nt = getType(nativeClass);
357-
final typeArguments = [
358-
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
359-
];
360-
return StaticInvocation(storeMethods[nt],
361-
Arguments([node.receiver, storeValue], types: typeArguments));
362-
} else if (target == elementAtMethod) {
363-
// TODO(37773): When moving to extension methods we can get rid of
364-
// this rewiring.
365-
final DartType pointerType = node.receiver.getStaticType(env);
366-
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
367-
final Class nativeClass = (nativeType as InterfaceType).classNode;
368-
final NativeType nt = getType(nativeClass);
369-
if (optimizedTypes.contains(nt)) {
370-
final typeArguments = [
371-
if (nt == NativeType.kPointer) _pointerTypeGetTypeArg(nativeType)
372-
];
373-
return StaticInvocation(
374-
elementAtMethods[nt],
375-
Arguments([node.receiver, node.arguments.positional[0]],
376-
types: typeArguments));
377-
}
378344
}
379345
} on _FfiStaticTypeError {
380346
// It's OK to swallow the exception because the diagnostics issued will
@@ -395,7 +361,9 @@ class _FfiUseSiteTransformer extends FfiTransformer {
395361
final DartType shouldBeElementType =
396362
convertNativeTypeToDartType(containerTypeArg, allowStructs);
397363
if (elementType == shouldBeElementType) return;
398-
// We disable implicit downcasts, they will go away when NNBD lands.
364+
// Both subtypes and implicit downcasts are allowed statically.
365+
if (env.isSubtypeOf(shouldBeElementType, elementType,
366+
SubtypeCheckMode.ignoringNullabilities)) return;
399367
if (env.isSubtypeOf(elementType, shouldBeElementType,
400368
SubtypeCheckMode.ignoringNullabilities)) return;
401369
diagnosticReporter.report(

0 commit comments

Comments
 (0)