dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.59k forks source link

Missing direct call opportunity in TFA in equals calls #56691

Open osa1 opened 2 months ago

osa1 commented 2 months ago
import 'dart:typed_data';

void main() {
  final buffer1 = Int8List.fromList([1, 2, 3]).buffer;
  final buffer2 = Object();
  print(buffer1 == buffer2);
}

In this program, when compiling with dart2wasm, TFA infers the concrete type of buffer1, which has an operator == override.

Since the arguments to == are not nullable, we should be able to inline the == call.

However TFA currently does not add the == node to the direct call metadata, so we don't generate a direct call to the == member.

Relevant parts of the kernel after TFA:

final typ::ByteBuffer buffer1 = [@vm.direct-call.metadata=dart._typed_data::_WasmI8ArrayBase.buffer] [@vm.inferred-type.metadata=dart._typed_data::_I8ByteBuffer] [@vm.inferred-type.metadata=dart._typed_data::I8List] typ::Int8List::fromList(<core::int>[1, 2, 3]).{typ::TypedData::buffer}{typ::ByteBuffer};
final core::Object buffer2 = new core::Object::•();
core::print([@vm.inferred-type.metadata=dart.core::bool (receiver not int)] buffer1 =={core::Object::==}{(core::Object) → core::bool} buffer2);

Note that the inferred type of buffer1 is _I8ByteBuffer and buffer2 is not nullable, but the == call is to Object.==, instead of _I8ByteBuffer.==: https://github.com/dart-lang/sdk/blob/0e587759863d44984450246cae46ede3df5be541/sdk/lib/_internal/wasm/lib/typed_data.dart#L1113-L1115

Compilation command:

dart compile wasm \
    -O4 test.dart --no-strip-wasm \
    --extra-compiler-option=--dump-kernel-before-tfa=before.kernel \
    --extra-compiler-option=--dump-kernel-after-tfa=after.kernel

Tried with the main branch at a5baf4e15b0.

cc @mkustermann

dart-github-bot commented 2 months ago

Summary: The issue is that TFA fails to inline the == call for a ByteBuffer object, even though the object's type is known and the == operator is overridden. This results in a call to Object.== instead of the optimized _I8ByteBuffer.== method.

mkustermann commented 2 months ago

/cc @alexmarkov

alexmarkov commented 2 months ago

Analyzing Object.== calls in a typical application involves huge number of combinations of argument types and considerably slows down TFA, so those calls are eagerly approximated:

https://github.com/dart-lang/sdk/blob/0850220ad5cb332acb38ed82c9f29c1be6113f43/pkg/vm/lib/transformations/type_flow/summary_collector.dart#L1959-L1970

Does it make a big difference for dart2wasm? If so, we can probably add a configuration flag to TFA to enable analyzing those calls at the expense of higher compilation time. Still, I think it should be turned off for AOT (and maybe for dart2wasm) by default.

mkustermann commented 2 months ago

@alexmarkov It's not about analyzing the calls. It's about the fact that TFA has inferred a precise type for the receiver of the operator== call, but hasn't annotated the EqualsCall AST node with direct-call-metadata.

Arguably a backend can get the inferred type for receiver and then re-resolve the == method from receiver class, but it would be nice if TFA could annotate it with direct call metadata as it does for all kinds of other calls.

alexmarkov commented 2 months ago

@mkustermann Monomorphic target of a call is detected when the call site is analyzed. Arguments of operator == call are approximated with static types, so the call is not monomorphic and it is not devirtualized. Also note that eager approximation of arguments removes data flow link between result of Int8List.fromList([1, 2, 3]).buffer and receiver of == call, so the inferred type of the receiver doesn't affect the call.

osa1 commented 2 months ago

Does it make a big difference for dart2wasm?

I don't know how big the difference would be with this improved, but for the cases where == is just a type test + identical of some field, right now we can't omit the type test and sometimes we also have to allocate intermediate objects.

Example: https://github.com/dart-lang/sdk/blob/8f89bbe1a97b9941af1ab821522b343f47eaf92f/sdk/lib/_internal/wasm/lib/typed_data.dart#L1128-L1129

When the use site is like x.buffer == y.buffer, we can avoid allocating the byte buffers and then type tests in the == if we can inline ==.