'canInvokeTypeEquality' was assuming that if a single case was a Type expression, then type equality should be used for the whole switch statement. This is incorrect because if a single case is a type expression but the rest are, for example, ints (as in the fixed test) then the int cases should be compared with identical while only the type expression should be compared with ==.
More importantly, the expected type of the case expression constants should be top type rather than the Type type. Top type is the union of Type and the other case type.
Switches that only include Type expressions will now use "top" type as well but in general switching on a Type expression is an antipattern we shouldn't optimize for. And the impact of not specializing the switch type to Type should be very minimal.
Fixes: https://github.com/dart-lang/sdk/issues/63476
Change-Id: Ib96172f157d8cf3093199e5127d4835e7b25011c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/509920
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This adds a VariableDeclaration node which is used in ForStatement instead of VariableStatement.
This is a step towards removing the initializer from Variable. Long term, VariableDeclaration will own the initializer expression for variables and function parameters will have a defaultValue property instead of using the initializer property for the default value.
TEST=existing
Change-Id: I4a663eeb6006a0f9f098fb2b3e3b502d2ae583b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505681
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
The standalone target used `JSStringImpl` as the name for its string
implementation even though JavaScript isn't involved in that at all.
This was to simplify parts of the compiler which can then refer to both
classes with the same name.
Changing this in the compiler is not that complicated however, so it
makes sense to align the string implementation name with the embedder
terminology we also use for other host imports.
TEST=pkg/dart2wasm/test/ir_tests/standalone.{dart,wat}
Change-Id: I1e112c8a72bb43a7edfa73ff7205d353edc7403a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504581
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This migrates the `String` implementation from using JS interop to
explicit host imports for the standalone target.
This moves a few helper methods shared between the JS and standalone
targets to `dart:_string_helper`. This also moves the embedder regexp
implementation to `dart:_string` to be able to access internals in some
string methods (similar to how the JS implementation special-cases
`JSSyntaxRegExp`).
This removes the final real use of JS-interop in the standalone target.
So, we can:
- Remove internal JS helper libraries from the target.
- Skip JS-interop transformations in the compiler.
- Stop emitting a helper module and support script.
Because `js_interop` is imported in `dart:_wasm`, we can't remove the
library entirely. This replaces it with a stub to avoid compilation
errors, a proper removal is tracked in dartbug.com/63166.
Change-Id: Ide495c210c3a272438deebf8fe4f3f44ba314ffa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501960
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Kevin Moore <kevmoo@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This separates VariableDeclaration from Statement. VariableDeclaration no longer implements Statement and variable declared in a block or in a for-statement are now wrapped by a VariableStatement.
Currently there are two VariableStatement implementations; LegacyVariableStatement for variables in the current model, called LegacyVariable, and VariableInitialization for variables used in the new, still experimental, encoding that supports scope computation.
This CL is a step towards realigning the AST nodes to the new model in which each kind of variable has its own distinct subclass. (LocalVariable, PositionalParameter, NamedParameter, SyntheticVariable, etc.)
Note that it is not the intent to use VariableStatement in ForStatement going forward but that will be handled in a follow-up.
TEST=existing.
Change-Id: I5b309cd62c9b138f95b74fb054686edffa49a393
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502681
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Similar to [0] which recognizes functions that return always nulls, we
recognize functions that never return and
* make the wasm function have no outputs
* make call sites emit `unreachable()` after the call (to inform
binaryen & wasm runtime that this is unreachable)
Before we had an artificial construct where we made such functions
have a `w.RefType(HeapType.none, nullable: false)` return type (i.e.
bottom, i.e. no values) and encoded that way it's unreachable.
We also change some exported wasm functions to explicitly use wasm
types in the signature (namely `WasmVoid` instead of Dart `void`).
[0] https://dart-review.googlesource.com/c/sdk/+/497620
Change-Id: I3724e777cda23c0cf2c8a7dd2e473f3fef0a4f54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499240
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
When there's no expression on the stack but we expect something on the
stack, then the code should be unreachable.
Though the current code would just synthesize a value that matches
the expected type (`convertType(voidMarker, <some type>)`). This
is problematic: If we ever used that synthesized value we may
have incorrect program behavior.
Now there were some valid uses where we synthesize values
* A function that has `void` return type but no explicit return
=> Here we should synthesize `null`
* Synthesize `null` in cases where we know it's not going to be used
=> e.g. for CFE desugaring of `a[i] = b` is roughly
`let tmp = b in (let ignored = a.[]=(tmp) in b)`
where we synthesize `null` as `a.[]=(tmp)` result,
`ignored` isn't used.
* ...
With this CL we no longer allow synthesizing a value of a type
out of thin air, instead all the places where this occurs have
to do that explicitly.
There's some impurities around how setters and index setters
are handled today (and even after this CL). Those impurities
start all the way at CFE, which treats setters and index
setters very differently. See the CFE issue [0].
For those we have two choices:
* special case all call sites that require synthesizing
null values
* special case all call sites that require dropping an
auto synthesized null value
This CL now marks instance setter/index-setter methods as
requiring auto-synthesizeing null values on usage sites and
make code that doesn't need them explicitly drop them.
Somewhat related to this change is how we deal with `void`
on the Dart <-> Wasm Import / Wasm Export boundary: When we
call an imported wasm function that has `void` as return
type (meaning no return values) we have to synthesize a `null`
(as the caller may "use"/"observe" the `void`).
=> We now are more strict and instead use `WasmVoid` as type
instead of allowing `void` as type on the import/export
functions.
[0] https://github.com/dart-lang/sdk/issues/63360
Change-Id: Ie30df3bd68553724437607bab3163c98f5467efe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501061
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Move the creation of
* name of a lambda to `translator.functions.getLambdaFunctionName`
* type of a lambda to `translator.functions.getLambdaFunctionType`
Removes dependency on `w.FunctionBuilder` in the lambda code generators
- as they don't need access to the wasm function. This may allow
inlining closure calls in the future.
Avoid creating `w.FunctionBuilder` eagerly when analyzing closures and
instead create it only when there's a call to it (or the closure object
gets instantiated).
Use `CallTarget` abstraction when invoking lambdas.
Avoid carrying around
`(lambda, enclosingMember, enclosingMemberClosures)` throughout
the codebase and instead store this information on `Lambda`.
=> All these changes make the codebase more uniform between
lambdas and normal functions and also is net code removal.
Change-Id: Ib69566ea9580827be0ed52c0c179884e84599d88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501983
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
This reduces e main module by around -0.3%.
We make the inliner have more precision when estimating the
callee size, especially around constructors but also other
cases.
We are also less generous with budget for iterators, modulo list
iterators which we want to be always inlined.
We also print the inlining decisions in `--print-wasm`
In deferred loading scenario this now inlines constructor bodies
more often than before as the size estimate is better. That in
return avoids us e.g. exporting empty constructor body functions
in the main module to be imported by other modules.
CoreLibraryReviewExempt: Backend-specific function changes.
Change-Id: I320db6f976a5d8c036b40908a5f72e7019d420ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498562
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This gives around 0.2% improvement in compressed e main module.
In Dart a function with `void` return type can actually return values
that callers can observe. But most of the time this doesn't happen, most
times those functions return `null` values and callers don't observe
them.
Let's use inferred return value information to see if a function is
guaranteed to only return `null`. If so we make the wasm function
signature not return any values. Callers will then synthesize a `null`
which may immediatly be dropped or (in rare cases) actually be used.
This leads to less less instructions in the callee (as a callee doesn't
need to push the null onto the stack) and the caller (as the caller
doesn't have to drop it from the stack).
Change-Id: I3ed1be7592798ad0c697c5bc3ab2c4b64c156f03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/497620
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Call sites targeting a procedure annotated with `external-effect` will
not produce any code, including the argument which will not be
evaluated.
However, the single parameter will be treated as 'live' for the purposes
of any global analysis the backends do. This is useful for things like
protobuf shaking where a user may want to retain certain protobuf
messages without actually emitting the code that retains those messages.
Today this functionality is available internally in the vm and wasm SDK
libraries. dart2js has similar functionality represented via the
opaqueTrue and opaqueFalse booleans (which will cause conditional
branches to get shaken after analysis). This will replace dart2js's
opaque(True/False).
This also adds validation to the frontend to ensure a method annotated
with 'external-effect' is well-formed.
Change-Id: If1c4096673e655c58fe7638840a16125003e7809
Tested: Backend tests for codegen were added. A frontend test was added for the validation. A language test was added to confirm the behavior.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/476020
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This reduces essentials main module around -0.4% and possibly
opens up for changes in the inlining (specifically to possibly
not force-inline all initializers anymore)
This shrinks the amount of information
* initializer result values
* the body parameters
* the allocator needs to forward less from initializer to body
We do that by analyzing constructor parameters to see
which parameters are needed for the constructor
Change-Id: I967fa4102ea6e9d498ff07aedabc368b038e1085
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/496341
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This removes 1% uncompressed size from e main module and
may make startup a bit faster.
The element segment of the dynamic dispatch table may have large
strides of identical functions. This happens e.g. when a base
class defines a method and large number of subclasses inherit tha
method.
This CL applies a similar optimization to the dynamic dispatch
table building code as we did in [0] for the normal dispatch
table.
Due to [1] which updated checked-in SDK, new formatter is used
for presubmit checks. Patchset 1-2 is formatting the files, the
actual changes is from Patchset 3+ onwards.
[0] https://dart-review.googlesource.com/c/sdk/+/467843
[1] https://dart-review.googlesource.com/c/sdk/+/484224
Issue https://github.com/dart-lang/sdk/issues/62639
Change-Id: Ia5d63ab31a4167e3ea4c5903ed5db764b6ad3327
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488720
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The CL does a few things:
* Rename "forwarder" to "dispatcher" to disambiguate them from actual
dynamic forwarders:
- "dynamic dispatcher": A function that takes all arguments of a
dynamic call & dispatches them to the right target (which is a
dynamic forwarder).
- "dynamic forwarder": A function that takes arguments, type checks
them, unboxes them (if applicable), calls the real function, boxes
the result (if applicable).
* Make dynamic setter dispatchers & forwarders have `void` return type.
Setters in Dart don't produce values, the call sites turn `a.b = c`
into something like this: `let tmp = c in (let a.b=(tmp) in tmp)`
* Make `CallShape` remember not just the argument shape but also whether
it was a getter, setter or method.
Issue https://github.com/dart-lang/sdk/issues/62639
Change-Id: Ia616f504a3ed3e9fa86a93d274af794b9437e5ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488081
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
The code in `generateNoSuchMethodCall` is calling the `noSuchMethod`
instance method. Doing so required duplication of logic from
normal instance invocations.
Instead we call a static method in core libraries that will perform the
instance invocation. Compiling that static method will then use the
normal logic we have for instance invocations (instead of duplicating
that logic in the code generator).
Part of https://github.com/dart-lang/sdk/issues/62639
Change-Id: Ic6a5e5bc7a87baa2039af551bd7a01b1557017a4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488160
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This CL is part of an effort to bump the SDK requirement to `3.12.0-0`
for all the packages in `pkg` that are not published to `pub`, so that
we can get better testing of the "private named parameters" feature.
(Packages that *are* published to `pub` can't be safely bumped yet,
because SDK 3.12 hasn't been released, and I don't want to block those
packages' ability to publish useful updates to customers.)
This change covers the following packages, which are owned by
OWNERS_WASM:
- pkg/dart2wasm
- pkg/wasm_builder
Changes to `pubspec.yaml` files were made manually.
Changes to `.dart` files were made automatically, using `dart fix` to
fix failures in the unnecessary_underscores lint.
Change-Id: Ia83ba9109dcd3d488755d2df990dd2496a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487944
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Currently a dynamic call will call a helper which will create
3 array objects (for type, positional and named arguments).
It will then pass those wasm arrays to type checker methods.
Those will then get values out of the array, type check and call
the target.
We change this now such that in the normal case (**) we avoid
the array creations. Instead we make a dynamic forwarder function
per target and call shape.
We also outline the array creation when creating `Invocation`
objects. So for simple caller shapes - such as `clone()` the
creation of an `Invocation` object is a call without arguments
instead of various array creations.
Issue https://github.com/dart-lang/sdk/issues/62640
(**) If the dynamic call could be call-via-field we still create
those arrays, as we use them for closure type checking.
Change-Id: Ia8f3f3cd95f650bd8706a15ed68a43db4de80a6e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487020
Reviewed-by: Srujan Gaddam <srujzs@google.com>
The CL changes a few things
* do not use instance field references in partitioning (they do
not have a storage location - compared to static fields
which need to be placed into a specific module)
* allow static getters and setters to possibly reside in different
modules
* separate concept of static field initializer function vs static
field storage (storage needed for getter&setter, init function
only for getter)
* fix `reference.isSetter` to also check for `isTypeCheckerReference`
* do not create class->method dependencies on abstract instance
methods
* run partitioning tests with compiler assertions
Change-Id: I61231b912c3f73f67a4d476f35113859694a80c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/485441
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Dart2wasm started using RTA to significantly speed up compilation.
However, a side effect of this is less exact type info during TFA.
In _invokeMain we conditionally use some js-interop logic when a
program's main takes arguments. On the web, most programs don't expect
any arguments so this is usually dead code. However, RTA sees
_invokeInternal is live and blindly treats all reachable classes as live
including the unused interop helper types in the other branches.
This code refactors the patching logic to make sure only the relevant
entry point is considered live.
This change uncovered a different bug that was suppressed by these
implicitly instantiated types. Dart2wasm was not marking some internally
instantiated classes as allocated in the function logic so entries in
the dispatch table were empty for those types. Instead any time we
generate the code for a constructor (which may not be reached via a
ConstructorInvocation like we were assuming before), we record the class
as allocated.
Change-Id: I556d3733c00b4a3e3455fa1ee2c90206d54c81f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/484540
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This updates JS exception catching as discussed in #55481:
- Only catch JS exceptions when the exception type is `dynamic`,
`Object`, or an extension of `JSValue`. (nullable or not)
(Previously we also caught JS exceptions when the type is `Error`.)
- When the JS value caught in Wasm is `null` or `undefined`, box it as a
non-interop class. For compatibility with dart2js, this class is
copied from dart2js and has the same `toString` as the dart2js class.
- In other cases: box the JS values as `JSValue`. This means the value
can be passed as any of the interop types, and can be passed back to
JS without manual jsification.
Fixes#55481.
Issue: https://github.com/dart-lang/sdk/issues/55481
Change-Id: I23e73074729f740b90df2ca8b3c713fb39966556
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/479640
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Today "dummy values" are generated per-module to stand in for things
like default parameter sentinels (where a given selector has multiple
default values for an optional parameter).
However, these values can end up crossing between modules. The logic is
set up to use ref_eq to check if an argument is one of these dummy
values. However, if one of these dummy values crosses between modules,
the passed value vs the ref_eq checked value will be different. Since
each module has its own canonical dummy value per type.
This new layout simplifies our handling of these dummy values by
treating them as Constants so that our normal module canonicalization
logic applies to them. We already have plenty of logic to canonicalize
constants across modules. This avoids the need for custom handling of
these dummy value globals.
Change-Id: Ia9c79923c788d7712b16705193ffbf3142141b5d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480320
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This seems to result in -0.6% compressed main module and a bit less
in uncompressed mode.
Sometimes we have many fields with lazy initializers of the same type.
That led us to emit 1 nullable wasm global for each such field. For
example all proto classes have a `static BuilderInfo i_` field. This has
led to thousands of `(mut (ref null $BuilderInfo))` wasm globals.
Now we use a wasm table for this, which is a O(1) in the binary as they
all get `null` by default, saving us all these globals. The downside is
that accesses have an extra instruction now, but overall this is a win.
The CL also cleans up `globals.dart` by separating the concept of a wasm
global and reading/writing to it from the concept of a Dart global - as
Dart globals can now be backed by wasm globals or table slots.
This CL uses the new capability made possible by the refactoring
in [0] - namely to emit element sections which initialize wasm table
slots with non-function expressions.
[0] https://dart-review.googlesource.com/c/sdk/+/459440
Change-Id: Ie57206dff8c0a57a1df5e48c0808167f822bc4a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475800
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This is another attempt at fixing #62319. See
https://dart-review.googlesource.com/c/sdk/+/470100 for the previous
attempt.
Add new member `bool get isCyclic` to `ClassInfo`: returns whether the
class has non-nullable reference cycles.
If a class has a cycle via non-nullable references, then it cannot be
dummy, because the values will need to have a cycle, but the type won't
allow late binding (as the references that form the cycle are
non-nullable).
In these cases create the locals of these types as `ref null #Top` and
use `ref.null` as the dummy values.
Generate `unreachable` in the members of un-instantiable classes, as
they can't be called.
Fixes#62319.
Issue: https://github.com/dart-lang/sdk/issues/62319
Change-Id: Iefa1949f0175eb4757fd7cfae3b25a87d17f3e8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471201
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Refactoring:
- Update the checks when updating a local for a captured variable to
check whether `local` is null, instead of whether it's not updated.
If the `local` is available then we know that it's not updated. It's
more direct to check whether we've created a local for the variable or
not.
- Add an assertion checking the the capture field and local for a
variable can only differ in nullability.
Documentation:
- Document that context field for a captured local will always be
nullable, to be able to allocate the context without dummy values.
- Document in a few places that `!capture.written` means the variable is
captured but not updated, so they can be held in a local (instead of
getting them from the context on every read).
Change-Id: I66048cb36f75e35ee3c479c41f2bbfca247a990f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/470981
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Example:
import 'dart:js_interop';
@JS()
external void eval(String code);
@JS()
external void throwFunction();
void main() {
eval('''
self.throwFunction = function() {
throw new Error('Hi from JS');
}
''');
try {
throwFunction();
} catch (e, st) {
print(e);
print(st);
}
}
Output before: ("..." parts are code locations, omitted)
JavaScriptError
at module0.main ...
at module0._invokeMain ...
at InstantiatedApp.invokeMain ...
at main ...
at async action ...
at async eventLoop ...
Output after:
Error: Hi from JS
at self.throwFunction ...
at _277 ...
at module0.main ...
at module0._invokeMain ...
at InstantiatedApp.invokeMain ...
at main ...
at async action ...
at async eventLoop ...
Fixes#62218.
Issue: https://github.com/dart-lang/sdk/issues/62218
Change-Id: Ia9347e938af209b8b87752479d35b6236f721acf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469062
Reviewed-by: Martin Kustermann <kustermann@google.com>
This reduces main wasm module a bit (<1%) as it avoids the main module
from having to define and export simple accessor (e.g. field accessor)
functions.
There's two scenarios to consider:
Deferred module calls function from main module
=> No issues
Main module calls function from deferred module
=> This call must have been preceded by a CheckLibraryIsLoaded
=> The wasm modules of the deferred import must have been loaded
=> No issues.
We also change the inlining to be a little more conservative on static
field getters, as a static field may need to do a lazy initialized check
and call initializer function.
We also give the initializer function a better name.
Change-Id: I5d9b138c9fcb41f8580602fd1ba9ec5aa4af5341
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/467820
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
There was a bug in `CallTarget.signature` which triggers if there's only
one entry that's statically dispatched against, which causes us to
inline the polymorphic dispatcher, which relies on this (previously
incorrect) signature.
The CL also changes the dispatch table building logic to not allocate
table entries for the statically dispatched regions (as they would
never be used).
Change-Id: Ic2d0c387e8863e89ef811e1892642fe81df9189a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/468000
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This shrinks essentials main module by 13% (-1.4 MB)
Before this CL the splitting of the application into wasm modules
was done on a library granularity level.
Now we split the application based on "static element" granularity -
those elements are:
* Static fields
* Static getters, setters, methods
* Constructors
* Class (all instance fields & methods)
This means that moving a class or static methods/fields from one library
to another will have no effect on the partitioning.
Differences to dart2js:
* No tracking of local functions
* No tracking of types
* No split constraint support (yet)
* (Works on Kernel AST instead of dart2js element/entity model)
The code is organized into
* `pkg/dart2wasm/lib/deferred_load/import_set.dart`
This is almost identical to the dartj2s version with minor differences:
- works on `LibraryDependency` objects
- does not assign names to parts
- no split constraint support (yet)
* `pkg/dart2wasm/lib/deferred_load/dependencies.dart`
This is a new implementation that collects dependencies of
`Reference`s/`Constant`s and in case of `Reference` whether the
dependencies are deferred or not.
* `pkg/dart2wasm/lib/deferred_load/partition.dart`
This is the main algorithm (core logic is the same as in dart2js)
`Reference`s/`Constant`s and (in case of `Reference`) whether the
dependencies are accessed under deferred loading guard or not.
Change-Id: I0fcdf86f5226060738671a1f69bb14cfffd631e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/467041
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This reduces acx main: module -0.35%, global section: -2.3%
Previous change in [0] made dynamic call sites have per-caller-shape
dynamic forwarders.
In this change we utilize this: Since we know the dynamic caller shape
we can make the dynamic forwarder call via a closure representation that
matches this caller shape (modulo the type arguments - as a dynamic call
without type arguments may get default type arguments of unknown count).
=> This means we no longer have to pass types, positionals and named
arguments as wasm arrays to dynamic call forwarder. Instead we can
pass the arguments unpacked to the vtable.
Additionally we change `Function.apply`: If we know there's no
`Function.apply` usage with named arguments, we can enumerate all
possible closure representations and call the corresponding vtable
entry. We use 2 `br_table` instructions for this:
* branch on number of type parameters
* branch on number of positional parameters
Together these two changes means that neither dynamic forwarders nor
`Function.apply` will use the dynamic call entry.
=> We remove the dynamic call entry from the vtable construction
The only exception to the above are:
* If dynamic submodules is enabled we don't compute closed world
information about closures and always call via dynamic call entry
* If `Function.apply` is used with named arguments then call via dynamic
call entry (and make vtables have it).
[0] https://dart-review.googlesource.com/c/sdk/+/461720
Issue https://github.com/dart-lang/sdk/issues/60458
Change-Id: I26e6a2a48ee37616061d701dc4d4327c2e109e82
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/462060
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This leads to:
* essentials: -160 kb module: -0.6%, code section: -1.4%
* acx gallery: -53 kb module: -2%, code section: -3.6%
This changes a few things in dynamic calls
* move null check into forwarder (instead of doing it on each call site)
* move type/pos/named array creations into forwarder
* make forwarder caller-shape specific
* exclude checking target implementations that don't match the shape
This is a precursor to removing the dynamic call vtable entrypoint in
the common case.
Issue https://github.com/dart-lang/sdk/issues/60458
Change-Id: I123423f379c8f4338130424df26fc71102777fa6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/461720
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Currently `web/wasm/flute_stress_test` is failing on
`dart2wasm-linux-optimized-jsc` mode.
That mode runs with `--use-sdk` which makes it use `dart compile wasm`.
The reason the test is failing is because it passes both
`--enable-deferred-loading` and `--multi-module-stress-test` flags to
dart2wasm. This causes us to trigger the deferred module flow instead
of the specialized multi-module flow.
Those two flags should be independent.
Change-Id: I8cac976cd91d4e585f38632ada3062e41fce401f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458800
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
This reduces ACX Gallery's main module from 8.4 MB to 2.75 MB.
This CL adds new infrastructure to our compiler: We allow codegen to
emit unresolved instructions which will get patched up later on in a
final link phase.
We use this for constants: Generating the body of a function (or
initializer of a global) may need to access a constant. Though in
deferred loading mode we may not have decided (yet) into which module
to place the constant.
* We emit an unresolved constant access (kind of dummy instructions
which still maintain stack machine) as a patchable region in the
instruction stream and remember that the constant (and any constant it
refers to, directly or indirectly) was used by the corresponding module.
* After code generation we have collected all constant uses (and the
modules they are used in) and have therefore all knowledge to decide
where to place constants. (See below on placement logic)
* In a final link phase that will walk over any instructions with
unresolved constant accesses (patchable regions) and patch them up
with the actual instructions to access the constants.
Part 1) Determination of global order
So far the creation order of globals determined the order in the global
section. But now we emit unresolved global uses and later on have to
define (or import) the globals in modules.
=> To allow this we now determine the order of globals when we build the
globals section instead.
=> This would also allow other things: Choose ordering of globals based
on usage count, etc.
Part 2) Separation of concerns in `constants.ensureConstant()`
So far the `constants.ensureConstant()` has done several things:
* performed constant lowering
* analyzing whether the constant should be lazy or eager
* determine the type of the global of the constant
* actual creation of global & initializer function (if needed)
* doing the above for all transitive constants
=> The result was the `ConstantInfo` object.
We now separate these things:
The first part will lower constants, determine lazy or not, determine
type. This will recursively walk the constant DAGs and create
`Constantinfo` as needed for all of them.
=> Each `ConstantInfo` (representing information about a `Constant`)
will now also remember the child constants (in the form of
`List<ConstantInfo> children`) it will use when defining the constant.
=> When code generation uses a constant we will remember that that
module-use of the constant and all it's child constants.
=> Representing this as `constantInfo.children` avoids recursive AST
visiting, avoids re-lowering the constants and ensures we don't have
to keep two AST visitors in sync.
Part 3) Tracking constant uses
When the code generation uses a constant, we remember it being used in
the module being currently compiled. We use this usage information in
the final stage to determine where to place constants.
Special situation: If we have constant uses across modules where
deferred loading is involved. For example here:
```
import 'foo.d.dart' deferred as foo;
main() {
...
print(foo.topLevelConstant);
}
```
which gets lowered to something like this
```
StaticInvocation(target=print, args=[
let
_ = StaticInvocation(target=checkLibraryIsLoaded, args=[StringLiteral('foo')])
in
ConstantExpression(topLevelConstant)
)
```
Even though the main module is using the `topLevelConstant` it does so
under what I call a deferred loading "load guard": The code accessing
the constant will never be executed unless the `foo` deferred library
was successfully loaded.
=> We make our usage tracking consider such uses not a usage of the main
module but rather the module containing deferred library of the
"load guard".
=> The `CodeGenerator` will track the active "load guard" when it goes
down the tree.
=> This allows pushing constants to deferred modules even if they are
used in main module code.
Part 4) Defining of constants
During code generation we (generally speaking) emit a patchable region &
record the constant use of the constant DAG (see above).
During the linking phase we then have global knowldge of constant uses
and start defining them.
Theoretically we want to define a constant in a wasm module in the
loading graph where all using modules have it as direct or indirect
dependency but the dependency being the closest one to the uses.
=> As simplification for now: If two different modules use a constant we
place it in the main module. We can later on make this more precise if
complexity is warrented.
To avoid emitting many patchable regions that we later on have to fix
up we add an optimization during code generation:
=> As soon as a use is in the main module, we define the constant DAG
in the main module.
=> As soon as there's 2 uses in different modules, we define the
constant DAG in the main module.
Misc
* We separate constant definition from importing / exporting them.
* The new architecture changes constant visiting slightly so the names
of constants in expectation files change as a side-effect of this.
Issue https://github.com/dart-lang/sdk/issues/61727
Change-Id: Ib44dee4c2514fb4af871e7078f5bfe43077922fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458240
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
TFA has handling of the `LoadLibrary` & `CheckLibraryIsLoaded` nodes. It
does logic based on them. For example it will ensure to that if one of
these nodes is retained in the AST that we also retain the deferred
library itself as well as the deferred library import.
For now dart2wasm lowered those two nodes during modular
transformations. This means TFA didn't see those nodes and weird things
can happen (e.g. a `checkLibraryLoad()` call without that library
existing anymore). Other interesting things that can happen: TFA
sees only one call to `checklibraryLoad` and constant propagates
the argument into the body & changes signature, ...
We could keep the existing behavior and make TFA aware of the dart2wasm
lowering (directly which is very hacky or via e.g. `Target`
indirection). Though this seems too complex.
So instead of making TFA aware of the lowerings, we move them to be
after TFA. Though we want precise TFA results if we don't use deferred
loading. So the approach is: If we actually need the runtime functions
we'll inject `@pragma('wasm:entry-point')` annotations before running
TFA and then lower those nodes after TFA.
This also means we avoid the ugly CFE nodes lowering followed by
lowering of the lowering (for load ids). Instead there's one place where
we lower those two nodes and we choose either to lower to "load id"
methods or the normal methods.
This is part of moving constants to deferred modules.
Issue https://github.com/dart-lang/sdk/issues/61727
Change-Id: Icebaf5a9495e00a8f85ecbf161b6ef891ea179ea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/457700
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This CL reduces the noise of future CFE experiments in the
backends. Whenever new nodes will be added as an experiment, the
related exception-throwing implementations will be added to the
mixins, eliminating the need to add the visit methods to the backend
code directly. The experimental type addresses a similar issue in
exhaustive switch statements and expressions.
TEST=existing
Change-Id: I1a021e11a159a992500c42287cbe6ef027bf54a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/455080
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Currently the IR has some serialization information attached to it (e.g.
it keeps track of the number of types that have names, etc).
This is redundant/derived information as it can be computed by looking
at the actual IR (e.g. iterate types and count the number of them that
have names).
=> We get rid of this impurity by making the serializer compute the
information it needs to serialize.
I suspect it has been done as an optimization, to avoid an extra pass
over the IR data structures. But this can be handled in another way
witout extra passes as well:
* a single pass traverses and writes the (index, name) pairs
* it keeps track of the number of (index, name) pairs written
* if the number is > 0 then we add the section, the count and the pairs
We do have to traverse all the functions, types, ..., at least once now,
but that's fine: Firstly we always assign names so in reality we cannot
skip these traversals (we instruct binaryen to emit or skip the names
when it runs). Secondly if we wanted to have core dart2wasm not emit
name section we can just skip serializing it if e.g. a
`--no-name-section` flag was passed.
We do a few more changes in this CL:
* We make `ir.DefType.name` be an optional string
=> This aligns it with `ir.Global.globalName`, `ir.BaseFunction.name`,
...
* Only assign `localNames[local] = ...` and `fieldNames[field] ` ...`
if name is not empty string
* We put the `sourceMapUrl` section behind the name section (to align
with the order used in binaryen)
* We remove redundancy in the section serializer: Instead of having
`isEmpty` that has to be kept in sync with `serializeContents` we
only have `serializeContents` and make the caller check if the bytes
are non empty.
Issue https://github.com/dart-lang/sdk/issues/60928
Change-Id: Ie9d153fefc25e8277fb30b83e45d2549731797c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452040
Reviewed-by: Ömer Ağacan <omersa@google.com>
We have a relatively clean separation between the data structures
representing a wasm module (e.g. `Module`) and the classes used to
build such a module (e.g. `ModuleBuilder`)
Though with deferred loading changes the core IR data structures started
to get a `ModuleBuilder get enclosingModule` getter.
This CL makes the IR classes self-contained again.
=> The `enclosingModule` getter will now return a `Module` instead of a
`ModuleBuilder`
=> The `*Builder` classes that need a `ModuleBuilder` can store that
themselves but only pass up `Module` to IR classes.
Since the IR data structures are cyclic (the `Module` has various parts
that refer back to it via `enclosingModule`), we allow the `Module`
object to be constructed in an uninitialized state and initialize it
after building the module is done.
Issue https://github.com/dart-lang/sdk/issues/60928
Change-Id: I5fddd6ca43b662a6e6c328230e91b1bc75332deb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451720
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This reverts commit b65435017d
This reverts commit 2254755d65
The switch-case optimizations introduced a bug that is surfaced when
running the dart2wasm self-compilation test in [0]. With that test
running
```
% python3 tools/test.py -n unittest-mac pkg/dart2wasm/test/self_compile_test
```
will fail with
```
...
Exception in StaticInvocation at file:///FakeSdkRoot/sdk/lib/_internal/wasm/lib/boxed_double.dart:346:29
Bad state: Unhandled WasmArray intrinsic: StaticIntrinsic.wasmArrayIndex
at module0.Error._throwWithCurrentStackTrace (wasm://wasm/module0-0121906a:wasm-function[160]:0xd0d76)
at module0.AstCodeGenerator.visitStaticInvocation (checked entry) (wasm://wasm/module0-0121906a:wasm-function[6304]:0x1668e5)
at module0._TreeVisitor1Default&Object&TreeVisitor1DefaultMixin&ExpressionVisitor1DefaultMixin.visitStaticInvocation (checked entry) (wasm://wasm/module0-0121906a:wasm-function[6306]:0x167b61)
at module0.StaticInvocation.accept1 (wasm://wasm/module0-0121906a:wasm-function[6297]:0x165f5f)
at module0.AstCodeGenerator.translateExpression (wasm://wasm/module0-0121906a:wasm-function[1742]:0xfcd1b)
at module0.AstCodeGenerator.visitEqualsCall (checked entry) (wasm://wasm/module0-0121906a:wasm-function[6960]:0x1777e4)
at module0.EqualsCall.accept1 (wasm://wasm/module0-0121906a:wasm-function[6944]:0x1773b4)
at module0.AstCodeGenerator.translateExpression (wasm://wasm/module0-0121906a:wasm-function[1742]:0xfcd1b)
/Users/kustermann/src/dart-sdk/sdk/pkg/dart2wasm/bin/run_wasm.js:346: [object WebAssembly.Exception]
```
The switch-case in the intrinsifier seems to be miscompiled.
Change-Id: I3bfe8887fa133573379c32d52e15769a4e6db43e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443082
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Often switches over enums are exhaustive or nearly exhaustive. The current generator uses identity for the enum values which requires iterating over each value to compare it to the test expression. So each time the switch body is entered is an O(n) operation (where is n is the # of case expressions).
This now generates a br_table using the index of the enum and jumps directly to the correct clause, effectively an O(1) operation.
A similar approach is taken for switches over an integer range. If the range of case expresison values is close in size to the # of values, it is advantageous to normalize the range around 0 and treat the values themselves as table indices.
This approach will also save code size when the index range is dense as the br_table is more compact than the identity/br_if checks. For sparse ranges this may produce a bit more code though only on the order of a few bytes per value in the range.
I've added a denseness heuristic to decide when to revert to the current strategy to avoid the code size penalty.
Golem benchmark: https://golem.corp.goog/Comparison?repository=dart#targetA%3Ddart2wasm-O2-d8%3BmachineTypeA%3Dlinux-x64%3BrevisionA%3D117402%3BpatchA%3Dnatebiggs-dart2wasm-switch-tables3%3BtargetB%3Ddart2wasm-O2-d8%3BmachineTypeB%3Dlinux-x64%3BrevisionB%3D117401%3BpatchB%3DNone
See 100% improvement in SwitchFSM.int and 90% improvement in SwitchFSM.enum.
Change-Id: Ie29e8fd59ef6235044ba5b4a4af04023d702ce57
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441760
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
Nate Biggs