Skip to content

Commit

Permalink
[vm/ffi] Rewrite sizeOf calls in CFE
Browse files Browse the repository at this point in the history 
This rewrites `sizeOf` calls in the CFE to skip the runtime entry when
the type argument is constant.

The runtime entry is still used when the type argument is generic.
Forcing the type argument to be constant and removing the runtime entry
will be done in follow up CLs.

Bug: #44621
Bug: #38721

TEST=tests/ffi/sizeof_test.dart

Change-Id: I17d14432e6ab22810729be6b5c2939a033d382c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182262
Reviewed-by: Clement Skau <cskau@google.com>
  • Loading branch information
@dcharkes
dcharkes authored and commit-bot@chromium.org committed Feb 2, 2021
1 parent a42244f commit 64bf734
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 30 deletions.
19 changes: 19 additions & 0 deletions pkg/vm/lib/transformations/ffi.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,32 @@ class FfiTransformer extends Transformer {
return FunctionType(argumentTypes, returnType, Nullability.legacy);
}

/// The [NativeType] corresponding to [c]. Returns `null` for user-defined
/// structs.
NativeType getType(Class c) {
final int index = nativeTypesClasses.indexOf(c);
if (index == -1) {
return null;
}
return NativeType.values[index];
}

/// Expression that queries VM internals at runtime to figure out on which ABI
/// we are.
Expression runtimeBranchOnLayout(Map<Abi, int> values) {
return MethodInvocation(
ConstantExpression(
ListConstant(InterfaceType(intClass, Nullability.legacy), [
IntConstant(values[Abi.wordSize64]),
IntConstant(values[Abi.wordSize32Align32]),
IntConstant(values[Abi.wordSize32Align64])
]),
InterfaceType(listClass, Nullability.legacy,
[InterfaceType(intClass, Nullability.legacy)])),
Name("[]"),
Arguments([StaticInvocation(abiMethod, Arguments([]))]),
listElementAt);
}
}

/// Contains all information collected by _FfiDefinitionTransformer that is
Expand Down
33 changes: 8 additions & 25 deletions pkg/vm/lib/transformations/ffi_definitions.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,23 +496,6 @@ class _FfiDefinitionTransformer extends FfiTransformer {
InterfaceType(pragmaClass, Nullability.nonNullable, [])));
}

/// Expression that queries VM internals at runtime to figure out on which ABI
/// we are.
Expression _runtimeBranchOnLayout(Map<Abi, int> values) {
return MethodInvocation(
ConstantExpression(
ListConstant(InterfaceType(intClass, Nullability.legacy), [
IntConstant(values[Abi.wordSize64]),
IntConstant(values[Abi.wordSize32Align32]),
IntConstant(values[Abi.wordSize32Align64])
]),
InterfaceType(listClass, Nullability.legacy,
[InterfaceType(intClass, Nullability.legacy)])),
Name("[]"),
Arguments([StaticInvocation(abiMethod, Arguments([]))]),
listElementAt);
}

Statement _generateGetterStatement(DartType dartType, NativeType type,
int fileOffset, Map<Abi, int> offsets) {
final bool isPointer = type == NativeType.kPointer;
Expand Down Expand Up @@ -558,7 +541,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
addressGetter)
..fileOffset = fileOffset,
numAddition.name,
Arguments([_runtimeBranchOnLayout(offsets)]),
Arguments([runtimeBranchOnLayout(offsets)]),
numAddition)
], types: [
dartType
Expand Down Expand Up @@ -594,9 +577,9 @@ class _FfiDefinitionTransformer extends FfiTransformer {
typedDataOffsetInBytesGetter)
..fileOffset = fileOffset,
numAddition.name,
Arguments([_runtimeBranchOnLayout(offsets)]),
Arguments([runtimeBranchOnLayout(offsets)]),
numAddition),
_runtimeBranchOnLayout(lengths)
runtimeBranchOnLayout(lengths)
]),
byteBufferAsUint8List),
InterfaceType(objectClass, Nullability.nonNullable))
Expand All @@ -610,7 +593,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
Arguments([
PropertyGet(ThisExpression(), addressOfField.name, addressOfField)
..fileOffset = fileOffset,
_runtimeBranchOnLayout(offsets)
runtimeBranchOnLayout(offsets)
]))
..fileOffset = fileOffset;
if (isPointer) {
Expand Down Expand Up @@ -646,12 +629,12 @@ class _FfiDefinitionTransformer extends FfiTransformer {
Arguments([
PropertyGet(ThisExpression(), addressOfField.name, addressOfField)
..fileOffset = fileOffset,
_runtimeBranchOnLayout(offsets),
runtimeBranchOnLayout(offsets),
PropertyGet(
VariableGet(argument), addressOfField.name, addressOfField)
..fileOffset = fileOffset,
ConstantExpression(IntConstant(0)),
_runtimeBranchOnLayout(lengths),
runtimeBranchOnLayout(lengths),
]))
..fileOffset = fileOffset);
}
Expand All @@ -669,7 +652,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
Arguments([
PropertyGet(ThisExpression(), addressOfField.name, addressOfField)
..fileOffset = fileOffset,
_runtimeBranchOnLayout(offsets),
runtimeBranchOnLayout(offsets),
argumentExpression
]))
..fileOffset = fileOffset);
Expand Down Expand Up @@ -719,7 +702,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
final Field sizeOf = Field.immutable(name,
isStatic: true,
isFinal: true,
initializer: _runtimeBranchOnLayout(sizes),
initializer: runtimeBranchOnLayout(sizes),
type: InterfaceType(intClass, Nullability.legacy),
fileUri: struct.fileUri,
getterReference: getterReference)
Expand Down
42 changes: 39 additions & 3 deletions pkg/vm/lib/transformations/ffi_use_sites.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@ import 'package:kernel/target/targets.dart' show DiagnosticReporter;
import 'package:kernel/type_environment.dart';

