Inform ASAN and MSAN about reads and writes through FFI pointers or external typed data. Previously we only informed MSAN about writes, which was enough to prevent false positives in foreign code, but gave false negatives for Dart code. This mostly uses the same machinery used for TSAN, and replaces some of the existing MSAN unpoison calls.
Also fixes some marshalling of compounds by value that generate loads extending past the end of the compound.
TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/62221
Change-Id: Ia3573edb5a0aec32b6a57035a63e8f323a655ecc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/467401
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is a preparation to detach code generation from flow graph
building and optimization passes.
Extra inlining info (inline_id_to_function, inline_id_to_token_pos and
caller_inline_id) is now added to FlowGraph and can be serialized and
deserialized.
ic_data_array is not needed for optimized compilation and is no longer
passed.
A separate CompilerPassState is created for code generation.
TEST=ci (refactoring)
Change-Id: Ib61af47c2ddde1353b9a0fe24995d29d6e85acad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399883
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Delay setting try-index for callee flow graph until the decision to inline is made.
This avoids confusing situation when running optimization passes on callee, where InsideTryBlock reports that the instruction is in try block, but there is no try block in the flow graph.
TEST=ci
Change-Id: Ia9dfa8b53dda9ca259dc73d810e7a4344971b904
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399320
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Dead Code Elimination can remove `RecordCoverageInstr`, but shouldn't.
This CL makes it stay by returning true in `HasUnknownSideEffects`
making `MayHaveVisibleEffect` return true, making Dead Code Elimination
keep it.
Note that `RecordCoverageInstr::Canonicalize` will still let it go away
if the position is already covered which is what we want.
Bug: https://github.com/dart-lang/sdk/issues/42061
TEST=pkg/vm_service/test/coverage_closure_call_after_optimization_test.dart,vm/cc/IL_RecordCoverageSurvivesOptimizations
Change-Id: Ifd72f9071a51924fd71f3dae91687acb1467047d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370220
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
When uploading CLs, the presubmit checks verify that the lines in the
diff are formatted correctly according to `git cl format runtime`.
However, when `buildtools/<os>-<arch>/clang/bin/clang-format` is
updated, it does not force reformatting of files that would be
reformatted.
This leads to two issues:
* Inconsistent style within the code base and within a single file.
* Spurious reformatting in CLs when (1) clang-format is used on the
whole file, or (2) the diff lines overlap.
`clang-format` doesn't change that frequently, so in general this is
not a large issue, but I've seen a bit too many "spurious formatting,
please revert" comments on CLs recently.
This CL formats the runtime to be in line with the current pinned
`clang-format`:
```
$ find runtime/ -iname *.h -o -iname *.cc | xargs buildtools/mac-arm64/clang/bin/clang-format -i
```
`git cl format` (which only formats changed lines, and does so with
`clang-format`) seems to not agree with itself, or clang-format, or
cpplint in a handful of places. This CL adds `// clang-format off`
for these. (See previous patchsets for the specific instances.)
TEST=A variety of bots including GCC, MacOS and Windows.
Change-Id: I470892e898971899fda14bb3b8f2c8efefd67686
Cq-Include-Trybots: luci.dart.try:vm-gcc-linux-try,vm-ffi-qemu-linux-release-riscv64-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-win-debug-x64-try,vm-win-debug-x64c-try,vm-mac-debug-x64-try,vm-mac-debug-arm64-try,vm-aot-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362780
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Specialized compiler pipeline for force-optimized functions is not
needed (force-optimized functions can be inlined into the arbitrary
functions and optimized using ordinary pipeline anyway) and error-prone
(certain optimizations could be missing by mistake).
This change unifies the pipelines by using ordinary pipeline for
force-optimized functions.
TEST=ci
Change-Id: Ia315abded61954e06acfc209d31ccce45c286437
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360960
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This is a reland of commit d24b5d1f5e
On top of the original change, the following is fixed and improved:
1) Flow graph builder (FGB) body is now also applied to
dynamic invocation forwarders (similarly to graph
intrinsics).
2) Frame can be omitted for functions which have a
call on a shared slow path. This is needed to make
FGB implementation of GetIndexed frameless, as it has
GenericCheckBound which calls on shared slow path.
(Graph intrinsics are frameless).
3) Range analysis is enabled for force-optimized functions,
so more efficient code can be generated for boxing
instructions. Range analysis is fixed to avoid crashes
and correctly intersect ranges with constant boundaries
(needed for some force-optimized FFI functions).
4) EliminateStackOverflowChecks pass is enabled for
force-optimized functions so CheckStackOverflow can be
eliminated.
Original change's description:
> [vm] Replace array GetIndexed graph intrinsics with flow graph builder implementation
>
> _Array, _GrowableList, internal and external typed data 'operator []'
> are now implemented in the flow graph builder.
>
> Unlike graph intrinsics, flow graph created in the flow graph builder
> can be used by the inliner. Corresponding graph intrinsics and native
> methods are removed.
>
> Also, this change adds missing external typed data indexing operations.
>
> TEST=ci
>
> Change-Id: Ic19784481feadf54c096a587413e67b4e18353dc
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359940
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
TEST=ci
Change-Id: I04ef008a04238d432683d7543cd047e35bad17c3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360560
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
After https://dart-review.googlesource.com/c/sdk/+/330600, there were
more chances for the optimizing compiler to introduce or move
GC-triggering instructions like allocations or boxings between the
retrieval of an untagged pointer to GC-moveable memory and its use.
To limit the chance of this happening, this CL removes the explicit
loading of the untagged payload address when building the initial
flow graph in most cases when the array is not known to be an external
array (an external string, an external typed data object, or an FFI
Pointer).
The remaining case is during view allocation, which extracts the
payload address of the base typed data object underlying the view
(which may be GC-movable) to calculate the payload address that should
be stored in the data field of the view object. See
https://github.com/dart-lang/sdk/issues/54884.
During canonicalization of LoadIndexed, StoreIndexed, and MemoryCopy
instructions, if the cid of an array input is an external array
(external string, external typed data object, or Pointer), then a
LoadField instruction that extracts the untagged payload address
is inserted before the instruction and the corresponding input is
rebound to that LoadField instruction.
Once all compiler passes that involve code motion have been performed,
a new pass looks for LoadIndexed, StoreIndexed, or MemoryCopy where
the cid stored in the instruction for the array is a typed data cid.
In these cases, if the array is not an internal typed data object,
then the payload address is extracted. Waiting until this point ensures
that no GC-triggering instructions are inserted between the extraction
of the payload address and the use. (Internal typed data objects are
left as-is because the payload address is inside the object itself
and doesn't require indirection through the data field of the object).
This CL also replaces code conditional on the array cid with code
that is instead conditional on the array element representation in
cases where it makes sense to do so, since this is a less brittle
check than checking the array cid (e.g., checking for kUnboxedInt8
to load, store, or copy an signed byte from an array instead of
listing all possible array cids that store signed bytes).
This CL also fixes an issue with the ARM64 assembler where calling
LoadFromOffset with an Address that has a non-Offset type would
silently generate bad code instead of triggering the ASSERT in
PrepareLargeOffset.
TEST=vm/dart/typed_list_index_checkbound_il_test
Issue: https://github.com/dart-lang/sdk/issues/54710
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Change-Id: I25b5f314943e9254d3d28986d720a5d47f12feeb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352363
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Codegen block order keeps loop blocks together and reflecting that in
live ranges allows to produce better register allocation decisions.
Consider for example the loop:
v = def();
while (cond) {
if (smth) { // (*)
use(v);
return true;
}
}
If block (*) is interspersed with loop blocks the register allocator
might decide to allocate it to the register in some of the loop blocks.
If the same block is "sunk" away from loop blocks - the register
allocator can clearly see that `v` does not necessarily have to live on
the register in the whole loop.
In JIT codegen block order is not topologically sorted and as such
is unsuitable for our linear scan.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-linux-release-x64-try
Change-Id: I0726815db998b559267949e157cd2158f5dd55f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358448
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Make FlowGraph constructor compute whether we are planning to reorder
blocks before code generation or not. This makes the state of the
graph at the end of the compilation more clear.
This is just a refactoring without any functional changes.
TEST=ci
Change-Id: Ieefb02237cc1ebd69d5d2b217bdc8ebbfdbf15c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358446
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This is follow up to 65f4a733bb,
which switched optimized code to use fixed frame for outgoing
arguments.
Change Kernel to IL translation to handle null-checks in
invocations differently: this code used to duplicate receiver on
the stack to accomodate for PushArgument in unoptimized code, but
PushArgument has not been inserted since f4e61eacfd,
which means duplication of the receiver is no longer necessary.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try
Change-Id: I6c1f1e8c354f9ea92424b6602b83b9e9ebce8b69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284184
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
After this change we no longer push outgoing arguments for normal
Dart calls, but instead simply move them into an area reserved for
outgoing arguments at the top of the frame. This allows us to stop
mutating SP and avoid explicit Drop(argc) after each call.
There are still other places in the generated code where we
push something onto the stack either to free a temporary
register or spill a value across a slow-path call.
This is the first step towards switching to native SP on ARM64.
# Code Size Impact
I have tested this on a large internal application and got the
following size delta
## ARM
| | Before | After | Delta | |
|--------|----------|----------|---------|-------|
| ROData | 6366632 | 6463680 | +97048 | +1.5% |
| Code | 22714192 | 22043984 | -670208 | -2.9% |
| Total | 33723701 | 33150477 | -573224 | -1.6% |
## ARM64
| | Before | After | Delta | |
|--------|----------|----------|---------|-------|
| ROData | 7122032 | 7214288 | +92256 | +1.3% |
| Code | 21607196 | 20741788 | -865408 | -4.0% |
| Total | 33375769 | 32602544 | -773225 | -2.3% |
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-product-x64-try,vm-precomp-ffi-qemu-linux-release-riscv64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm64-try
Bug: https://github.com/dart-lang/sdk/issues/39083
Change-Id: Icace0c986285b9c16e41786cded44f88277bd25f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282029
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
When building SSA form for callee graph during inlining, numbering of
SSA values starts with the number of SSA values in the caller graph.
However, parameter stubs may contain constants which are already added
to the callee graph and occupy certain range of SSA numbers. If that
number is larger than number of SSA values in the caller graph, then
numbers, already assigned to constants, can be incorrectly reused
during SSA construction.
The fix is to adjust number of SSA values in the callee graph
before parameter stubs are created, and then use current number of
SSA values as a starting point when building SSA.
TEST=runtime/tests/vm/dart/regress_flutter119220_test.dart
Fixes https://github.com/flutter/flutter/issues/119220
Change-Id: I7efd52e626b63bf64fb47861ec210b291911ef28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280041
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Unnecessary boxing/unboxing instruction can be eliminated if
their operand already has a desired representation.
This change also schedules Canonicalize pass before
TypePropagation/SelectRepresentations_Final so it has a final chance
to cleanup IL after preceeding passes but before the representations are frozen
and certain canonicalizations are disallowed.
TEST=vm/cc/IL_Canonicalize_RepresentationChange
Related issue: https://github.com/dart-lang/sdk/issues/51019
Change-Id: I187767e3efa69497cdd1f44dd916a8b9fb9f7261
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279274
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This is a reland of commit 9700458975
Original change's description:
> [vm/compiler] Initial implementation of IL binary serialization
>
> This change adds binary serialization/deserialization of flow graphs.
> It supports all IL instructions and certain objects which can be
> referenced from IL instructions. IL binary serialization is a useful
> machanism which would allow us to split compilation into multiple parts
> in order to parallelize AOT compilation.
>
> The program structure (libraries/classes/functions/fields) is not
> serialized. It is assumed that reader and writer use the same
> program structure.
>
> Caveats:
> * FFI callbacks are not supported yet.
> * Closure functions are not re-created when reading flow graph.
> * Flow graph should be in SSA form (unoptimized flow graphs are not
> supported).
> * JIT mode is not supported (serializer currently assumes lazy
> linking of native methods and empty ICData).
>
> In order to test IL serialization, --test_il_serialization VM option is
> added to serialize and deserialize flow graph before generating code.
TEST=vm/dart/splay_test now runs with --test_il_serialization.
TEST=Manual run of vm-kernel-precomp-linux-debug-x64-try with
--test_il_serialization enabled (only ffi tests failed).
TEST=gcc build on dart-sdk-linux-try bot.
Issue: https://github.com/dart-lang/sdk/issues/43299
> Change-Id: I7bbfd9e3a301e00c9cfbffa06b8f1f6c78a78470
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254941
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
Change-Id: I64ff9747f761496a096371e490ef070a14023256
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255840
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This reverts commit 9700458975.
Reason for revert: breaks Dart SDK build using gcc and dart-sdk-linux-main bot.
Original change's description:
> [vm/compiler] Initial implementation of IL binary serialization
>
> This change adds binary serialization/deserialization of flow graphs.
> It supports all IL instructions and certain objects which can be
> referenced from IL instructions. IL binary serialization is a useful
> machanism which would allow us to split compilation into multiple parts
> in order to parallelize AOT compilation.
>
> The program structure (libraries/classes/functions/fields) is not
> serialized. It is assumed that reader and writer use the same
> program structure.
>
> Caveats:
> * FFI callbacks are not supported yet.
> * Closure functions are not re-created when reading flow graph.
> * Flow graph should be in SSA form (unoptimized flow graphs are not
> supported).
> * JIT mode is not supported (serializer currently assumes lazy
> linking of native methods and empty ICData).
>
> In order to test IL serialization, --test_il_serialization VM option is
> added to serialize and deserialize flow graph before generating code.
>
> TEST=vm/dart/splay_test now runs with --test_il_serialization.
>
> TEST=Manual run of vm-kernel-precomp-linux-debug-x64-try with
> --test_il_serialization enabled (only ffi tests failed).
>
> Issue: https://github.com/dart-lang/sdk/issues/43299
> Change-Id: I7bbfd9e3a301e00c9cfbffa06b8f1f6c78a78470
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254941
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
TBR=vegorov@google.com,kustermann@google.com,rmacnak@google.com,alexmarkov@google.com
Change-Id: Iae4e4868f183815a8fc3cd79597141b3896e23d7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Issue: https://github.com/dart-lang/sdk/issues/43299
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255780
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This change adds binary serialization/deserialization of flow graphs.
It supports all IL instructions and certain objects which can be
referenced from IL instructions. IL binary serialization is a useful
machanism which would allow us to split compilation into multiple parts
in order to parallelize AOT compilation.
The program structure (libraries/classes/functions/fields) is not
serialized. It is assumed that reader and writer use the same
program structure.
Caveats:
* FFI callbacks are not supported yet.
* Closure functions are not re-created when reading flow graph.
* Flow graph should be in SSA form (unoptimized flow graphs are not
supported).
* JIT mode is not supported (serializer currently assumes lazy
linking of native methods and empty ICData).
In order to test IL serialization, --test_il_serialization VM option is
added to serialize and deserialize flow graph before generating code.
TEST=vm/dart/splay_test now runs with --test_il_serialization.
TEST=Manual run of vm-kernel-precomp-linux-debug-x64-try with
--test_il_serialization enabled (only ffi tests failed).
Issue: https://github.com/dart-lang/sdk/issues/43299
Change-Id: I7bbfd9e3a301e00c9cfbffa06b8f1f6c78a78470
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254941
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
The CCallInstr takes unboxed word-sized integers and does a call to C
without any VM transitions.
This can be used for the following.
1) Leaf runtime entries. This CL removes the EnterHandleScope,
ExitHandleScope, and AllocateHandle instructions.
2) Compilation of isolate independent code where all definitions in IL
are unboxed integers.
Currently, the CCallInstr expects all it's arguments and the return type
to be word-sized.
This CL also:
- Adds loadImmediate and CCall with a register for each assembler.
- Adds a CSE pass to force-optimized so that LoadThreadInstr gets de-
duplicated.
TEST=tests/ffi(_2)/*
Change-Id: I67c6aaa1b7d6aa7d60e274477686a54e5a331216
Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-simriscv64-try,vm-kernel-precomp-linux-debug-simriscv64-try,vm-kernel-nnbd-mac-debug-arm64-try,vm-kernel-nnbd-mac-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-win-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246240
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@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>
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>
Every AOT user out there has been using bare instructions mode
and continuing to maintaining non-bare instructions mode simply
adds costs (both in terms of time spent making changes to work
in a mode that is not used and CI resources spent on testing it).
This change removes FLAG_use_bare_instructions and changes the code
to assume that FLAG_precompiled_mode implies bare instructions.
TEST=ci
Cq-Include-Trybots: luci.dart.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-dwarf-linux-product-x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,app-kernel-linux-release-x64-try,app-kernel-linux-debug-x64-try
Change-Id: I5032b13bfcb613f79865f2cfa139cca8d1b42556
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220964
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
* Add Canonicalize pass after CSE to erase graph differences between
64-bit and 32-bit platforms. These differences originate from different
treatment of integers (we unbox more eagerly on 64-bit platform) and
consequently affect load forwarding - which generates some redundant[1]
phis on 64-bit platform, but not on 32-bit platform. Adding this pass
also addresses other cases of unpredictable performance when IL changes
can cause CSE not to trigger a Canonicalize inside itself and regress
performance, by inhibiting subsequent optimizations.
* Allow IL tests to check whether the graph was compiled in sound
null safety mode or not - because graphs can have minor differences.
* Use `"$i"` instead of `i.toString()` in the test to allow TFA to
infer non-nullability of fields in non-sound-nullsafety mode.
[1] The definition of redundant phi differs between canonicalization
pass and load forwarding pass. The former unwraps redefinitions and
the latter does not. We are going to address this difference in a
more systematic way when we split input dependencies from control
dependencies.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-simarm_x64-try
Change-Id: Ifbd68fbb7f3b374ecee8ada1b3a4a2249139a108
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220541
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
In Dart doing a type check against one value can constrain the type
of another value in an indirect fashion. This possibility needs to be
taken into account when inserting Redefinitions to inhibit the code
motion. This CL addresses two previously ignored situations:
* Given a generic container `C<T> c` which holds a value `T v` doing a
type check against `c` might narrow the type of `v`.
* Given two generic classes `C<T>` and `H<T>`, doing a type-check
against an instance of type `C<X> c` might narrow the type of an
unrelated instance of `H<X> v` within the same scope.
The second situation is currently limited to a situation when we do an
invocation on `this` and the target of the call has constrained generic
parameter (e.g. because the target of the call is in the subclass which
instantiates the base class with a more specific type
`class C_ extends C<int>`). Calls on `this` are special because
they are allowed to bypass argument type checks which are
usually performed due to covariance.
In both cases we need to ensure that all uses of `v` which are
using narrowed type are pinned to stay within the true successor
of the type-check against `c`.
Previously we would only insert redefinitions for the value that is
being type checked, but this is not enough and this CL tries to
address the newly discovered cases:
* When replacing loads in load optimizer we must ensure that
if replaced load was dependant on the redefinition (and thus
was pinned) the replacement is similarly pinned (if the type of the
load might have been narrowed). If neccessary we must insert a
redefinition for the loaded value itself.
* When performing inlining of calls on `this` we must insert
redefinitions for all parameters that might have their types
narrowed.
* When replacing phis which have redefinitions as all inputs.
Fixes https://github.com/flutter/flutter/issues/91370
Fixes https://github.com/dart-lang/sdk/issues/43652
TEST=vm/dart{,_2}/flutter_regress_91370_il_test
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I89c1f165615dd827102e9f6af90365af7d8f32b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219241
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
* Add support for dumping flow graphs in JSON (to make it machine
readable);
* Add support for controlling which functions are dumped through
a pragma annotation:
`@pragma('vm:testing:print-flow-graph'[, "passes filter"])`
* Replace simple matching DSL with programmatic matching
* Support obfuscated builds
Fixes https://github.com/dart-lang/sdk/issues/47340
TEST=vm/dart{,_2}/aot_prefer_equality_comparison_il_test
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: Ie067ba451d311e6019a8c3a88c012231b0c50eb5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219240
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This changes Profiler::DumpStackTrace to include information available
in the CompilerState attached to the thread into the crash report.
This includes the name of the function being compiled, compilation mode,
current phase, flow graph, instruction being compiled.
TEST=manually by causing compiler to crash
Change-Id: I310629f032a86781e30f0a5e218d21df7d27528e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220001
Auto-Submit: Slava Egorov <vegorov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Unfortunately, we never capitalized on having it (e.g. original plan was
to use it for writing tests, among other things) and continuing to
maintain it does not make sense.
TEST=ci
Change-Id: I02b2c35149298a295aa0de75916d48434d4a3ea9
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,pkg-linux-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190220
Auto-Submit: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
When this flag is enabled, then the validity of real token positions are
checked in three places:
* FlowGraphChecker::VisitInstruction
* CodeSourceMapBuilder::BufferChangeLocation
* DescriptorList::AddDescriptor
The validity check ensures that, if the flow graph has an inlining
ID, that the instruction has an inlining ID and that any real token
position is a valid source offset for the appropriate script (found
via the inlining ID) by attempting to look up line and column.
This flag is disabled by default until existing issues with invalid
inlining IDs and token positions are resolved.
TEST=Enabled the flag manually to check that it caught invalid token
positions.
Bug: https://github.com/dart-lang/sdk/issues/44436
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: I86439db3661d976ac4ea608d132e3277425615d5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175641
Reviewed-by: Martin Kustermann <kustermann@google.com>
The bodies provided to COMPILER_PASS macro uses are put into
a DoBody method of a generated class that has the following signature:
virtual bool DoBody(CompilerPassState* state);
CompilerState::Run uses this return value to decide whether to repeat a
given pass.
Previously, if no precompiler was provided in the compiler state, the
SerializeGraph compiler pass returned the compiler state. Since the
compiler state itself was not nullptr, it would implicitly convert to
true, meaning that CompilerState::Run would always run SerializeGraph
twice in this case. Since SerializeGraph was marked as an AOT pass,
though, this case never happened as the precompiler is always provided
for AOT passes.
In the meantime, others have used it as a basis for new compiler passes
with early returns that _could_ happen and then were confused by seeing
the pass happen twice in those cases. Fix the return value so it avoids
such confusion in the future.
Change-Id: I3a91e84abe2fbcc8437c64ce1b5b91f3d4808c65
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161784
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Generally, methods that take a *TextBuffer pointer do not care how the
internal buffer is allocated, and so they could be used for either if
both were subclasses of a base class that contained the printing
methods. This CL makes that base class, and now TextBuffer and
ZoneTextBuffer now share the exact same set of methods for printing to
the internal buffer.
Since the base class is in platform, this does mean dropping the
overload of AddString for Dart String objects that was part of
ZoneTextBuffer. Instead, this CL just adds an intermediate call to
ToCString() for the small number of callers that used the overload,
keeping the printing interface the same for both.
In addition, one use of TextBuffer that then re-allocated the buffer
contents into the zone manually has been replaced with a ZoneTextBuffer
instead.
Change-Id: I438a085e7e20d55d93987fd7f36afd636f95955f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/157741
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Moves AllocateObject and CreateArray instructions down to their
dominant use (use that dominates all other uses) when such a use exists
and the move is not hampered by environment uses (which can happen when
the allocation is inside a try block).
This improves write barrier elimination for inlined constructors, since
it moves the allocation after evaluation of the arguments. Any Dart
calls in an argument would disable elimination after it.
The optimization is particularly effective for Flutter Widget code,
since such code typically contains many nested constructor calls.
Reduces instructions size of Flutter Gallery by about 0.8%.
Change-Id: Ife30850c1a23f0986f85d42c1015f4caa7cf1fa6
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153602
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
After constant propagation is performed on the callee function body
during inlining, there are extra opportunities for specializing
calls. Unspecialized calls are not inlined, so it is important to
specialize those calls during inlining to open further opportunities
for inlining.
With this extra pass inliner is able to inline get:length after
specializing inlined List.generate constructor, and eliminate
bounds check from the initialization loop.
This change improves micro-benchmark creating 100,000 lists using
List<int>.generate(1000, (int x) => x, growable: false);
in AOT mode on x64 from 308ms to 167ms.
This makes it on par with pre-NNBD fixed-size list initialization:
var result = List<int>(1000);
for (var i = 0; i < 1000; i++) {
result[i] = i;
}
In fact, after all optimizations final flow graph (and generated
code) for List.generate and explicit loop become very similar.
This change has no effect on Flutter gallery size in release mode.
Issue: https://github.com/dart-lang/sdk/issues/42283
Change-Id: I3566386608733226e1c11cb5d6892cd869e564d7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151466
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
#if defined(DART_PRECOMPILED_RUNTIME)
#error "AOT runtime should not use compiler sources (including header files)"
#endif // defined(DART_PRECOMPILED_RUNTIME)
Remove #if defined(DART_PRECOMPILED_RUNTIME) from most compiler sources.
Change-Id: Id175c83fdbea38d9d5e1371ff433e3888f2afe8a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/143523
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Three bugs were fixed:
1. BitVector::Equals was not fully fixed by the original CL.
2. We need to add old objects to the deferred marking queue during
RememberLiveTemporaries().
3. The thread being scanned in RestoreWriteBarrierInvariant may not
be scheduled, so we cannot use its store buffer block.
In addition, this changed uncovered another bug fixed in:
https://dart-review.googlesource.com/c/sdk/+/138960.
Original CL is in patchset 3.
This reverts commit 30a12a349e.
Change-Id: I36169b09563998ed5b3c3eac70ee0ebe78853e62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138920
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Samir Jindel <sjindel@google.com>
Ryan Macnak