This change fixes a bug in the IA32 FG Moves, where any moves
of Int8 and Uint8 from a register other than the first four general
purpose registers would be bogus.
The moves would generate to move the second byte of the first four
GP registers because the other four registers do not have byte
registers.
This change also fixes an issue where X64 byte register moves would
generate incorrect bytecode for otherwise correct instruction prints.
It would for instance print "movb [rsp],rdi", but emit "movb [rsp],bh".
This change also fixes a bug in NativeFpuRegistersLocation::Equals
that cause different FPU regs. to compare equal despite being different
kinds.
This in turn caused moves between these different regs. to be dropped
because they looked like no-ops.
See also:
https://dart-review.googlesource.com/c/sdk/+/221624
TEST=Existing.
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-ia32-try,vm-kernel-nnbd-win-release-ia32-try
Change-Id: I09e69295c8a522d74f8a8fd817188b4de656c619
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237690
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
The element data array in the VM's hash map/set implementation always
has a power-of-two length, growing exponentially as entries/elements are
added.
For constant maps/sets, this is not necessary, and the unused parts of
these data arrays is just wasted space.
This changes the data arrays for constant maps and sets to only contain
the actual entries/elements of the map/set.
Tested: ci
Change-Id: I061eb44b6c74c87157ef920143bf1885582f1b9a
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237140
Reviewed-by: Daco Harkes <dacoharkes@google.com>
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>
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>
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>
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>
Alexander Aprelev