import 'ffi.dart'
show FfiTransformerData, NativeType, FfiTransformer, optimizedTypes;
show
FfiTransformerData,
NativeType,
FfiTransformer,
optimizedTypes,
nativeTypeSizes,
WORD_SIZE,
UNKNOWN,
wordSize;

/// Checks and replaces calls to dart:ffi struct fields and methods.
void transformLibraries(
Expand Down Expand Up @@ -184,12 +192,18 @@ class _FfiUseSiteTransformer extends FfiTransformer {
} else if (target == sizeOfMethod) {
final DartType nativeType = node.arguments.types[0];

// TODO(http://dartbug.com/38721): Change this to an error after
// package:ffi is no longer using sizeOf generically.
if (!isFfiLibrary) {
_warningNativeTypeValid(nativeType, node);
}

// TODO(http://dartbug.com/38721): Replace calls with constant
// expressions.
if (nativeType is InterfaceType) {
Expression inlineSizeOf = _inlineSizeOf(nativeType);
if (inlineSizeOf != null) {
return inlineSizeOf;
}
}
} else if (target == lookupFunctionMethod) {
final DartType nativeType = InterfaceType(
nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
Expand Down Expand Up @@ -315,6 +329,28 @@ class _FfiUseSiteTransformer extends FfiTransformer {
return node;
}

Expression _inlineSizeOf(InterfaceType nativeType) {
final Class nativeClass = nativeType.classNode;
final NativeType nt = getType(nativeClass);
if (nt == null) {
// User-defined structs.
Field sizeOfField = nativeClass.fields.single;
return StaticGet(sizeOfField);
}
final int size = nativeTypeSizes[nt.index];
if (size == WORD_SIZE) {
return runtimeBranchOnLayout(wordSize);
}
if (size != UNKNOWN) {
return ConstantExpression(
IntConstant(size),
InterfaceType(listClass, Nullability.legacy,
[InterfaceType(intClass, Nullability.legacy)]));
}
// Size unknown.
return null;
}

// We need to replace calls to 'DynamicLibrary.lookupFunction' with explicit
// Kernel, because we cannot have a generic call to 'asFunction' in its body.
//
Expand Down
15 changes: 14 additions & 1 deletion tests/ffi/sizeof_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import 'dart:ffi';

import "package:expect/expect.dart";

import "coordinate.dart";

get is32Bit => 4 == sizeOf<IntPtr>();
get is64Bit => 8 == sizeOf<IntPtr>();

void main() async {
Expect.equals(true, 4 == sizeOf<Pointer>() || 8 == sizeOf<Pointer>());
if (is32Bit) {
Expect.equals(4, sizeOf<Pointer>());
Expect.equals(20, sizeOf<Coordinate>());
}
if (is64Bit) {
Expect.equals(8, sizeOf<Pointer>());
Expect.equals(24, sizeOf<Coordinate>());
}
Expect.throws(() => sizeOf<Void>());
}
15 changes: 14 additions & 1 deletion tests/ffi_2/sizeof_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import 'dart:ffi';

import "package:expect/expect.dart";

import "coordinate.dart";

get is32Bit => 4 == sizeOf<IntPtr>();
get is64Bit => 8 == sizeOf<IntPtr>();

void main() async {
Expect.equals(true, 4 == sizeOf<Pointer>() || 8 == sizeOf<Pointer>());
if (is32Bit) {
Expect.equals(4, sizeOf<Pointer>());
Expect.equals(20, sizeOf<Coordinate>());
}
if (is64Bit) {
Expect.equals(8, sizeOf<Pointer>());
Expect.equals(24, sizeOf<Coordinate>());
}
Expect.throws(() => sizeOf<Void>());
}

0 comments on commit 64bf734

Please sign in to comment.