This reverts commit d0a1404d11.
Reason for revert: Doesn't follow declared type normalization rules in spec.
TEST=Revert
Original change's description:
> [vm] Normalize FutureOr<T?> to FutureOr<T?>?.
>
> Previously, FutureOr<T?>? was normalized to FutureOr<T?>, since the two
> types are equivalent: FutureOr<T?> is the union of T? and Future<T?>,
> and so is nullable since T? allows null. However, this means there are
> nullable AbstractTypes where nullability() returns kNonNullable.
>
> Instead, normalize FutureOr<T?> to FutureOr<T?>?, so that checking the
> nullability of the outer type is sufficient to determine nullability.
>
> TEST=vm/dart/regress_48522
>
> Fixed: https://github.com/dart-lang/sdk/issues/48522
> Change-Id: I710facc84ec79485716ee543c9d272f59f87106c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237061
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Tess Strickland <sstrickl@google.com>
TBR=kustermann@google.com,sstrickl@google.com
Change-Id: I6999963d907b9ca79d348aca85bf8949e06471b6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237081
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Previously, FutureOr<T?>? was normalized to FutureOr<T?>, since the two
types are equivalent: FutureOr<T?> is the union of T? and Future<T?>,
and so is nullable since T? allows null. However, this means there are
nullable AbstractTypes where nullability() returns kNonNullable.
Instead, normalize FutureOr<T?> to FutureOr<T?>?, so that checking the
nullability of the outer type is sufficient to determine nullability.
TEST=vm/dart/regress_48522
Fixed: https://github.com/dart-lang/sdk/issues/48522
Change-Id: I710facc84ec79485716ee543c9d272f59f87106c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237061
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This CL documents the existence of `RawStoreFieldInstr` and why a
`RawLoadFieldInstr` is not needed.
Also it adds a unit test exercising `RawStoreFieldInstr`. With this type
of unit test we can land IL instructions without using them flow graph
builder yet.
Split off https://dart-review.googlesource.com/c/sdk/+/229544.
TEST=runtime/vm/compiler/backend/il_test.cc vm/cc/IRTest_RawStoreField
Bug: https://github.com/dart-lang/sdk/issues/47777
Change-Id: I20115fd2f72abe705447aa4b16ca6ecb30ad05fa
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234980
Reviewed-by: Tess Strickland <sstrickl@google.com>
Make R(reg) use unsigned integer for constant 1 to avoid
strange effects for R(R31).
This comes into play when storing R(R31) in larger sized
integers (e.g. int64_t) and using these values as
bit-vectors - you suddenly get bits set which don't
correspond to any real registers.
TEST=ci
Change-Id: I69a09b37889bac13e9c146014ffaaefbe7b0f14f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233881
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
* Use CompilerPass::GenerateCode in AOT to ensure we print
the graph if the code generation crashes.
* Enable Dart_DumpNativeStackTrace in gen_snapshot even in
PRODUCT builds.
Additionally enable disassembler in gen_snapshot even in
PRODUCT builds.
TEST=tested manually
Change-Id: I951d62ca07c7882fa1d8e765d4b5ffd26edd9456
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233880
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
The RISC-V frame pointer convention is different from that of x86 and ARM. FP is the caller's SP, and the saved FP is at FP[-2] instead of FP[0].
Making Dart frames match the convention of C frames allows stack walkers to continue their traversals through transitions between Dart and C (e.g., for stack dumps or profiling).
TEST=ci
Change-Id: I463348beba70c1a75bfb0d902b3391be524de0fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235960
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This CL implements a slightly altered compilation process for `dart
compile` on MacOS. Instead of the traditional approach of
concatenating a dart snapshot with the dart runtime, this new
implementation uses a new MachO segment to store the snapshot. Doing
so ensure users can properly sign the resulting binaries.
The dart runtime has also been updated to look for the snapshots in
this new segment.
There are new unit tests ensuring the dart runtime can correctly
identify MachO-formatted executables and an end-to-end test ensuring
`dart compile` produces code that has the new segment and produces
code that can be signed.
TEST=pkg/dartdev/test/commands/compile_test
Change-Id: Ibdce80455e02bc2b6fd345d80be8d75f989a9c28
Cq-Include-Trybots: luci.dart.try:analyzer-mac-release-try,dart-sdk-mac-arm64-try,dart-sdk-mac-try,pkg-mac-release-arm64-try,pkg-mac-release-try,vm-kernel-mac-debug-x64-try,vm-kernel-mac-product-x64-try,vm-kernel-mac-release-arm64-try,vm-kernel-mac-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/228080
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
We're getting shard timeouts now and again involving this timeout,
so skip it entirely before we resort to adding new shards.
TEST=Status file change.
Change-Id: I8dff4db13f7f6e742a3ff54b8401b1eea9f46556
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/236081
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
These GCs usually did not free very much memory but did consume a lot of CPU. They would on low-powered, low-memory devices often take ~1s, during which time the OS might decide the OOM signal wasn't working and kill us before the compaction can complete and free pages. Instead, only release pooled memory.
Also use more appropriate GCReasons in calls of CollectMost/AllGarbage.
TEST=ci
Bug: b/216333343
Change-Id: Ia56b9ca409410f17d40508c69fb1bc9df0ce4028
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235300
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
On architectures with a link register, don't generate a profiler marker into what a foreign stack walker will interpret as the caller's frame area. If the caller is unwound based on SP instead of FP, this will cause it incorrectly identify its own caller.
TEST=ci (--profile_vm=false)
Bug: b/220804295
Change-Id: I3cc9d579f9c95476e23040c84574d2965b5eb7ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235164
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This makes it easier to identify the responsible program when a Dart process has high CPU or memory usage.
Add missing include of pthread.h, which seems to be indirectly included on proper MacOS but not iOS.
TEST=look at Activity Monitor
Change-Id: I682186cf1b16ae90ce6ee775b5c5bc72cdb2810f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235700
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
We used to emit an unwinding rule for caller's SP, however
this does not really work as we expected it to work. Instead there is
an unwritten rule that caller's SP is equal to the CFA which overwrites
our unwinding rule (at least in libunwindstack library used by
Android).
This leads to unwinding issues on the boundary between C++ and Dart:
when we unwind through the InvokeDartCode stub frame we arrive to
C++ code with SP equal to callee's CFA, which is set to FP by our
unwinding rules. In this case if C++ code does not use FP based frames
we will fail to unwind C++ frame property - because SP will be
incorrect. This manifest in simpleperf profiles as stacks which
abruptly truncate at DartEntry::InvokeCode.
This CL rewrites unwinding rules to fix this: instead of using
FP as CFA we define CFA to be caller's SP (FP+2*kWordSize). This ensures
that unwinding on the boundary between C++ and Dart works correctly:
as CFA will always be consistently set to caller's SP.
Additionally this CL fixes InvokeDartCode stub to remove a value
that this stub pushes above saved return address in PRODUCT mode.
This value acts as a profiler marker used by builtin profiler
which is only included in non-PRODUCT builds - but the presence of this
value interfers with unwinding (caller's SP will be off by 1 due the
presence of this value).
We could produce custom unwinding information for this stub instead, but
just removing this value in PRODUCT builds should solve the problem
as we usually only profile Flutter release builds with simpleperf.
Fixes b/220804295
TEST=manually profiled a Flutter app and confirmed correct flamegraph.
Change-Id: I2094afaab6e54e89625c0b5a89aebc6b9823d67c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235226
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Previously, compiler looked at the size of Dart source code when
deciding that the method is huge and needs to be compiled
with less optimizations. That size includes comments and nested
closures, so it is very inaccurate and the heuristic had
false positives seen in practice.
This change revises the heuristic to look at number of kernel
AST nodes, which are counted during building of flow graph.
This change also introduces --print_huge_methods option which
shows the warning if method is huge and compiled with less
optimizations.
TEST=manually verified that huge method from
https://github.com/flutter/flutter/issues/94461 is printed
when --print_huge_methods option is specified.
Issue: https://github.com/dart-lang/sdk/issues/48495
Issue: https://github.com/dart-lang/sdk/issues/48284
Change-Id: I81188b0cc3c147423e9cee91f2a6f544d60d319b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235173
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
When looking at a phi use of an allocation, we must consider *all*
inputs that refer to this allocation and not just the first one.
Otherwise, we might end up sinking allocation down to a use that does
not dominate all uses of the allocation.
Fixes https://github.com/flutter/flutter/issues/98466
TEST=runtime/tests/vm/dart{,_2}/flutter_regress_98466_il_test.dart
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: Iab87b4c225a29a4967eefad56a7e425629ea4b70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234361
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This makes it easier to identify the responsible program when a Dart process has high CPU or memory usage.
TEST=look at Activity Monitor
Change-Id: Ibcdcc1becf72a25dab5b010accdd8dcd199a0ba1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214137
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Before this fix, Canonicalize pass could change representation
of inputs of EqualityCompare after the last SelectRepresentations
pass. This results in unmatched representations and invalid code
generated.
The fix is to disallow canonicalization of EqualityCompare
from null-aware to non-null-aware after the last SelectRepresentations.
TEST=vm/dart/regress_flutter98967_test
Fixes https://github.com/flutter/flutter/issues/98967
Change-Id: I05359737fe322fbb2a0fe6025e3716ba5d04ebbf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234324
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
If new-space is larger the old-space, the old calculation could set the soft allocation threshold lower than size of old-space after the GC. This would cause the first old-space allocation to immediately trigger concurrent marking.
Note that since the headroom is now calculated as an addition to growth policy's output instead of as a subtraction, this effectively changes the meaning of --old_gen_growth_space_ratio from an upper bound to a target.
TEST=ci
Change-Id: Id053bfd4b0d8a8d3b43e118f265f199a45570c54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233161
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This adds a [MacroSerializer] interface to abstract how URIs are
created for precompiled macro components.
Two strategies are added. The existing solution using a temporary
directory in TempDirMacroSerializer and a new solution using the
recently added (experimental) Isolate.createUriForKernelBlob feature
in IsolateSerializer.
Change-Id: Id7117cc518f09e9b3762d6ca924c788c81fd9ac0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234282
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Ryan Macnak