On Android native libraries can be mapped directly from an APK
they are stored uncompressed in it. In such situations the name
of the mapping no longer provides enough information for libunwindstack
to find the original ELF file and instead it has to rely on heuristics
to locate program header table. These heuristics currently assume that
program header table will be located in the RO mapping which precedes
RX mapping. This implies the following order of segments: RO (program
header, build id section if present, .rodata), RX (.text), RW (.dynamic
and .bss).
TEST=ci, manual
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I3fa4de75c5d8841a90fd4cae1ab230d02c554f94
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206620
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
When doing optimizing compilations on the mutator thread (e.g. due to
--deterministic or --no-background-compilation) we can have situations
in which one thread is starting or in the middle of optimizing a
function while another has just deoptimized it, reaching the
deoptimization counter threshold.
We just remove this assertion which doesn't hold anymore with
--enable-isolate-groups enabled.
Fixes https://github.com/dart-lang/sdk/issues/46622
TEST=Fixes flaky assertion hit in vm/dart_2/isolates/regress_46539_test
Change-Id: I9812e3c6500113b7d948f96dc4cfcee607f56d39
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207023
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This CL fixes AOT failures that happened after [0] landed. That code
removed lookup code for the `future_class()` in the
`ObjectStore::LazyInit*()` methods and replaced it with an assert instead.
The change was correct, since the lazy members should be initialized
on-demand - after the core, non-lazy members.
=> Though this seemed to not be the case in AOT, which hit the assert.
The reason [0] only caused failures in AOT is because in JIT the VM is
bootstrapped only from the vm_platform.dill file and as a secondary
embedder call the application.dill file is loaded.
Yet in AOT we have a combined full_application.dill which will all be
loaded during bootstrapping. That has caused the application to be
loaded before `ObjectStore::InitKnownObjects()` was invoked.
The initialization of [ObjectStore] which contains common core library
classes/functions/fields should happen before we use the kernel loader
to load user classes (since general kernel loading depends on the
[ObjectStore] being populated).
This also ensures that `ObjectStore::InitKnownObjects()` will be called
before `ObjectStore::LazyInit*()` - since they depend on the former having
run (e.g. that the non-lazy `future_class()` member of `ObjectStore`
was set).
[0] https://dart-review.googlesource.com/c/sdk/+/206782
TEST=Fixes AOT builders.
Change-Id: I0861d1a2f39effbcea08d7796742845b874a0084
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207002
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
If two threads concurrently trigger initialization of the lazily
populated ObjectStore members, they have to coordinate.
We ensure mutual exclusion by holding a writer lock, to ensure only one
thread can perform the lazy initialization. Furthermore we use
store-release/load-acquire for those ObjectStore fields to ensure that
once an object is made available on ObjectStore all of it's initializing
stores are visible at that point.
We no longer make public setters for those fields, since they shouldn't
be used (only the getters + lazy initialization should be used)
Fixes https://github.com/dart-lang/sdk/issues/46611
TEST=Fixes TSAN races reported on iso-stres builder.
Change-Id: Icaf54bb224e74baf1bfbb4a9bfc386ebfbd52755
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206782
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
TTS can be generated on multiple threads at the same time and we should
therefore given them unique names by using an atomic counter.
Fixes https://github.com/dart-lang/sdk/issues/46609
TEST=Fixes one TSAN issue on iso-stress builder.
Change-Id: I4a318106a901cebe30914ac42f858cf3daced74c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206742
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
- Account for initialization-in-progress sentinel when checking static field types for reload.
- Don't read the true stack limit when setting or clearing interrupts.
TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/46596
Change-Id: I80adb4d7d69f01125b7eae8215b5da4d2e467bda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206662
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
When size get updated, class table update is not needed, causes tsan data race warnings.
Fixes https://github.com/dart-lang/sdk/issues/46538
TEST=IsolateSpawn on tsan
Change-Id: I8dac5faa413b0581e42d6b33adeef8e94e492870
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206329
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
We use message passing as comunication mechanism between isolates.
The transitive closure of an object to be sent is currently serialized
into a snapshot form and deserialized on the receiver side. Furthermore
the receiver side will re-hash any linked hashmaps in that graph.
If isolate gropus are enabled we have all isolates in a group work on
the same heap. That removes the need to use an intermediate
serialization format. It also removes the need for an O(n) step on the
receiver side.
This CL implements a fast transitive object copy implementation and
makes use of it a message that is to be passed to another isolate stays
within the same isolate group.
In the common case the object graph will fit into new space. So the
copy algorithm will try to take advantage of it by having a fast path
and a fallback path. Both of them effectively copy the graph in BFS
order.
The algorithm works effectively like a scavenge operation, but instead
of first copying the from-object to the to-space and then re-writing the
object in to-space to forward the pointers (which requires us writing to
the to-space memory twice), we only reserve space for to-objects and
then initialize the to-objects to it's final contents, including
forwarded pointers (i.e. write the to-space object only once).
Compared with a scavenge operation (which stores forwarding pointers in
the objects themselves), we use a [WeakTable] to store them. This is the
only remaining expensive part of the algorithm and could be further
optimized. To avoid relying on iterating the to-space, we'll remember
[from, to] addresses.
=> All of this works inside a [NoSafepointOperationScope] and avoids
usages of handles as well as write barriers.
While doing the transitive object copy, we'll share any object we can
safely share (canonical objects, strings, sendports, ...) instead of
copying it.
If the fast path fails (due to allocation failure or hitting) we'll
handlify any raw pointers and continue almost the same algorithm in a
safe way, where GC is possible at every object allocation site and
normal barriers are used for any stores of object pointers.
The copy algorithm uses templates to share the copy logic between the
fast and slow case (same copy routines can work on raw pointers as well
as handles).
There's a few special things to take into consideration:
* If we copy a view on external typed data we need to know the
external typed data address to compute the inner pointer of the
view, so we'll eagerly initialize external typed data.
* All external typed data needs to get a finalizer attached
(irrespective if the object copy suceeds or not) to ensure the
`malloc()`ed data is freed again.
* Transferables will only be transferred on successful transitive
copies. Also they need to attach finalizers to objects (which
requires all objects be in handles).
* We copy linked hashmaps as they are - instead of compressing the
data by removing deleted entries. We may need to re-hash those
hashmaps on the receiver side (similar to the snapshot-based copy
approach) since new object graph will have no identity hash codes
assigned to them. Though if the hashmaps only has sharable objects
as keys (very common, e.g. json) there is no need for re-hashing.
It changes the SendPort.* benchmarks as follows:
```
Benchmark | default | IG | IG + FOC
----------------------------------------------------------------------------------------------------------------------------
SendPort.Send.Nop(RunTimeRaw): | 0.25 us (1 x) | 0.26 us (0.96 x) | 0.25 us (1.00 x)
SendPort.Send.Json.400B(RunTimeRaw): | 4.15 us (1 x) | 1.45 us (2.86 x) | 1.05 us (3.95 x)
SendPort.Send.Json.5KB(RunTimeRaw): | 82.16 us (1 x) | 27.17 us (3.02 x) | 18.32 us (4.48 x)
SendPort.Send.Json.50KB(RunTimeRaw): | 784.70 us (1 x) | 242.10 us (3.24 x) | 165.50 us (4.74 x)
SendPort.Send.Json.500KB(RunTimeRaw): | 8510.4 us (1 x) | 3083.80 us (2.76 x) | 2311.29 us (3.68 x)
SendPort.Send.Json.5MB(RunTimeRaw): | 122381.33 us (1 x) | 62959.40 us (1.94 x) | 55492.10 us (2.21 x)
SendPort.Send.BinaryTree.2(RunTimeRaw): | 1.91 us (1 x) | 0.92 us (2.08 x) | 0.72 us (2.65 x)
SendPort.Send.BinaryTree.4(RunTimeRaw): | 6.32 us (1 x) | 2.70 us (2.34 x) | 2.10 us (3.01 x)
SendPort.Send.BinaryTree.6(RunTimeRaw): | 25.24 us (1 x) | 10.47 us (2.41 x) | 8.61 us (2.93 x)
SendPort.Send.BinaryTree.8(RunTimeRaw): | 104.08 us (1 x) | 41.08 us (2.53 x) | 33.51 us (3.11 x)
SendPort.Send.BinaryTree.10(RunTimeRaw): | 373.39 us (1 x) | 174.11 us (2.14 x) | 134.75 us (2.77 x)
SendPort.Send.BinaryTree.12(RunTimeRaw): | 1588.64 us (1 x) | 893.18 us (1.78 x) | 532.05 us (2.99 x)
SendPort.Send.BinaryTree.14(RunTimeRaw): | 6849.55 us (1 x) | 3705.19 us (1.85 x) | 2507.90 us (2.73 x)
SendPort.Receive.Nop(RunTimeRaw): | 0.67 us (1 x) | 0.69 us (0.97 x) | 0.68 us (0.99 x)
SendPort.Receive.Json.400B(RunTimeRaw): | 4.37 us (1 x) | 0.78 us (5.60 x) | 0.77 us (5.68 x)
SendPort.Receive.Json.5KB(RunTimeRaw): | 45.67 us (1 x) | 0.90 us (50.74 x) | 0.87 us (52.49 x)
SendPort.Receive.Json.50KB(RunTimeRaw): | 498.81 us (1 x) | 1.24 us (402.27 x) | 1.06 us (470.58 x)
SendPort.Receive.Json.500KB(RunTimeRaw): | 5366.02 us (1 x) | 4.22 us (1271.57 x) | 4.65 us (1153.98 x)
SendPort.Receive.Json.5MB(RunTimeRaw): | 101050.88 us (1 x) | 20.81 us (4855.88 x) | 21.0 us (4811.95 x)
SendPort.Receive.BinaryTree.2(RunTimeRaw): | 3.91 us (1 x) | 0.76 us (5.14 x) | 0.74 us (5.28 x)
SendPort.Receive.BinaryTree.4(RunTimeRaw): | 9.90 us (1 x) | 0.79 us (12.53 x) | 0.76 us (13.03 x)
SendPort.Receive.BinaryTree.6(RunTimeRaw): | 33.09 us (1 x) | 0.87 us (38.03 x) | 0.84 us (39.39 x)
SendPort.Receive.BinaryTree.8(RunTimeRaw): | 126.77 us (1 x) | 0.92 us (137.79 x) | 0.88 us (144.06 x)
SendPort.Receive.BinaryTree.10(RunTimeRaw): | 533.09 us (1 x) | 0.94 us (567.12 x) | 0.92 us (579.45 x)
SendPort.Receive.BinaryTree.12(RunTimeRaw): | 2223.23 us (1 x) | 3.03 us (733.74 x) | 3.04 us (731.33 x)
SendPort.Receive.BinaryTree.14(RunTimeRaw): | 8945.66 us (1 x) | 4.03 us (2219.77 x) | 4.30 us (2080.39 x)
```
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=vm/dart{,_2}/isolates/fast_object_copy{,2}_test
Change-Id: I835c59dab573d365b8a4b9d7c5359a6ea8d8b0a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203776
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Previously in Elf::Relocation, symbol names had this mapping:
* nullptr: relative to the start of the snapshot
* ".": relative to the position of the relocation
* otherwise, relative to the given static symbol value
Change this as follows:
* nullptr: relative to the position of the relocation
* "": relative to the start of the snapshot
* otherwise, relative to the given static symbol value
This uses the fact that in ELF, symbol tables always have an initial
symbol with a name of "" that has a value of 0, so now we only have to
handle the nullptr case specially there, instead of having to handle
three different cases.
TEST=Refactoring, so existing tests of snapshots.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: Icb428551091e5fcf04facd34a5bf57ea0d664fa4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206544
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
To do this, BitsContainer is changed to be a more rope-like
representation of section portions. In addition to containing
most of the same information stored per-section previously, each
portion also has a section-relative offset that is calculated when
it is added. Thus, merging two compatible BitsContainer sections
is just adding the portions from the second to the first, tweaking
the section-relative offset for each.
Other changes in this CL:
* Create PseudoSections subclasses for the elf header, program
header table, and section header table, so we can treat them
more uniformly with the other parts of the ELF snapshot.
* We now only allocate as much BSS space in the snapshot as is needed
for any text sections in the snapshot, instead of always allocating
a big enough BSS space for both VM and isolate, even for deferred
snapshots where there is no VM isolate.
* We already separated segment and section alignment in previous CLs,
so the fact that our own ELF loader needs load segments to be
page-aligned no longer means that the sections within those segments
also needs to be. Thus, we align individual instructions sections to
kMaxObjectAlignment, like readonly data sections, since both hold a
single Image object. This removes unnecessary intra-section padding.
TEST=Tests that check DWARF information and trybots that use ELF
snapshots.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: If0315c8b7b0f31481b676a8901f49cd3a44b5561
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206365
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Create all symbol table-related sections after all other sections, right
before section re-ordering. Also remove the need to keep fields in the
Elf object for most of these sections, only keeping fields for the
static symbol table and the dynamic table, which handles updates
and finalization for the dynamic symbol table.
Reworks symbols so that they are stored in a growable array in the
symbol table instead of as separately allocated objects. Since this
disallows constant fields in Symbol objects due to the need for a copy
constructor, initialize the offset of symbols with the section-relative
offset. This can be adjusted to a snapshot-relative offset after
section memory offsets are computed.
Instead of updating indices and offsets by individually looking up
symbols via their name, just create a map from section indices to new
section indices (for index updates) or to memory offsets (for offset
adjustment) and iterate over all the symbols, applying the map
appropriately.
Move non-PT_LOAD, non-PT_PHDR segment creation into
OrderSectionsAndCreateSegments.
TEST=Refactoring, so existing ELF-based tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: Ic22f1bf3ab0b00ff3e73c431f92209526e787927
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206220
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
If two mutators concurrently execute a switchable call that targets a
code object which has been disabled, they both go to runtime trying to
fix that switchable call site's object pool.
This Cl ensures we use the same logic as for other miss handlers to
ensure we properly guard against concurrent accesses.
The attached regression test will trigger a segfault - though only
with low probability.
The changes to first land is: Avoid assuming the target can be called
directly. Instead always go through IC Stub when returning from miss
handler. We do that since a call site might not provide ARGS_DESC but
the target might need it.
Fixes https://github.com/dart-lang/sdk/issues/46539
Fixes https://github.com/dart-lang/sdk/issues/46553
TEST=vm/dart{,_2}/isolates/regress_46539_test
Change-Id: I18018fa286173d905c52b6e058a6c87b544ff18e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206373
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Revert the optimization that stores function entry point addresses
directly in these caches. It doesn't work in compressed mode, and
should be an unimportant optimization these days as most cases are
handled by the dispatch table.
Change-Id: I2ebbf6549aadb49c767553574a3bafe5d867d194
TEST=CI
Bug: https://github.com/dart-lang/sdk/issues/46468
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205636
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
`_CompactLinkedHashSet` now extends `_HashVMBase`. The class hierarchy
is organized as mixins similar to LinkedHashMap to accomodate for the
other Sets not extending `_HashVMBase`.
Also, rearranges some code so that introducing ImmutableHashMap and
ImmutableHashSet is easier.
1) snapshot.h and snapshot.cc now have a MapReadFrom, MapWriteTo,
SetReadFrom, and SetWriteTo to facilitate code sharing between
mutable and immutable implementations similar to ArrayReadFrom and
ArrayWriteTo.
2) Macros for CLASS_LIST_MAPS and CLASS_LIST_SETS to facilitate
treating mutable and immutable implementations with the same handle.
Also similar to Array.
Clustered snapshots for HashMaps is currently dead code. This CL makes
it explicit by marking these as unreachable. Immutable maps and sets
will end up in the clustered snapshot in follow up CLs.
Bug: https://github.com/dart-lang/sdk/issues/36077
Bug: https://github.com/dart-lang/sdk/issues/45908
TEST=runtime/vm/object_test.cc
TEST=tests/**_test.dart on many bots
Change-Id: If3cc5ebb3138535aeb0d5e06d9da3d1c9fb2deb2
Cq-Include-Trybots: luci.dart.try:analyzer-nnbd-linux-release-try,app-kernel-linux-debug-x64-try,dart-sdk-linux-try,front-end-nnbd-linux-release-x64-try,pkg-linux-debug-try,vm-canary-linux-debug-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-checked-linux-release-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-simarm64c-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-linux-release-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206222
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
It's not used outside of elf.cc and rarely used as a type outside
of SymbolTable, so nest it to avoid defining dart::Symbol.
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: Idc8895b95b021edf831bc310aff0cf3106de8eb1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206367
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
When FDE refers to CIE it needs to use the offset to CIE start
not CIE's payload start.
TEST=manually
Change-Id: Ic0661e882ab9284d34f5768070d05f9dad440dda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206362
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Test some of the assumptions from go/dart-vm-const-maps about
`CanonicalizeHash` in the VM being equal to Dart's `hashCode`.
In this CL this is true for integers, doubles, booleans, strings, null,
and type consts.
In this CL this is not true yet for Symbols, and user-defined const
instances.
Bug: https://github.com/dart-lang/sdk/issues/45908
TEST=runtime/vm/object_test.cc
Change-Id: Ic3b4495942177ad90aa4365eb7691ca731572cb5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206240
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
datastream.h is included in places where we cannot import the compiler
namespace, so we can't make compiler::target::word the type of
WriteTargetWord. That also would possibly silently truncate a
host-word sized value that was passed in instead of alerting to a
possible issue if those bits were important.
However, just using IsInt fails if the value being passed in is
a compiler::target::uword that is larger than the max value
that fits in a compiler::target::word. Instead, check for
truncation by checking the bit length of the value, which works
for both signed and unsigned target word values.
Original description:
Also remove previous (incorrect) uses in datastream.cc and
image_snapshot.cc, now that Utils::IsInt<T>(N, value) can now
be run for values of N >= sizeof(T).
Fixes https://github.com/dart-lang/sdk/issues/46572
TEST=language/generic/super_bounded_types_test passes on NNBD simarm,
added value that triggered failure to vm/cc/MagnitudeIsUint
Cq-Include-Trybots: luci.dart.try:vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-precomp-linux-debug-simarm_x64-try
Change-Id: Idbfdda9f28243d206d482a1d9ac7ae76a5022ad2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206201
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
After https://dart-review.googlesource.com/c/sdk/+/201864 maintaining
vm/cc/PrintJSONPrimitives became manual.
This CL stops checking the actual token positions, so that we don't have
to update this test manually.
TEST=runtime/vm/object_test.cc PrintJSONPrimitives
Change-Id: I22475145f65de7d4f00770a8a6d0b711b8ff19f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206084
Auto-Submit: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Also remove previous (incorrect) uses in datastream.cc and
image_snapshot.cc, now that Utils::IsInt<T>(N, value) can now
be run for values of N >= sizeof(T).
Fixes https://github.com/dart-lang/sdk/issues/46572
TEST=language/generic/super_bounded_types_test passes on NNBD simarm,
added value that triggered failure to vm/cc/MagnitudeIsUint
Cq-Include-Trybots: luci.dart.try:vm-kernel-nnbd-linux-release-simarm-try
Change-Id: Ibc2d2bb5037a8fdee11fc26fa2a313149d3ca274
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206083
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Instead create a copy of BitmapBuilder and mutate that. This
simplifies the logic, avoids unnecessary side-effects and
allows us to delete some code which existed solely to validate
that two independent slow-path calls don't mutate the same
stack_bitmap in incompatible ways.
To make it cheap to copy BitmapBuilder we inline BitmapBuilder
backing storage into the object itself. Inlined capacity has
been chosen by looking at bitmap size distributions from large
Flutter applications.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I62ecb4ead388a367ad6904471d5923f5d1f5e3f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205783
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
This relands https://dart-review.googlesource.com/c/sdk/+/205633
but without renaming TARGET_OS_IPHONE to DART_TARGET_OS_IPHONE.
It also changes uses of TARGET_OS_IOS to
DART_TARGET_OS_MACOS_IOS to be consistent with the rest of the
VM.
TargetConditionals.h for XCode 13 defines several
TARGET_OS_* preprocessor symbols that confuse the
Dart build. There is probably a more targeted fix
for this, but renaming the symbols that Dart uses
will also prevent this problem if more symbols
are added to the platform headers in the future.
See: https://github.com/dart-lang/sdk/issues/46499
TEST=It builds.
Change-Id: Ie775c19dd23cfdf5f65e5ebc6ee4ec3a561676fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205860
Commit-Queue: Zach Anderson <zra@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Previous CLs made it so that we don't need to calculate memory offsets
until finalization. This means that we can now reorder sections as
desired and then calculate the memory offsets at the same time as the
file offsets.
Thus, we reorder them so that writable, non-executable allocated
sections come first (due to requirements by some loaders to not sandwich
writable segments between non-writable ones), followed by non-writable,
non-executable allocated sections, followed by non-writable, executable
sections, and finally unallocated sections (to avoid differences in
memory offsets between snapshots and separate debugging information).
This ordering minimizes the number of segments that need to be created
(5 or 6, depending on whether a build ID exists).
Since we can reorder, we also delay the creation of the build ID (if
generated) and BSS sections until finalization, instead of creating them
up front in the constructor.
TEST=Further refactorings, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: Id4142a021c2bb800938e8bc711b1b8a7529bff51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205780
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This CL splits the creation of ELF symbols into two parts: an
initialization that creates the symbol with all information but
the offset, and a finalization that sets the offset appropriately.
The initialization happens eagerly when adding sections, and the
finalization happens just prior to writing out the ELF contents,
after all memory and file offsets have been calculated.
To ensure safety, retrieving the offset from a symbol checks that
the symbol has been properly finalized.
This removes another obstacle to reordering sections and segments.
TEST=Further refactorings, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: I5dcb0d2c685fb341478e9a7d90c67fc4649a4e75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205481
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Now that this class is used for more than wrapping Object tags, move it
to be alongside the BitField class and change its name to something more
representative of its role.
Also remove the need to change BitField declarations if the source type
of the bitfield is decltype(field) and the declaration of field is
changed from type T to type AtomicBitFieldContainer<T>.
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-tsan-linux-release-x64-try,vm-kernel-precomp-tsan-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm_x64-try
Change-Id: If2f48ed4e209d71461c1b1ec81769e1485fbd6b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205782
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The flag isn't used anywhere in our tests or in embedder code. Turning
it on will result in a VM startup error.
We should therefore remove all uses of the flag and the flag itself.
This is a unmodified reland of
https://dart-review.googlesource.com/c/sdk/+/204500
after some remaining g3 usages have been fixed (the flutter
roll didn't port the GN changes to BUILD changes in g3)
TEST=Existing test suite.
Change-Id: Ic28c9b334a0b04524ee57e2554cc8d713a83fbfb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204785
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Previously there were several pieces of information shared between
both FunctionType and Function, mostly in the packed fields, but
named argument names were also kept in both places.
Now the FunctionType is the primary source for this information, with
the Function only keeping the names of positional arguments, which are
discarded in AOT snapshots.
This does mean extra work to access this information via the function
object, but for the most part, this information is only accessed in the
compiler or during dynamic lookups or checks in the runtime.
After adding the count of type parameters to the packed information
in FunctionType, the packed information has been split into two pieces:
one for parameter counts, another for type parameter counts. This
split does not increase the size of UntaggedFunctionType, as there
were 2 bytes available in the existing padding.
Changes on flutter gallery in release mode:
* ARM7 code size: total -0.91%, readonly -0.22%, isolate -4.32%
* ARM7 heap size: total -2.00%
* ARM8 code size: total -0.93%, readonly -0.22%, isolate -4.32%
* ARM8 heap size: total -2.12%
Changes on flutter gallery in release-sizeopt mode:
* ARM7 code size: total -0.24%, readonly -0.08%, isolate -1.49%
* ARM7 heap size: total -0.88%
* ARM8 code size: total -0.26%, readonly -0.11%, isolate -1.49%
* ARM8 heap size: total -1.01%
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try
Change-Id: Ic4d59a7b4acca039a5647f9163e716f6019163f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203241
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
This reverts commit aa9201b76b.
Reason for revert: blocks G3 roll (b/192627187)
Original change's description:
> [vm] Prefix HOST_OS_* and TARGET_OS_* with DART_
>
> TargetConditionals.h for XCode 13 defines several
> TARGET_OS_* preprocessor symbols that confuse the
> Dart build. There is probably a more targeted fix
> for this, but renaming the symbols that Dart uses
> will also prevent this problem if more symbols
> are added to the platform headers in the future.
>
> See: https://github.com/dart-lang/sdk/issues/46499
>
> TEST=It builds.
> Change-Id: I3b33a03b4a9a14b76d55fe12f8cdefec4b3c3664
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205633
> Commit-Queue: Zach Anderson <zra@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>
TBR=rmacnak@google.com,zra@google.com,asiva@google.com
Change-Id: Ib06ca418c7e9d3b4df62c72c033cd39f462f7667
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205790
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
If two mutators concurrently execute a switchable call that targets a
code object which has been disabled, they both go to runtime trying to
fix that switchable call site's object pool.
This Cl ensures we use the same logic as for other miss handlers to
ensure we properly guard against concurrent accesses.
The attached regression test will trigger a segfault - though only
with low probability.
Fixes https://github.com/dart-lang/sdk/issues/46539
TEST=vm/dart{,_2}/isolates/regress_46539_test
Change-Id: I91d84d25d74fb742ea992016a581b118345dd404
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205649
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This removes the need for the image snapshot writer to know the
starting memory offset of any ELF sections. Instead, relocation
and symbol information are recorded in the ELF writer to be
resolved during finalization.
This is a step towards being able to reorder sections in the
ELF memory space for better packing into segments. Delaying
relocation and symbol resolution also moves towards merging
multiple text or data sections into a single section if desired.
In addition, this CL removes redundant rows from the DWARF line
number program matrix to reduce the size of debugging information.
Originally, rows were written for every PC offset visited by the
CodeSourceMap. However, no row is needed if the source information
(file, line, column) is the same as the previously written row.
There are also fixes to Utils::{IsInt,IsUint,IsAbsoluteUint} to
allow N >= the bit size of the value.
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-dwarf-linux-product-x64-try
Change-Id: I09f87cea214bca06b6fca60cd66138dae6da9e83
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203766
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
TargetConditionals.h for XCode 13 defines several
TARGET_OS_* preprocessor symbols that confuse the
Dart build. There is probably a more targeted fix
for this, but renaming the symbols that Dart uses
will also prevent this problem if more symbols
are added to the platform headers in the future.
See: https://github.com/dart-lang/sdk/issues/46499
TEST=It builds.
Change-Id: I3b33a03b4a9a14b76d55fe12f8cdefec4b3c3664
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205633
Commit-Queue: Zach Anderson <zra@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Restore a check that was lost in refactorings
and made canonicalization rule useless: original rule
was ignoring use of CreateArray() by _interpolate call
itself.
TEST=darkp-*
TBR=sstrickl@google.com
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try
Change-Id: Iaa30c19b9f221248b87ef4ee26d7d3e8c8f0ea54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205646
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This instruction is essentially just a StaticCall to a fixed
target. Make this explicit by actually using StaticCall to
represent it.
TEST=existing tests
Change-Id: I50b8415ed917eb7c784170e4ee8836f1b99186b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205640
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
When intalling a new TTS stub two fields have to be updated:
- AbstractType::type_test_stub
- AbstractType::type_test_stub_entry_point_
Right now the code takes the program lock when updating those two
fields, which is very heavy weight, since this happens very often and
therefore the lock will be contended.
Instead we'll solve the write-write races by ensuring they are applied
using atomics in (any) order - though ensuring that they will be in
sync.
It is fine for them to be out-of sync for a short time, as long as we're
guaranteed that whenever we hit a gc safepoint they are in sync (because GC
might collect old TTS stubs that are not referenced anymore - i.e. we
need to avoid situations where the cached entrypoint is old but the code
pointer is new)
There can be multiple mutators computing and installing a TTS at the same
time, they will try to install in sequence.
=> This reduces the average waiting time on write access to
program lock by 3x
Fixes https://github.com/dart-lang/sdk/issues/46124
Issue https://github.com/dart-lang/sdk/issues/46252
TEST=Existing test coverage.
Change-Id: Id1ed29adeb26fa93494e8710206fd680d427bd34
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205520
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
state_bits_ field in ICData can be accessed concurrently, so this CL makes it an atomic field. Concurrent access comes for example from monomorphic miss handlers that read tracking_exactness for the field, while deopt code writes deopt_reason for the same field.
Fixes https://github.com/dart-lang/sdk/issues/46508
TEST=IsolateSpawn on tsan
Change-Id: I42375e52590f3e7a6506fcd40c0053f86e6aa6e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205100
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Use of relaxed atomic updates is fine because changes in field's type_exactness state should not update compilation result.
Fixes https://github.com/dart-lang/sdk/issues/46492.
TEST=IsolateSpawn on tsan
Change-Id: Icfda1c788d931f594b32b78d690e26b6a7df9f35
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205246
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Fixed instances where we were using 'isolate' instead of 'isolate
group'.
Fixes#46374
TEST=N/A
Change-Id: I6820954b6cbcce0c5205d46dba334691c426147c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205321
Reviewed-by: Ryan Macnak <rmacnak@google.com>
TEST=examine a heap snapshot or snapshot profile
Change-Id: I86d1981a909dd7a1b078a1445ff7c1d661422965
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205242
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Adjusts the counting phase to use the same visitor as the writing phase.
TEST=deliberately omitting checks from reader for backward compatibility
Change-Id: I8a08c1e9549fd4137714097a868541b09f160157
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205245
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
The storebuffer verification code did check that objects in the store
buffer do not have the IsCardRemembered bit set.
This CL adds some extra checks to also ensure the bit is correctly set
(or said differently, that we never add large arrays to the store
buffer)
TEST=Existing test coverage.
Change-Id: I764ac15eec30fef3769706972e1783a31f911e57
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204800
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This change concludes switching to the new invocation nodes in the VM.
Support for the old invocation nodes (MethodInvocation, PropertyGet
and PropertySet) is removed from the VM and VM-specific kernel
transformations.
TEST=ci
Closes https://github.com/dart-lang/sdk/issues/45340
Change-Id: I0717732feb1b9c6ebdf0f6079ed42a90d00970a5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204741
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
The call specializer replaces implicit setter calls with
AssertAssignable+StoreInstanceField. If the former lazy-deopts it will
deopt to after-call and therefore the StoreInstanceField will not be
performed.
This CL makes use of newly added infrastructure that can mark such
instructions as lazy-deopt to before-call, therefore causing a
re-try of the instance call.
Fixes https://github.com/dart-lang/sdk/issues/46446
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=vm/dart{,_2}/deopt/restart_call_on_deopt_regress_46446_test
Change-Id: I460e9a3c86b89a1ae127145024ace2d3d860d0af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204721
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Adding this representation allows us to create slots for storing
and retrieving values from uint16_t-typed object fields in the runtime.
Also generalize BoxUint8Instr to BoxSmallIntInstr, which holds any
unboxed integer representation where the value is guaranteed to fit
into a Smi.
To unify the implementation of BoxSmallIntInstr across architectures,
we add ExtendValue(Register dst, Register src, OperandSize sz), which
extends the sz-sized value in src to a full register size in dst. If
sz is a signed OperandSize, then the resulting value is sign
extended appropriately.
We also add MoveAndSmiTagRegister / ExtendAndSmiTagValue, which is like
the base operation but also tags the value as a Smi. On some
architectures, this can be performed using a single instruction.
Also adds sbfx/ubfx on ARM for use in ExtendValue.
TEST=Existing tests for BoxSmallIntInstr, followup CL for the new
representation.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-linux-debug-simarm64c-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-precomp-ffi-qemu-linux-release-arm-try
Change-Id: I9c0b3a23f5e28480e0bfbde55b9494f7b822dcfa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203761
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This reverts commit b1f1aee94d.
Reason for revert: Some left-over uses in g3 need to removed first
(some were removed in b/380758599 but apparently there's some
usages left).
Original change's description:
> [vm] Remove --causal-async-stacks flag
>
> The flag isn't used anywhere in our tests or in embedder code. Turning
> it on will result in a VM startup error.
>
> We should therefore remove all uses of the flag and the flag itself.
>
> TEST=Existing test suite.
>
> Change-Id: I19dfba052df7948dfdb379c0610dab67ebbcd12d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204500
> Reviewed-by: Clement Skau <cskau@google.com>
> Commit-Queue: Martin Kustermann <kustermann@google.com>
TBR=kustermann@google.com,cskau@google.com
Change-Id: I03aad46f46153d5ea4ac2fcdd5685d0ef2a0d9af
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204723
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The flag isn't used anywhere in our tests or in embedder code. Turning
it on will result in a VM startup error.
We should therefore remove all uses of the flag and the flag itself.
TEST=Existing test suite.
Change-Id: I19dfba052df7948dfdb379c0610dab67ebbcd12d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204500
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The new macros mean that the field type is only generalized to ObjectPtr
in the precompiler, and otherwise stays the same. Since the serializer
is the only part of the system that needs to know about
WeakSerializationReferences and it accesses the fields of the untagged
object directly, have the macro create a getter that automatically
unwraps if needed and casts the wrapped value to the normally expected
type so no other code in the VM needs to change.
This change also changes WeakSerializationReference::New to return an
ObjectPtr, not a WeakSerializationReferencePtr, so we can return the
target instead of wrapping it if it's an object that's guaranteed to be
serialized (e.g., null).
WeakSerializationReference::New also checks to see if the target is
already a WeakSerializationReference, and returns it if the replacement
is the same. If the replacement is different, it uses the target of the
WSR as its target. This way, WSRs are never nested, which fits the
assumptions from the original implementation.
TEST=Primarily a refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: Ia6790e4e5bd5cf302b732180dee42f796d28d59e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204141
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Right now RegExps get compiled to code like this:
v1 <- Constant(<igoto offset table (int32 array)>)
v2 <- InstanceCall([], ...)
v3 <- InstanceCall([], v1, v2)
IndirectGoto(v3)
The [InstanceCall]s can go to runtime. When returning from runtime we
can cause a lazy-deopt. The re-materialization of the unoptimized frame
works correct. Though the "v3" variable will contain a PC-relative
offset (telling the indirect goto how much to jump forward/backward in
the optimized code).
Obviously this PC-relative offset is specific to the optimized code.
When we continue in unoptimized code, the indirect goto has to jump a
different distance.
The fix this CL makes is to avoid calling the `InstanceCall([], ...)`
and instead encapsulate everything inside the IndirectGoto:
v1 <- InstanceCall([], ...)
IndirectGoto(v1 /*<- this is an index instead of an offset now*/)
This ensures in unoptimized code no deopt safepoint can happen between
looking up the offset in the constant offset array and the actual
indirect jump.
For the future: Maybe we should consider emitting a literal table at
the end of the instruction stream instead of using a
TypedDataInt32Array.
Issue https://github.com/dart-lang/sdk/issues/45213
Closes https://github.com/dart-lang/sdk/issues/46399
TEST=vm/dart{,_2}/deopt/indirect_goto_regress_46399_test
Change-Id: I81dc570ea997b3f477dd3f950a8fea03776eee0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204204
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
events on Developer stream.
Also removes intrinsic implementations for UserTag_makeCurrent to allow
for service events to be sent on UserTag change.
TEST=DartAPI_UserTags,pkg/vm_service/test/user_tag_changed_test.dart
Change-Id: I5dc9ee77c0048590d3c6e33a652eee5bc3bf522a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204440
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This reverts commit 613d8cd88e.
Reason for revert: order of initialization seems to affect core-jit
Original change's description:
> [vm] Avoid race between profiler and stub code initialization.
>
> TEST=ci
> Bug: https://github.com/flutter/flutter/issues/83939
> Change-Id: I1b1e5e759fbdb766bac5a89328d05389a712aa98
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204441
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: https://github.com/flutter/flutter/issues/83939
Change-Id: I7221e0ac1d9c9ab967c2068af8997ba8609a4784
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204623
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is a reland of 09d2025685
Fixes https://github.com/dart-lang/sdk/issues/23746
Intrinsics for sameRuntimeType wrongly assumed that types with unequal type arguments are unequal, which is not true:
1) nullability of individual type arguments may be different
2) one vector may be a prefix of the other vector
Note that the intrinsic for type equality did not make this assumption.
Case 2 above was not handled properly in the runtime.
TEST=added regression test
Original change's description:
> [VM/runtime] Handle generic types in intrinsics for type equality and runtimeType comparison.
>
> Generic types with equal class ids and equal type arguments are now considered equal by the intrinsics and a runtime call is avoided.
>
> Fixes https://github.com/dart-lang/sdk/issues/23746
>
> TEST=existing ones
>
> Change-Id: I668db119ac6d2525eac3a4f17a44f36c53b9dbf5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203143
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Régis Crelier <regis@google.com>
Change-Id: I53fc00d856ecd9d9b8d66b8da95285e6e0bd508e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204363
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
Reduces VM code size by about 33k.
TEST=ci
Change-Id: Ifc74284fa69fe209ebf12566ca81bda765f19057
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202863
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
The rehash-on-null-index code has been dead since 84dbb6e0b6.
TEST=ci
Change-Id: I4eec641ca412b6b254360a049f20e67b2c9e9718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204061
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
AllocateHandleInstr returned a pointer to uninitialised memory.
This CL initialises the handle to sentinel.
This addresses the MSAN issue in
https://github.com/dart-lang/sdk/issues/46367.
TEST=dartk-msan-linux-release-x64 vm/cc/Dart_SetFfiNativeResolver
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try
Bug: https://github.com/dart-lang/sdk/issues/46367
Change-Id: I90b731dcb25ccdca77364fb0e97d040c954942d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203768
Auto-Submit: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
* Make sure to emit .eh_frame before .dynamic section to satisfy
a single non-writable segment requirement (see Elf::WriteProgramTable)
* Use compiler::target::kWordSize instead of kWordSize when aligning
CIE/FDE and for data alignment factor.
TBR=aam@google.com
TEST=dartkp-linux-debug-simarm-crossword
Fixes https://github.com/dart-lang/sdk/issues/46403
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm_x64-try
Change-Id: Ieb73b7b90edcce91709201dc92caf0bb74ac1189
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204201
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
TEST=tested manually
This is a reland of ddfbe05eee
Original change's description:
> [vm/aot] Emit simple .eh_frame in ARM/ARM64 ELF snapshots.
>
> This allows simpleperf to unwind through AOT compiled frames and
> consequently allows Android tooling to produce useful
> timelines and flamegraphs.
>
> Note: emitted .eh_frame is rather primitive and does not cover all possible
> asynchronous unwinding cases (e.g. tick might arrive before Dart frame was
> fully setup). We will address this in a separate CL.
>
> Additionally, this CL tweaks .eh_frame emitted in assembly snapshots to
> workaround Android-specific unwinding bug in libunwindstack (b/191113792):
> current versions of libunwindstack don't push CFA value when evaluating
> DW_CFA_expression.
>
> TEST=tested manually
>
> Bug: b/184102680,b/190248517
> Change-Id: Iea209b2354cca0753385093f165b0320857104b1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203771
> Commit-Queue: Slava Egorov <vegorov@google.com>
> Reviewed-by: Tess Strickland <sstrickl@google.com>
Bug: b/184102680,b/190248517
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try
Change-Id: Id888f906307ec3b2e73deec3cf1c2ead5672a2af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204100
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
This reverts commit 09d2025685.
Reason for revert: https://github.com/flutter/flutter/issues/84813
Original change's description:
> [VM/runtime] Handle generic types in intrinsics for type equality and runtimeType comparison.
>
> Generic types with equal class ids and equal type arguments are now considered equal by the intrinsics and a runtime call is avoided.
>
> Fixes https://github.com/dart-lang/sdk/issues/23746
>
> TEST=existing ones
>
> Change-Id: I668db119ac6d2525eac3a4f17a44f36c53b9dbf5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203143
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Régis Crelier <regis@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I4d02f0550d6b4c1409fb16b19664c3d750601ddc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204140
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Storing a persistent handle in [Message] objects instead of the
[Bequest] (which is just a wrapper around a persistent handle) will
allow us to use the same mechanism for sending copies of transitive
object graphs.
Since messages can have maps inside them that need rehashing, we'll
make the persistent handle point to an array of length 2 of the format:
array = [<message>, <array-of-objects-in-message-to-rehash>]
The sendAndExit doesn't use the second part atm (since it preserves
identities and therefore avoids the need for rehashing).
Though sending transitive copies of object graphs will require this
functionality (a follow-up CL which will land with this one).
For ease of reviewing this CL was split out.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Refactoring, relying on existing test coverage.
Change-Id: I2afa78b42ef82d46477579623fd54f027136333f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203769
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This change improves Function::Hash() to take owner class into account
(in addition to function name). This makes hash collisions less likely
to happen.
This change improves time of generating AOT snapshot for a large
Flutter application in Flutter/release mode for arm64:
158s -> 115s (-27%)
b/187020358
TEST=ci
Change-Id: Ie20e2c7a2b9ee42a22c278cbfd25b2bd716c19bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204003
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reduces VM code size by about 69k.
TEST=ci
Change-Id: I44298ed2be6f3b945b19656100ce4d6555741c9d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203667
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This reverts commit ddfbe05eee.
Reason for revert: broke ARM32 builds
Original change's description:
> [vm/aot] Emit simple .eh_frame in ARM/ARM64 ELF snapshots.
>
> This allows simpleperf to unwind through AOT compiled frames and
> consequently allows Android tooling to produce useful
> timelines and flamegraphs.
>
> Note: emitted .eh_frame is rather primitive and does not cover all possible
> asynchronous unwinding cases (e.g. tick might arrive before Dart frame was
> fully setup). We will address this in a separate CL.
>
> Additionally, this CL tweaks .eh_frame emitted in assembly snapshots to
> workaround Android-specific unwinding bug in libunwindstack (b/191113792):
> current versions of libunwindstack don't push CFA value when evaluating
> DW_CFA_expression.
>
> TEST=tested manually
>
> Bug: b/184102680,b/190248517
> Change-Id: Iea209b2354cca0753385093f165b0320857104b1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203771
> Commit-Queue: Slava Egorov <vegorov@google.com>
> Reviewed-by: Tess Strickland <sstrickl@google.com>
TBR=vegorov@google.com,sstrickl@google.com
Change-Id: I57ab5df1d567bb4ddea813024245ef606712a79c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/184102680,b/190248517
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203778
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This allows simpleperf to unwind through AOT compiled frames and
consequently allows Android tooling to produce useful
timelines and flamegraphs.
Note: emitted .eh_frame is rather primitive and does not cover all possible
asynchronous unwinding cases (e.g. tick might arrive before Dart frame was
fully setup). We will address this in a separate CL.
Additionally, this CL tweaks .eh_frame emitted in assembly snapshots to
workaround Android-specific unwinding bug in libunwindstack (b/191113792):
current versions of libunwindstack don't push CFA value when evaluating
DW_CFA_expression.
TEST=tested manually
Bug: b/184102680,b/190248517
Change-Id: Iea209b2354cca0753385093f165b0320857104b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203771
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Generic types with equal class ids and equal type arguments are now considered equal by the intrinsics and a runtime call is avoided.
Fixes https://github.com/dart-lang/sdk/issues/23746
TEST=existing ones
Change-Id: I668db119ac6d2525eac3a4f17a44f36c53b9dbf5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203143
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
Call site attributes metadata is used to pass receiver static type
from kernel AST to the VM. In certain cases, call site metadata is
superseded by the new invocation nodes. This change revises how call
site attributes metadata is generated in call site annotator and
used in the VM for the new invocation nodes.
* When building flow graph for FunctionInvocation,
the decision to generate unchecked calls is now done by
looking only at the function access kind, without taking static
receiver type into account. Call site annotator now verifies that
static receiver type matches function access kind for
FunctionInvocation nodes to make sure nothing is lost during this
transition. Also, flow graph builder asserts that
function access kind is either Function or FunctionType.
* Call site metadata is no longer generated for LocalFunctionInvocation
and FunctionInvocation nodes, as it is no longer used by the VM.
* Call site annotator now verifies that InstanceInvocation nodes
cannot be used for unchecked closure calls. Flow graph builder no
longer recognizes unchecked closure calls for InstanceInvocation
and DynamicInvocation nodes.
* When generating flow graph for DynamicInvocation and DynamicSet,
call site metadata is no longer taken into account (metadata is not
generated by call site annotator for these nodes).
Also, 'this' receiver is not recognized for DynamicInvocation and
DynamicSet nodes as it may potentially violate dynamic call semantics.
TEST=ci
Issue: https://github.com/dart-lang/sdk/issues/45340
Change-Id: Ia8c5e547965ac8d7a17908d4be4bd048e5cfb23f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203668
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Adds support for marking external functions as @FfiNative's,
which will be called using fast FFI calls.
Resolution happens by calling out to a new embedder provided
Dart_FfiNativeResolver which the embedder can specify via
Dart_SetFfiNativeResolver.
TEST=vm/cc/DartAPI_FfiNativeResolver
Bug: https://github.com/dart-lang/sdk/issues/43889
Change-Id: I3cfff360b05314499a81444b90f4ea0a1b937b0b
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170092
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reduces VM code size by about 27k.
TEST=ci
Change-Id: I7bce096e7e0263a8f74b36ef7e88ecea915b3fbd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203141
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reduces VM code size by about 20k.
TEST=ci
Change-Id: I32b8f5fe8d6fe80da50da18e14630bc3b77657d7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203142
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
The Incremental Kernel Compiler (IKG) will produce kernel files without
metadata.
Right now the VM will make all closure calls dynamic calls if the
call-site attribute metadata is not present. This causes closure code
to run significantly slower due to dyn:call forwarders and the type
checks it will perform.
This change takes - in addition to the metadata - the new
FunctionInvocation.kind into account - which also tells us whether a
function invocation is based on a real function type.
Fixes https://github.com/dart-lang/sdk/issues/46320
Fixes https://github.com/dart-lang/sdk/issues/45421
TEST=vm/cc/StreamingFlowGraphBuilder_TypedClosureCall
Change-Id: Ie1136c3727fc8b5e3e3b9822a1db9479fa9da689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203302
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Makes (u)int8 and (u)int16 extended to 32 bits in registers on arm64
MacOS. This makes it consistent with iOS. (Now the calling convention
on arm64 MacOS is fully identical to arm64 iOS.)
TEST=runtime/vm/compiler/ffi/native_calling_convention_test.cc
TEST=tests/ffi_2/function_test.dart
Bug: https://github.com/dart-lang/sdk/issues/46305
This CL makes all remaining failing tests pass listed in the bug,
except for tests using `--use-slow-path`.
Change-Id: If0d93bfa774e561f39468d2b63fc9381b7646b18
Cq-Include-Trybots: luci.dart.try:vm-kernel-mac-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203305
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
This change adds a new VM-internal class for sentinel objects.
Previously sentinel objects used class Never.
TEST=existing tests
Issue: https://github.com/dart-lang/sdk/issues/46141
Change-Id: Ibb3361092967132f4f1952d64fe0168659f3075e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202870
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
On MacOS and iOS arm64, non-struct stack arguments are aligned to their
own size rather than word size. However, after a struct, the first non-
struct argument is word-aligned. This is achieved by doing alignment of
the stack also _after_ every argument passed on the stack.
Doing alignment _after_ every argument passed on the stack is a no-op
in all other calling conventions because there all stack arguments are
word-size aligned.
TEST=runtime/vm/compiler/ffi/unit_tests/structPacked/arm64_macos.expect
TEST=tests/ffi_2/function_structs_by_value_generated_test.dart
TEST=tests/ffi_2/function_callbacks_structs_by_value_generated_test.dart
Change-Id: Iffa6625cd40774fc1d2ca9180a42be7200d93a01
Cq-Include-Trybots: luci.dart.try:vm-kernel-mac-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203303
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Expands the test PassStruct9BytesPackedMixedx10DoubleInt32 to see how
the second integer is allocated.
Updates runtime/vm/compiler/ffi/unit_tests/structPacked to have the same
signature.
Fixes the structs by value test generator to output // @dart 2.9 for
legacy mode.
Deletes the copy of the test generator from tests/ffi, the copy did not
run in legacy mode.
TEST=only test changes
Change-Id: I65b7db56225a8e7963493dbccc2f296faca6fee1
Cq-Include-Trybots: luci.dart.try:vm-kernel-mac-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202968
Reviewed-by: Tess Strickland <sstrickl@google.com>
In practice, the Dart stack is usually much deeper than 8 frames, causing a typical tick to chain multiple Samples together. Increasing the number of frames covered by a Sample causes the typical tick to waste less memory on the non-frame portions of linked Samples.
For a run of dart2js compiling hello_world, increasing the frames-per-Sample from 8 to 32 decreases the memory required to represent all the ticks from 11930560 to 6907120 (-42%), with diminishing returns thereafter.
This CL does not make a compensating decrease the default number of Samples in the buffer, so it has the effect of increasing the time range covered by the buffer for deep stacks by ~4x while only increasing memory usage a bit less than 2x.
TEST=ci
Change-Id: I60b17263dcb900fff4198ca6039753828295bf38
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202308
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Trying to evaluate an expression on some internal VM stuff one should
get an error, but instead gets a FormatException because the VM sends
invalid json.
This CL fixes that by returning after the error.
A test that previously got the exception is also added.
TEST=Test added.
Change-Id: Idc9332b6ee1eb3e6fc37145b28967056e5df6882
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202766
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This change replaces generation of old invocation AST nodes
(such as PropertyGet, PropertySet and MethodInvocation) in async
transformers with new nodes (InstanceGet, InstanceSet,
InstanceInvocation, LocalFunctionInvocation).
The old nodes will be deleted eventually.
TEST=existing tests
Issue: https://github.com/dart-lang/sdk/issues/45340
Change-Id: I8c2ead9509cfd2def2f75f9d82dc13b2a9490fdf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202801
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Use separate local blocks for pushing and popping like V8, which makes the traversal order a bit more like BFS and a bit less like DFS.
TEST=ci
Change-Id: I1b965f7490a1306ff3f7fc4391ce424860bf6586
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202800
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Catapult provides more comprehensive stats. Reduces VM code size by about 16k.
TEST=ci
Change-Id: Ica146299f9e9578e06c0ec56ec36d6bef1ac9a86
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202864
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reduces VM code size by about 32k.
TEST=ci
Change-Id: Id29619379fc2fd1d0fbe85715863df0bae5bb3df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202866
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Fix the alignment of structs on MacOS arm64 so that it is the same as on
iOS arm64.
This fixes some of the ffi test failures, but not all. I'll follow up
with further CLs. (Having this landed will allow us to see further
failures on the bots.)
Bug: https://github.com/dart-lang/sdk/issues/46305
TEST=runtime/vm/compiler/ffi/unit_tests/struct12bytesFloatx6/arm64_macos.expect
TEST=tests/ffi(_2)/(.*)function(.*)_test.dart
Change-Id: I66d214ecf672734c685ff97316b1666a2859b106
Cq-Include-Trybots: luci.dart.try:vm-kernel-mac-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202940
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Split Dart_New so that most tests run correctly and only the two cases
with redirecting factories (issue 42939) are tracked as
failures.
TEST=new test added
Change-Id: Ib6db815e3c5d1c0139150c9cac546a88deab0ac4
Fixed: 46126
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201361
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reduces VM code size by about 18k.
TEST=ci
Change-Id: Ic0dbdb6a7807a0f0d37333c6f32bed5cb3e7b905
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202543
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This avoids allocating and instantiating the runtime type of closures with identical signatures and instantiators.
This also fixes runtime type equivalence for types (FunctionType and Type).
TEST=all existing ones.
Change-Id: Ic05f2abf1398546d565b98a04cd5f67416840443
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202780
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
This reverts commit 89996356b5.
Reason for revert: b/190457957 reveals that there are still duplicates in the canonical type table.
Original change's description:
> [VM/runtime] Tighten asserts and do not allow duplicate canonical recursive types.
>
> Several improvements in previous CLs should have eliminated duplicate canonical recursive types.
>
> TEST=existing ones
>
> Change-Id: Ic7ef6ea614baa678a9aa99d3f2cae4ceadc71396
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200884
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Commit-Queue: Régis Crelier <regis@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I312a6b84554931bae736584fda043693b6798966
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202767
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This change adds can_be_sentinel() flag to CompileType and
prevents unboxing of phis which can be sentinel.
This flag means that set of values can potentially contain
Object::sentinel() which is used as a marker for the uninitialized
value of late variables.
TEST=runtime/tests/vm/dart/regress_46141_test.dart
Fixes https://github.com/dart-lang/sdk/issues/46141
Change-Id: I32f19488f54c6f69932584ecec3094e3b78cc0d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201600
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
The only remaining use of the macro was to do a string concat
which might as well be done directly.
TEST=CQ
Change-Id: Idbf2eb823c74afbd249a60649f185397caf8a32e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202680
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
In 64-bit with compressed pointers enabled, we still store identity
hashes in the object header, but the Smi range is reduced.
The 0xffffffff is a Smi in 64-bit architectures, but not if
compressed-pointers are enabled.
This updates the test to use the same maximum value as the corelibraries
could use when setting the identity hash code.
TEST=Fixes vm/cc/AsmIntrinsifier_SetHashIfNotSetYet in compressed-pointers mode
Change-Id: I285a28127767a6a0b606354f3e3f994a15b372b5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202480
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This fixes idle GCs never running for the service and kernel isolates.
TEST=--verbose-gc
Change-Id: Iea0b0aa808c757a527724e3d2599c170cb383574
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202362
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Usually the identity hashcode is implemented by returning if present and
otherwise generating a random number, setting the hash code and
returning it.
This works fine in single-mutator environments, though when multiple
mutators might race to install identity hash codes we should ensure only
one of them wins and all others will obtain whatever hash code has been
set by the winner.
This will matter once we start sharing more objects across isolates.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=vm/cc/AsmIntrinsifier_SetHashIfNotSetYet
Change-Id: Ie760ca9658e6ec0640255361544d6822b07574e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201827
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
In function entry, there are redundant register assignings for constant.
This removes the register assignings.
TEST=Existing test coverage.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try
Change-Id: I08e75118a60f01b0d1a010ef4e2eaa3f5760a4ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202180
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
JSON_JSONStream_DartObject used to elide a JSON string longer
than 1024 into a buffer of 1024 bytes, which was causing memory
issues, including tripping ASAN.
This is fixed by doubling the buffer.
This CL also adds ASSERTs to ensure all buffers used with
`ElideJSONSubstring(..)` are big enough.
TEST=Existing, updated tests.
Bug: https://github.com/dart-lang/sdk/issues/46246
Change-Id: I7ce86429efecd91e768be413fd86f7d3c53c5d52
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202244
Auto-Submit: Clement Skau <cskau@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Token position and script reference information are cheap to provide and
make it possible to tie objects to scripts without requiring additional
requests for full objects.
TEST=Existing
Change-Id: I917714149a72a53081fee5626ccad858e86f5313
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201864
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Instead of explicitly listing certain subclasses in some places, instead
allow any AllocationInstr subclass and just reject certain subclasses if
necessary.
Code size different in Flutter gallery (release-sizeopt):
* ARM7: Total -0.12%, instructions -0.14%, readonly -0.11%
* ARM8: Total -0.12%, instructions -0.15%, readonly -0.09%
TEST=Current test suite.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: I4ce42d7185d4b3a83356e5131a5835fa858c4882
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201832
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
With this change, allocation instructions can now be direct inputs to
other allocation instructions, so the redundancy eliminator is extended
to handle this possibility.
Methods for working with instruction input-related slots are added to
subclasses of AllocationInstr, so that the redundancy eliminator can be
written more generically in places, instead of needing to add cases for
new allocation instructions and/or instruction inputs.
Code size different in Flutter gallery (release-sizeopt):
* ARM7: Total -0.30%, instructions -0.41%
* ARM8: Total -0.31%, instructions -0.47%
TEST=Current test suite.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: Idc1aa2a1cb8c0c62f0bcb64aee89a7525dd3d1e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198406
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
The frontend is no longer capable of handling embedded scripts.
TEST=ci
Change-Id: Iea8bb45e701f29648ec2fe15f6be68c4561a6933
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201980
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This adds the providesSymbol method to DynamicLibrary. It returns
whether the library contains a function with the given name.
As per dlsym(3), it is valid for dlsym to return nullptr in a success
case if the symbol actually has a NULL value. So I've changed the logic
to check for dlerror() after we invoke dlsym(), both in the existing
lookup and in the new method.
Closes https://github.com/dart-lang/sdk/issues/46192
TEST=tests/ffi(_2)/has_symbol_test.dart
Change-Id: Ibcb1c051cc0cdd95a104fe86ef2fc76da5bafb5d
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm64-try,vm-ffi-android-debug-arm-try,vm-kernel-linux-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-mac-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201900
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
If an IR instruction lazy-deopts it usually continues in unoptimized
code in the same IR instruction after-call.
Though in certain situations we want to continue before-call in
unoptimized code.
Two cases relevant in this CL:
* An instruction gets LICMed: If it lazy-deopts it will continue
at the Goto instruction outside the loop.
* A recognized method which got it's InstanceCall replaced by several
IR instructions. If any of them (except the last one) lazy-deopts
it should re-try the call in unoptimized code (e.g. []=)
In order to faciliate this we add a bit to the [Environment] which
encodes whether the continuation point in unoptimized code is
before-call - if so, we issue corresponding metadata.
Issue https://github.com/dart-lang/sdk/issues/45213
Issue https://github.com/dart-lang/sdk/issues/46070
TEST=runtime/tests/vm/dart{,_2}/regress_46070_test.dart
Change-Id: Ib824081768a2fd6293751a8fe09753e0d8155c87
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200644
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
These natives are implemented in kernel_to_il.cc. Now that the bytecode
interpreter has been removed, the RTEs are dead code.
TEST=test/language includes many tests with Map.
Change-Id: I42bb49434f4be0b0a881353f2f24816bfda7a3ed
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-simarm64c-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201820
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
An UnboxIntegerInstr may have a Redefinition as its input when environments are removed from instructions that !ComputeCanDeoptimize, but the Redefintions may be removed during graph finalization, changing the type attached to the unbox's input from Int to non-Int, and so changing ComputeCanDeoptimize back to true.
TEST=vm/cc/IL_UnboxIntegerCanonicalization
Bug: https://github.com/dart-lang/sdk/issues/46018
Change-Id: I9e89055f07d0f40a374b1e8fd81122763a7a6bd6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200906
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
When iterating over isolates in IsolateGroup::RegisterStaticField we need to grab read isolates_lock
to prevent data races as isolates_ list might be being updated.
TEST=generated_stress_test
Fixes https://github.com/dart-lang/sdk/issues/46071
Change-Id: I46b9826a268d38d07b1aeec9362de3e5945642fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201342
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
On IA32 arguments are passed in registers. When a struct is returned
by value, a pointer is passed in on the stack containing the address to
which the return value is written. We did not account for this pointer
in the stack-height calculation.
This problem only surfaced when there are no arguments to the function.
Because if an argument is passed it has a higher stack height than the
pointer for the result being passed in.
Fix in: runtime/vm/compiler/ffi/native_calling_convention.cc
TEST=runtime/vm/compiler/ffi/native_calling_convention_test.cc
TEST=tests/ffi/regress_46127_test.dart
Closes: https://github.com/dart-lang/sdk/issues/46127
Change-Id: Ia78fe07cc7e3a3c8625143d491935a959b4a7895
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201269
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
This CL replaces the KernelConstantsMap (backed by an Array that got
replaced on growing) by an Array that is allocated to be the required
size.
Does not cache the start of the constants table in the constructor
as proposed in [1], because the constants table is null when the
ConstantReader constructor is called in the KernelLoader constructor.
[1] https://dart-review.googlesource.com/c/sdk/+/196925/6..10/runtime/vm/compiler/frontend/constant_reader.cc#b116
Closes: https://github.com/dart-lang/sdk/issues/45903
TEST=vm test suite, including GC tests.
Change-Id: Ie111bedc7eb7fcd627ab700a9906245bc7dd14ce
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try
Fixed: 45903
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200871
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Also make all other Allocate*ABI structs explicitly use the same result
register as AllocateObjectABI for consistency.
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-linux-debug-simarm64c-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try
Change-Id: Iede8ff499ae3e7741e57090c36bc6b5dcc9217b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201184
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
* Rename values and methods like kElementTypePos or element_type() to
corresponding names like kTypeArgumentsPos or type_arguments() (since
the input to CreateArrayInstr is the type arguments vector for the
array, not just the type of the element.)
* Create a AllocateArrayABI struct for the input and output registers of
the AllocateArray stub and use those where applicable.
* Explicitly list what registers are clobbered in the AllocateArray
stubs in their documentation comment.
* Avoid clobbering the type arguments input register in the arm64
version of AllocateArrayInstr, so all input registers are preserved
across all architectures.
TEST=Refactoring, so existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-linux-debug-simarm64c-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try
Change-Id: I3a7c2b6afdd307c26f8d4f97a4c8bd7684e7b242
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201183
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Avoid calling SetTypeTestingStub from PostLoad because it
uses write lock.
Saves ~10-15% on ReadProgramSnapshot on a large Flutter app.
Issue https://github.com/dart-lang/sdk/issues/46116
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try
Change-Id: If843828661e68f18df19824af204df326bf016a0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201180
Auto-Submit: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Currently external data is counted twice on the receiver size: once for
received transferable, second for newly created external typed data.
Ensure new space is collected when allocating external at the limit.
TEST=SendReceiveBytesTransferable with verbose-gc
Change-Id: I129d913f89e098b5d3066ce249ac0c4702e1394d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199842
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This CL adds FFI leaf calls by adding `lookupFunction(.., isLeaf)`
and `_asFunctionInternal(.., isLeaf)`, which generate FFI leaf calls.
These calls skip a lot of the usual frame building and generated <->
native transition overhead.
`benchmark/FfiCall/` shows a 1.1x - 4.3x speed-up between the regular
FFI calls and their leaf call counterparts (JIT, x64, release).
TEST=Adds `tests/ffi{,_2}/vmspecific_leaf_call_test.dart`. Tested FFI tests.
Closes: https://github.com/dart-lang/sdk/issues/36707
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-release-arm64-try,vm-ffi-android-release-arm-try,vm-ffi-android-product-arm64-try,vm-ffi-android-product-arm-try,vm-ffi-android-debug-arm64-try,vm-ffi-android-debug-arm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-precomp-nnbd-mac-release-simarm64-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-precomp-ubsan-linux-release-x64-try,vm-kernel-precomp-tsan-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try
Bug: https://github.com/dart-lang/sdk/issues/36707
Change-Id: Id8824f36b0006bf09951207bd004356fe6e9f46e
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179768
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This updates RelationalOp and EqualityCompare to use Mint as immediate on ARM.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm-try
Change-Id: I2ea68ed4d54ba1dc04953a629ad57e0bc439cec5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200960
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
When building method extractors in a given isolate group, the copy of
the AllocateContext stub in the isolate group should be used. Otherwise,
calls to that stub cannot be turned into PC relative calls when
appropriate.
Also retrieve the closure allocation stub from the object store directly
instead of going through GetAllocationStubForClass, now that closure
allocation is handled via a distinct stub.
Be more specific about the types of the stub objects passed to
GenerateBuildMethodExtractorStub, as they're guaranteed to be Code
objects.
TEST=Existing test cases.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: I05bc5e8a133834e66af3e1003d8a6597ee11c312
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200868
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This removes the need for passing the flag to use
Dart_ExecuteInternalCommand, which is done in several tests
that otherwise have nothing to do with testing pragmas.
Also adds status file skips for precomp-win targets that currently
crash due to https://github.com/dart-lang/sdk/issues/40579.
TEST=CQ
Bug: https://github.com/dart-lang/sdk/issues/46059, https://github.com/dart-lang/sdk/issues/46061
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-win-release-x64-try,vm-kernel-precomp-nnbd-win-release-x64-try,vm-kernel-win-debug-x64-try
Change-Id: I3024ad9bedb7a74abaaaa1020b7525e5d8b1bd47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200461
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The same signature (canonical function type) may be shared between different closure functions.
Clean up asserts verifying that members are canonical when updating type test cache.
TEST=existing ones
Change-Id: Ic206409e56d34dfd02d6705af93065fd3b83b8fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199561
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Canonical tables are now rebuilt during serialization.
TEST=ci
Change-Id: I4666ca1ed73a6f84b3546d35b394395623bf5854
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200923
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
This avoids the extra redirection through the closure function, which
does not need to be loaded otherwise during closure calls in this mode,
and thus removes another runtime dependency on the closure function in
bare instructions mode.
In non-bare mode, CODE_REG is populated with the code object for the
function, so caching wouldn't change the number of loads there.
This does not increase the size of closure objects, as there was
already a free word in them due to object alignment.
TEST=Existing tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: Ida6e0d277919259a8c0e8dcbfaa101379fd22ff1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195920
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This was disabled previously due to inconsistent cpu time reading reported by the ios.
It seems to be working correctly now.
TEST=verified flutter application cpu timeline numbers in observatory on physical ios device
Change-Id: Id97a550bbbc96a32b5c2ccf59ca9805ea8cb9ec4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200703
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
CL was made by removing all symbols that don't have Symbols::* usage
and then adding back ones which are constructed via compile-time
concatentation in macros.
TEST=Existing test suite.
Change-Id: I3651808739d90a8d36b162f300b09f02fe2916aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200420
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
When TTSs access type argument vectors we should avoid using
Address/FieldAddress - which only work if offsets are guaranteed to be
within certain instruction encoding limits.
It also fixes Assembler::LoadCompressedFieldFromOffset on ARM64
to not assume (base, offset) is instruction encode-able.
TEST=vm/dart{,_2}/flutter_regress_82278_2_test
Change-Id: I9be316a068f222a6c1897c323111c4b18adb1e7a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200222
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Ran into this error when trying to set up a finalizer with an Object
owner. The super class is null in this case.
Change-Id: I27f67a738c72c0b433e367257c638a6d3d2e495f
TEST=ci and manual testing
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199602
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
This should improve performance of lightweight isolates configuration
TEST=ci, perf benchmarks
Change-Id: I98ac9a7e02318d58db3431a6d33f08ab95e607fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199700
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Annotations are evaluated in the CFE and the VM does therefore
not encounter any AST nodes anymore.
TEST=Existing test suite.
Change-Id: Id0ac60cf0d1a8d1667c79541c1de66765778ce90
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200183
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
If the type arguments vector gets introduced in subclasses where the
base classes have already many fields we may not be able to load the TAV
in one instruction on ARM64.
Issue https://github.com/flutter/flutter/issues/82278
TEST=vm/dart{,_2}/flutter_regress_82278_test
Change-Id: I164ef42af3afe8267fe23a8a11af9401776eccdb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199481
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
New FunctionInvocation nodes have explicit static type information such
as function access kind and function type. However, an attempt to use
this information revealed that it is less accurate compared to the old
call site attributes metadata
(see https://github.com/dart-lang/sdk/issues/46003 for details).
This change reverts back to using call site metadata when building
flow graph for FunctionInvocation nodes. It fixes regression of
ListCopy benchmark with new invocation nodes.
TEST=benchmarks/ListCopy/dart/ListCopy.dart with new invocation nodes
Issue: https://github.com/dart-lang/sdk/issues/45340
Issue: https://github.com/dart-lang/sdk/issues/46003
Change-Id: I73e5fae49b8056365211989e6e656544c79bcc50
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199563
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
It needs to check whether the class the type arguments field is in
has compressed pointers, not whether TypeArguments has compressed
pointers.
Since we need the class to know whether compressed pointers are in
play, remove the version that just takes the offset.
TEST=Existing tests.
Change-Id: Ie6de8e0b2366b86f99a7fda362809dd1e10f99b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199484
Auto-Submit: Tess Strickland <sstrickl@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Instead of specifying FINAL_COMPRESSED and VAR_COMPRESSED, the
macros for defining boxed native slots use the class's
ContainsCompressedPointers() predicate to determine whether a Slot
points to a compressed field.
Also use owner.HasCompressedPointers() when getting a Slot for a
particular Field.
TEST=Refactoring, so just running against the current test suite.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-linux-debug-x64-try
Change-Id: I4bf25ffa2dbe89dc9a922305820dc4b752f47d13
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198044
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
When we switched from allocating closures with AllocateObject to
using AllocateClosure, a couple of places that needed to check for
IsAllocateClosure() were missed. This caused AllocateClosureInstrs to
never have a NotAliased Identity(), which then forced some
AllocateUninitializedContextInstrs to be kept instead of eliminated.
TEST=Checked using benchmarks that saw regressions due to this change.
Change-Id: If63d4cae190453233429b5657cbe177cac265074
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199430
Commit-Queue: Tess Strickland <sstrickl@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Auto-Submit: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
BARRIER_MASK requires 8 low bits and HEAP_BASE requires 32 high bits. Combine them into a single register HEAP_BITS, and use ARM's shifted operands to continue accessing the relevant bits in a single instruction.
TEST=ci
Change-Id: I5a4cd43fec2d19615239ec5cec2ac282d17c461c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198100
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
TEST= removed flags from test. No behavior should change.
Change-Id: I401bfb68c082d1bd405a118d5eca6a47a807945f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199241
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Doing any runtime call inside an `<XXX>Instr::EmitNativeCode()` is dangerous
because doing so almost always would require adjusting the lazy-deopt
environment to ensure the continuation in unoptimized code would have
the right stack state when returning from a runtime call.
=> Going to runtime should almost always happen via a stub call.
To discourage using runtime calls inside `EmitNativeCode()` functions we
remove the `FlowGraphCompiler::GenerateRuntimeCall()` helper method.
The remaining runtime calls used in `EmitNativeCode()`s fall into three
categories:
* StackOverFlowInstr: In this particular case the deopt environment
wouldn't need to be modified.
* leaf runtime calls: Those do not need any metadata (neither deopt
nor stackmaps) - since leaf runtime entries cannot safepoint
* unopt-only runtime calls: Those do not need any metadata (neither
deopt nor stackmaps) - since all slots in unoptimized frames are
GC-able and unoptimized frames cannot be lazy-deoptimized.
We add assertions for the latter to cases with a comment to make it
clear.
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=Only adds assertions.
Change-Id: Idb8badfbe65fff55585959338129405c71908d25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199042
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Running isolates tests in debug mode is currently very slow due to heap
verification and related code on isolate startup & shutdown.
This CL limits those verifications to only run on the first isolate of
an isolate group.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Existing test suite.
Change-Id: I1f329bca9e4c1d56ab60f36ffa8b9cc037b818f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199249
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
-----
Adds a few type traits for operating on (Compressed)ObjectPtr types:
* is_compressed_ptr<T>::value is true for compressed pointer types and
false for uncompressed pointer types.
* is_uncompressed_ptr<T>::value is false for compressed pointer types
and true for uncompressed pointer types.
* base_ptr_type<T>::type is ObjectPtr for uncompressed pointer types and
CompressedObjectPtr for compressed pointer types.
Note: If DART_COMPRESSED_POINTERS is not enabled, all pointers are
uncompressed and the above traits function accordingly. That means
that is_compressed_ptr<T>::value is always false,
is_uncompressed_ptr<T>::value is true for all object pointers, and
base_ptr_type<T>::type is ObjectPtr for all object pointers, even if
they contain Compressed in the name.
-----
The following changes have been made to the VISIT_* macros:
* VISIT_NOTHING: no change.
* VISIT_FROM: takes only a field name now, and retrieves the type of
the field to determine which base pointer type to use for from().
Note that VISIT_FROM must now come _after_ the declaration of the
field, since it retrieves the type from the declaration.
* VISIT_TO: takes only a field name now, and retrieves the type of the
field to determine which base pointer type to use for to().
* (removed) VISIT_TO_LENGTH: Instead, VISIT_TO creates a to() method
that takes an optional length argument (defaults to 0).
* (new) VISIT_FROM_PAYLOAD_START: takes the object pointer type of the
payload elements and creates a from() that returns the beginning
element of the payload. Note that a payload element must be a single
object ptr to use this macro.
* (new) VISIT_TO_PAYLOAD_END: takes the object pointer type of the
payload elements and creates a to() which takes a non-optional length
argument and returns the last element of the payload. Note that a
payload element must be a single object ptr to use this macro.
If one of the {COMPRESSED_,}VARIABLE_POINTER_FIELDS macros are used,
then VISIT_TO_PAYLOAD_END should not be, as those macros already
include a use of it.
-----
In addition, each Untagged<Class> class now has a static constexpr bool
field kContainsCompressedPointers. This field is false for
UntaggedObject, and for other Untagged<Class> classes, it is either
inherited from the parent class or set during VISIT_FROM and the new
VISIT_FROM_PAYLOAD_START macro.
VISIT_TO and the new VISIT_TO_PAYLOAD_END macro double-check at compile
time that the type retrieved or provided is a compressed type if
kContainsCompressedPointers is true, and uncompressed if false.
If the elements of a payload are not object pointers, but rather
a composite object that contains object pointers, then
VISIT_FROM_PAYLOAD_START/VISIT_TO_PAYLOAD_END cannot be used. Instead,
use DEFINE_COMPRESSED_POINTERS(type), where type is the type of one of
the object pointers in the composite object, to appropriately define
kContainsCompressedPointers.
Note that this field will always be false when DART_COMPRESSED_POINTERS
is not defined.
-----
For classes, a static constexpr ContainsCompressedPointers() predicate
is created, based on the associated untagged class's field.
For instances of Class, there is an instance predicate
Class::HasCompressedPointers() that returns whether instances of a given
class object contain compressed pointers.
Change all calls to InitializeObject and Object::Allocate to pass in
the result of the appropriate predicate.
TEST=Refactoring, so current tests on CI, especially on x64c trybots.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-linux-debug-x64-try
Change-Id: Ifb61f72885bd8b951167becbccf8ec3337a922b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196931
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
When generating snapshot with loading units trace
deferred pools for code objects as well - otherwise
we might miss code references from one deferred
unit to another unit (e.g. if the only remaining
reference to some code object in the root unit is
actually from a deferred unit).
Fixes a bunch of issues with deferred libraries (Crash -> Pass):
- language_2/deferred/split_constants_canonicalization_test/1
- vm/dart_2/deferred_isolate_test
- vm/dart_2/deferred_loading_and_weak_serialization_references_test/0
- vm/dart_2/deferred_loading_and_weak_serialization_references_test/1
- vm/dart_2/deferred_loading_call_modes_test/2
Fixes issue https://github.com/dart-lang/sdk/issues/45917
TEST=ci
Fixed: 45917
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm-try
Change-Id: Iccd3efcab6a5396d4b6f70968d9176ff18d7147c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198405
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Now AllocateClosureInstr takes the closure function as a Value* instead
of as a const Function&, a new kFunctionReg is added to
AllocateClosureABI, and the AllocateClosure stub sets the closure
function internally instead of initializing it to null and requiring it
to be set post-allocation.
A convenience method known_function() is added to AllocateClosureInstr
which returns either the closure function, if known at compile time,
or null otherwise.
TEST=More refactorings, so just passing current tests.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: If843b401e0f282b191dd60b1c6c73c01f1d5bc70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198285
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Also create a subclass of AllocationInstr, AllocateClosureInstr,
which is used when allocating closures.
Followup CLs add inputs to this instruction and to the AllocateClosure
stub, starting with the closure function. Adding these inputs allows
the related field to be set within the stub, instead of using
StoreInstanceField manually at each closure allocation point.
Since this CL only adds the initial stub/instruction implementation,
the overhead on snapshots is minimal: just the addition of the new
stub to each isolate.
-----
This CL also adds virtual and non-virtual methods to assembler_base.h
revolving around field loads and stores and attempted inline object
allocation, to ensure all architectures have these methods.
It also adds LoadFromSlot/StoreToSlot/StoreToSlotNoBarrier, which
appropriately calls one of the other methods based on whether the slot
is compressed or not and whether it is a boxed or unboxed field.
With these additions, the AllocateClosure stub generator can be defined
in an architecture-independent way.
TEST=Basically a refactoring, so check using current test suites.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: I71f5691307679f8d5e3604007699de4706f86eb8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198284
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Instead of trying to lazily create artificial nodes when needed for
WeakSerializationReference (WSR) targets in all cases, create them
eagerly for targets in reachable WSRs, since those are guaranteed
to be needed.
Those for unreachable WSRs are still lazily created as needed, since the
WSR won't even be accessed by the clustered snapshot writer unless the
unreachable WSR is part of another unreachable object that has an
artificial node created.
This rework avoids some issues seen on upcoming CLs where the artificial
nodes for WSR targets weren't getting correctly generated.
-----
Also extend the v8 snapshot profile writer tests to check the sizes of
the text and data sections in ELF snapshots. That means the v8 snapshot
profile writer tests check up to three different measures, from most
precise to least precise, depending on the output mode:
* If writing an ELF snapshot directly: the sum of the sizes attributed
to the text and data section symbols are checked to be exactly the
same as the sum of the sizes of the objects in the profile.
* If writing an ELF snapshot either directly or via assembly: the sum
of the sizes of the text and data sections are checked to be the same
as the sum of the sizes of the objects in the profile. If using an
assembler that merges text and data sections, then account for the
padding between the sections using an approximate check.
* For all: Check that the size of the snapshot as a whole is
approximately equal to the sum of the sizes of the objects in the
profile, accounting both for possible padding and for the global
header information and non-data/non-text sections like the dynamic
symbol section.
TEST=vm/data{,_2}/v8_snapshot_profile_writer_test
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-mac-release-simarm64-try
Change-Id: I66e0e7fdb5bb98045621bf516f150a4723e08147
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198942
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
This flag allows dumping timings of various precompiler passes.
TEST=precompiled dart2js with --print-precompiler-timings
Change-Id: I54d6fdf26c25a0e43ae4f2717e833e09e9321d81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198407
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
To avoid accidental code that calls stubs without emitting proper
(deopt-id, deopt-env) metadata, we enforce that it's always present for
stub calls and add another GenerateNonLazyDeoptableStubCall for cases
where it's intentionally omitted.
The environment has in many cases been still emitted before, due to the
usage of `pending_deoptimization_env` though we make code pass it
explicitly (just as the deopt-id). We may want to consider deprecating
this `pending_deoptimization_env`.
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=Existing test suite.
Change-Id: I93f1d5ba4d74da5f9afa4b526ad57b9d032ca99e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197164
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
The "RunWithMutatorsStopped" wrapper is used in places where the invoked
callback can cause deoptimization of code. It should therefore ensure
it's running with a "deopt safpoint operation scope" to ensure mutators
are stopped at well-defined places that allow lazy-deopt.
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=Existing code base, will add "fuzzer"-like test to nightly/weekend builders.
Change-Id: Icb9a4183c13fab0f084e481c10dfc56a0308126a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197162
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
The optimizing compiler can insert box instructions at various places
during the optimization pipeline. There is no guarantee that a box
instruction will get a proper (deopt-id, deopt-env) assigned to it - in
many places it will not.
Furthermore StoreInstanceField's into "unboxed fields" might cause
box allocations to happen. Those stores might also not have deopt
information associated with them.
In general we require stackmaps when allocation slow paths go to runtime
and cause GC. If GC throws OOM we theoretically would also need deopt
information in order to populate correct catch-entry state. Though OOM
is uncommon and the VM could decide to ignore the top-frame - if it's
missing deopt information - since box allocations aren't something
users are in control of (as opposed to user-allocated objects inside
try/catch).
=> This CL makes box allocations go to a runtime entry that doesn't
support lazy-deopt. While being in those runtime entries the mutators
will also not participate in "deopt safepoint operation" requests.
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=Existing test suite.
Change-Id: I1b61f77e3166da82efad08bb49bc1756576d220c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196928
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This extends the existing safepoint operation mechanism by allowing to
perform two different operations:
* "gc safepoint operations": All mutators are stopped at places where
it's safe to GC. It therefore requires stackmaps to be available for
all optimized mutator frames.
* "deopt safepoint operations": All mutators are stopped at places
where it's safe to GC, but also safe to lazy-deopt mutator frames.
It therefore requires deopt-id/deopt-info to be available for all
optimized mutator frames.
Mutators can be asked to block for any of those two safepoint operations.
If a mutator is at a place where its safe to GC it will respond to "gc
safepoint operations" requests, if a mutator is additionally at a place
where it's also safe to lazy-deopt it will respond to "deopt safepoint
operation" requests.
Depending on how the runtime was entered (which is tracked via the
[Thread::runtime_call_deopt_ability_] value) - the mutator might
participate in both or only in gc safepoint operations.
During the start of a "deopt safepoint operation", the safepoint handler
will request all threads to stop at a "deopt safepoint". Some threads
might first want to initiate their own "gc safepoint operation"
(e.g. due to allocation failure) before they reach a "deopt safepoint".
We do allow this by letting the safepoint handler own a "deopt safepoint
operation" but still participate in other thread's "gc safepoint
operation" requests until all mutators are checked into places where
it's safe to lazy-deopt at which point the "deopt safepoint operation"
also owns a "gc safepoint operation".
In order to facilitate this, the Thread's safepoint_state will be
extended to consist of the following bits:
* AtSafepoint
* SafepointRequested
* AtDeoptSafepoint
* DeoptSafepointRequested
* BlockedForSafepoint
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=vm/cc/SafepointOperation_*
Change-Id: Icdc2827718f6780818f99b829a5e806d6bb5b130
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196927
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
As a pre-requisite of partitioning our safepoints into "gc only
safepoints" and "lazy-deopt'able safepoints" we'll let mutators keep
track of whether a runtime call into the VM is deoptable.
A mutator is lazy-deopt'able except for one particular case: When the
mutator calls into the VM from a place where no deopt-id/deopt-env
gets recorded (e.g. StoreInstanceField's box allocation goes to
runtime).
If a mutator is in the VM in a non-lazy-deopt'able state, it cannot call
out to the embedder, transition from "vm" to "native" state, ...
A future change will make new non-lazy-deopt'able runtime entries taking
advantage of this CL.
Issue https://github.com/dart-lang/sdk/issues/45213
TEST=Existing test suite & tests in future CLs.
Change-Id: I30dbee02dc839b2ba9632b20ffa233aff4d58f64
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196922
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
For yielding functions (async, sync*, async*) we need to look for
annotations on the outer function instead of the synthetic inner
function associated with the handler frame.
Fixes https://github.com/dart-lang/sdk/issues/45673
TEST=runtime/observatory{,_2}/tests/service{,_2}/notify_debugger_on_exception_yielding_test.dart
Bug: https://github.com/dart-lang/sdk/issues/45673
Change-Id: I8b1718b3614852f6f8db98811177b21fe587fea1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198408
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
'''
> git cl lint
runtime/vm/compiler/frontend/kernel_to_il.cc:2614: \
Declaration has space between type name and * in \
TargetEntryInstr *null_ftav [whitespace/declaration]
'''
TEST=Built and ran some existing tests.
Change-Id: I8095a0ea93cce9b57ce4e4965476b1e0a66e7ff0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198402
Auto-Submit: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
This is a reland of 8a21ab195a
Original change's description:
> [VM/runtime] Refactor the representation of type parameters in the VM.
>
> This introduces a new VM internal class 'TypeParameters' representing the declaration of a list of type parameters, either in a class or function.
> The reference to (or use of) a type parameter is still represented by the existing 'TypeParameter' class.
>
> Fixes https://github.com/dart-lang/sdk/issues/43901
> Fixes https://github.com/dart-lang/sdk/issues/45763
>
> TEST=existing ones and a regression test
>
> Change-Id: I1fde808bf753cc1cb829f2c4383c1836651cee80
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/189942
> Commit-Queue: Régis Crelier <regis@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
This fixes https://github.com/dart-lang/sdk/issues/45911
TEST=existing ones and a regression test
Change-Id: I709d38b1df3d73fe3c9796d5aca3cbbdcf77fd38
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198380
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This reverts commit 8a21ab195a.
Reason for revert: Test failure: http://b/187227619
Original change's description:
> [VM/runtime] Refactor the representation of type parameters in the VM.
>
> This introduces a new VM internal class 'TypeParameters' representing the declaration of a list of type parameters, either in a class or function.
> The reference to (or use of) a type parameter is still represented by the existing 'TypeParameter' class.
>
> Fixes https://github.com/dart-lang/sdk/issues/43901
> Fixes https://github.com/dart-lang/sdk/issues/45763
>
> TEST=existing ones and a regression test
>
> Change-Id: I1fde808bf753cc1cb829f2c4383c1836651cee80
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/189942
> Commit-Queue: Régis Crelier <regis@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
TBR=rmacnak@google.com,alexmarkov@google.com,regis@google.com,sstrickl@google.com
Change-Id: If12caa1a84cb6d1c1b8225589f3c994d25abb120
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198282
Reviewed-by: Michal Terepeta <michalt@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Michal Terepeta <michalt@google.com>
This introduces a new VM internal class 'TypeParameters' representing the declaration of a list of type parameters, either in a class or function.
The reference to (or use of) a type parameter is still represented by the existing 'TypeParameter' class.
Fixes https://github.com/dart-lang/sdk/issues/43901
Fixes https://github.com/dart-lang/sdk/issues/45763
TEST=existing ones and a regression test
Change-Id: I1fde808bf753cc1cb829f2c4383c1836651cee80
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/189942
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Previously constants were referenced via there relative binary offset,
i.e. a constant reference saying 42 meant at byte offset 42 relative to
the start of the constant table in the binary. This was done to be able
to (on the VM side) read the needed constants lazily. It meant, though,
that constants had to be stored in a map, mapping from the byte position
to the constant.
This change adds a level of indirection when needing the lazy reading,
but lets the constant references reference the constant number instead
so that a constant reference saying 42 means constant number 42,
i.e. constants can be stored in a list instead of in a map.
This is done on the dart side, but the VM still stores it in a map.
The level of indirection is a tabel next to the constant table where
each entry has constant size (4 bytes) from which one can read the
relative byte offset into the constant table from the constant number,
thus still being able to read needed constants lazily.
This CL also cleans up a leftover where for instance double constants
had their textual representation saved as a string in the string indexer
(and thus the output dill) even though they were never referenced.
File size changes:
* Platform: increses 7,816 bytes.
* Compile of dart2js (including platform): decreases 71,424 bytes.
Speed changes:
* Adding `UserTag`s to the code and looking at observatories cpu profile
on a `pkg/kernel/test/binary_bench.dart --golem AstFromBinaryLazy`
run of a compile of dart2js reading the constant table has gone
from ~10% to ~5%.
* Doing statistics on
`pkg/kernel/test/binary_bench.dart --raw AstFromBinaryLazy` run of a
compile of dart2js says -6.28169% +/- 4.97269%.
* Golem runs has nothing above the noise level.
It does say "AstFromBinaryLazy (Intel Xeon) -4.006% (0.4 noise)" and
"AstFromBinaryEagerP50 (Intel Core i5) -8.929% (0.6 noise)" though.
TEST=All tests running the VM tests this.
Change-Id: I07ead457527a4477de803ee55ba742f5557413d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196925
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
We once relied on making Symbols etc unreachable in the compiler's heap to remove unused ones from the snapshot, but we have since moved to using reachablity during snapshotting.
TEST=ci
Change-Id: I6a95bd71582deedc271441e4b6732201b2293c5b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197442
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
BreakpointLocation token_pos and end_token_pos are populated when
breakpoint is resolved and are read when vm needs to see if a
function has breakpoints.
Since the breakpoint is resolved once and token positions are
populated with real values only once using simple atomics is sufficient
to resolve data race.
Fixes https://github.com/dart-lang/sdk/issues/45877
TEST=service[_2]/break_on_function_many_child_isolates_test.dart on tsan
Change-Id: I92066d094ca3a86c80f0de648debea05ccde5e49
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197561
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Retrieve CodeBreakpoint::ToCString() while we hold on to the CodeBreakpoint, before BreakpointLocation potentially deleted with terminated isolate.
GroupDebugger::NotifyCompilation(func) might be called several times for a given function when multiple isolates in a group race to compile it. Debugger needs to handle this scenario gracefully, ensure that already resolved breakpoints won't get resolved again.
TEST=observatory_2/tests/service_2/break_on_function_many_child_isolates_test
Fixes https://github.com/dart-lang/sdk/issues/45516.
Change-Id: Ieebab86103b1999a13daff7b7bbc87ffa778ec82
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197520
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Also creates LoadFromOffset/StoreToOffset variants across all
architectures (was missing in some) that take the register to
load to/store from, the memory address, and the size to load/store.
Fixes LoadFromOffset on X64 to use MOVSXD when the size is kFourBytes.
Creates slots for the data field of TypedDataBase/Pointer objects
and replaces the StoreUntagged instruction with uses of
StoreInstanceField instead to create uses of the new version of
the instruction.
BUG=https://github.com/dart-lang/sdk/issues/42793
TEST=Tests that involve FFI pointers or TypedData constructors.
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-simarm64c-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-x64-try,vm-kernel-linux-debug-x64c-try
Change-Id: I2c96e83bb086aa93c56b834e809e7141a32cfc35
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196924
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
We might interrupt Object._objectHashCode(obj) helper to serve a
vm-service request between checking for unset hash code and setting
a new hash code. While serving this request we might set the hash
code of [obj] (e.g. when generating heap snapshot or in
Instance::PrintJSONImpl). Later Object._objectHashCode will call
Object_setHash, which (on 64-bit platforms) assumes that
hash field in the object header is set to 0 and uses bitwise-or
to initialize it. This leads to a mismatch between hash code that
the first invocation of _objectHashCode will return and the
value stored in the header (because we OR non-zero value set
by vm-service with a value which _objectHashCode intended to
use as a hash).
This CL changes Object_setHash to avoid overwriting or mangling
the hash value if it was already set.
We also fix hash code generation in vm-service to ensure that we
only generate values which are valid Smis because the rest of the
code expects that.
TEST=pkg/front_end/test/incremental_compiler_leak_test.dart
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm64-try
Change-Id: Ica913af8bc1cfef0ad60a9e7504531ee4de53015
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197400
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
current_context_var is special cased in the liveness analysis and never
pruned even though it does not have uses by instructions. The lack of
pruning caused the assert to fire. This CL adds the special casing to
this assert as well.
Closes: https://github.com/dart-lang/sdk/issues/45855
TEST=tests/language/vm/regress_45855_test.dart
Change-Id: Ica74a43bd2449dd994639c686253449146216458
Fixed: 45855
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197541
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Returning 'false' from a RPC handler in service.cc results in the VM
shutting down due to an UNIMPLEMENTED error. Since we always return
'true', even in the case of RPC errors, and the TODO to handle 'false'
returns is over 6 years old, RPC handlers should just have a void
return.
Example: invoking getSourceReport on an AOT VM should just return a RPC
error, but 'false' was being returned from GetSourceReport causing the
VM to shutdown.
Fixes https://github.com/flutter/devtools/issues/2955.
TEST=Existing service tests
Change-Id: I344c2f4cf849d27f08f59e8c61fb4c17c32719d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197241
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This makes the AOT CodeRelocator more precise by removing some
heuristics which allows writing more precise tests as well.
Follow-up to:
https://dart-review.googlesource.com/c/sdk/+/195682
TEST=vm/cc/CodeRelocator_*
Change-Id: I36e55f4c8db0bd88dc5e3a58fc2f12d889dae2a0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197383
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Some 64-bit microarchitectures store only the low 32-bits of targets as part of indirect branch prediction, predicting that the target's upper bits will be the same as the call instruction's address. This leads to misprediction for indirect calls crossing a 4GB boundary. Ask mmap to place our generated code near the VM binary to avoid this.
Cf. https://chromium-review.googlesource.com/c/v8/v8/+/2726500
TEST=ci
Change-Id: If99850c50383751fcde1b71e38019c78ff97a787
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197104
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Previously, Code objects of invisible functions were retained
in order to omit frames corresponding to such functions from stack
traces when stack trace is printed.
This change drops Code objects of invisible functions. That also
means that frames corresponding to such functions are no longer
skipped in binary DWARF stack traces.
In order to account for that, DW_AT_artificial attribute is added to
generated DWARF debug information to mark invisible functions.
Stack trace decoding now looks at this attribute and skips those
frames when symbolizing stack trace.
Flutter gallery in release-sizeopt mode:
Heap size of snapshot objects -4.2% (arm), -4.4% (arm64).
A large application in --dwarf_stack_traces mode:
Number of discarded Code objects increased from 72.4% to 83.7%
(out of all Code objects).
Heap size of Code objects -37.4%.
Heap size of all snapshot objects -5%.
TEST=tests/standalone/dwarf_stack_trace_invisible_functions_test.dart
Issue: https://github.com/dart-lang/sdk/issues/44852
Change-Id: Ib804852aba1e083670f1d9b9d66cbaab7dcdcff9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196583
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Calls from C++ to Dart entry points are currently performed by
calling via Function::entry_point() in AOT mode
(with --use_bare_instructions). Such calls no longer use
Code objects, so Code objects can be discarded even if they
belong to a Function which is used as an entry point.
TEST=ci
Issue: https://github.com/dart-lang/sdk/issues/44852
Change-Id: Iaf9dd67392780ef4344fc518865ffbe30648762e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196720
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This is important for keeping shifts compiling properly on some
platforms: because some shift variants only support constant shift
distances, so an interfering IntConverter inserted by the
representation selection pass causes compilation to fail.
This fixes Meteor benchmark, which is currently crashing the compiler.
TEST=runtime/tests/vm/dart{,_2}/regress_lsl_with_constant.dart
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try
Change-Id: Ie2f316cd917a8ebd8d3d96f394b39eac4d135d95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196662
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
When --resolve-dwarf-paths is enabled, then paths output to DWARF
information will be resolved to either an absolute or relative path.
If this cannot be done, snapshot creation fails.
File URIs are output as absolute paths.
SDK URIs are output as paths relative to the SDK root.
TEST=vm/dart{,_2}/use_resolve_dwarf_paths_flag_test
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: I63c694f0f707ef6a3d3faa690e001fefe2b26094
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196491
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
