Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement VM IL serialization and deserialization #36882

Closed
mraleph opened this issue May 8, 2019 · 12 comments
Closed

implement VM IL serialization and deserialization #36882

mraleph opened this issue May 8, 2019 · 12 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@mraleph
Copy link
Member

mraleph commented May 8, 2019

Currently VM IL can be printed in human readable form - but this form is not machine readable and omits some information.

There are use cases where we would benefit from having a format that is both human and machine readable and writable:

  • Writing tests.
  • Writing alternative backends for Dart VM.

This bug is to track the design and development of such a form.

@mraleph mraleph added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label May 8, 2019
@mraleph
Copy link
Member Author

mraleph commented May 8, 2019

/cc @mkustermann @aartbik @alexmarkov

dart-bot pushed a commit that referenced this issue Jul 26, 2019
Instead of hand-writing S-expression output in the methods used by the
serializer, just create an appropriate S-expression data structure (with
handling of extra info maps) that handles serialization.

Bug: #36882
Change-Id: I9b56827d7452271de4b32c0c01cc2e4c4bcc0367
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109706
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Jul 30, 2019
Bug: #36882
Change-Id: I991d160ecbde45a09e7533867f51f1ab72c3b215
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110220
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Jul 31, 2019
* Add a new `SExpDouble` atom and change `SExpNumber` to `SExpInteger`.

* Allow for negative integers in deserialization.

* Add support for `LocalVariable`s and related instructions.

* Function objects are now represented by actual S-expressions generated
  with the new `FunctionToSExp` method. Previously, they were only represented
  by a symbol containing their canonical name.

* The top-level tag for a serialized flow graph is now `FlowGraph`, not
  `function`. This avoids confusion between serialized flow graphs and
  serialized function references. Similarly, the old `FunctionToSExp`
  method is now called `FlowGraphToSExp`.

* Made all SExpression* returning functions that take Object (or subclass)
  instances return nullptr if the passed in instance is the null object,
  except for ObjectToSExp, which returns the symbol `null`.

* Factored out creating tags for the different kind of block/function
  entry and also created an `Entries` section to the top-level `FlowGraph`
  form that contains function entry points similar to the `Constants` one
  instead of inlining entries as separate elements in the `FlowGraph` form.

* Additional extra information in verbose mode for some elements.

Bug: #36882
Change-Id: Iede3865ec64f81955a87fd57b10e74d49ee8414c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110917
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Aug 7, 2019
* Adds non-static final field printing for `Instance`s
* Adds specialized printing for `Closure`s (tearoffs).
* Instances of `Mint` and `Type` can also appear in constant
  pools, so handle both in ObjectToSExp.
* DartValueToSExp is a wrapper method for ObjectToSExp that
  also checks to see if the value is defined in the constant
  pool. If it is, the object is serialized as a reference to
  that definition.
* Reworks the `tmp_*_` fields used for non-reentrant cases
  to be per function to make it easier to understand how they
  interact. (Only cross-function tmp field that remains is
  `tmp_string_`, since its contents are always immediately
  converted to a C string and then no longer needed.)

Bug: #36882
Change-Id: Ibad4bc2497dd1f580d1daa536c4a267818981c14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/111425
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Aug 26, 2019
This adds in a version of the IL deserializer that doesn't actually do
any deserialization yet.

However, it adds in a couple of compile passes that passes the FlowGraph
for compiled functions through the serializer and deserializer.  If the
deserialization step succeeds, then the FlowGraph stored in the
CompilerPassState is replaced with the new FlowGraph for the rest of the
compiler.

Also change the default zone handling in the FlowGraphSerializer to use
the zone for the current thread, instead of the zone stored in the
FlowGraph, so that it is appropriately affected by StackZone uses.

Bug: #36882
Change-Id: I8fa1c7af434f724ccec45ddb73263f84730af5b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113184
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
dart-bot pushed a commit that referenced this issue Aug 29, 2019
At this point, we do get some successful deserializations (of
methods that are optimized into constant value returns).

Adds support for:
  GraphEntry (including constant pool)
  FunctionEntry
  TargetEntry
  Return

Results from compiling hello world program:

* Early round trip
    * Contains unhandled instructions: 4180
    * Failed during deserialization: 0
    * Successful round trip: 0
* Late round trip
    * Contains unhandled instructions: 4163
    * Failed during deserialization : 0
    * Successful round trip: 15

Bug: #36882
Change-Id: I9e0919f6ffbfe059ce0efd55f9a767c1ba805795
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113325
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
dart-bot pushed a commit that referenced this issue Aug 29, 2019
Adds support for:
  CheckStackOverflow
  Parameter
  SpecialParameter

Adds handling of Integer, Double, Class, Type, and TypeArguments
in ParseDartValue. Adds handling of values visited in the graph prior
to the corresponding definition to keep traversal of block contents as
one pass.

Results from compiling hello world program:

* Early round trip:
    * Contains unhandled instructions: 4157
    * Failed during deserialization: 0
    * Successful round trips: 21
* Late round trip:
    * Contains unhandled instructions: 4023
    * Failed during deserialization: 29
    * Successful round trips: 126

Bug: #36882
Change-Id: Icb2972749ee86891d5847519a3b5097ad5c63c54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113326
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Sep 3, 2019
Adds support for:
  Constant
  Goto
  JoinEntry
  PushArgument
  StaticCall

Adds canonical name parsing, which allows us to parse Function
and Field values. Adds ImmutableList, Instance, and TypeParameter
parsing.

Results from compiling hello world program:

* Early round trip:
    * Contains unhandled instructions: 4070
    * Failed during deserialization: 2
    * Successful round trips: 106
* Late round trip:
    * Contains unhandled instructions: 3789
    * Failed during deserialization: 4
    * Successful round trips: 385

Bug: #36882
Change-Id: If9cb4c133e3ba8c62016e545f8471c67cc126290
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113684
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
@sstrickl
Copy link
Contributor

sstrickl commented Sep 3, 2019

After d2e99c4, we started seeing the following failures:

BOT TEST RESULT EXPECTED
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/2 Crash Pass
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/3 Crash Pass
vm-kernel-precomp-win-release-simarm64 vm/dart/il_round_trip_deserialization_test/2 Crash Pass
vm-kernel-precomp-win-release-simarm64 vm/dart/il_round_trip_deserialization_test/3 Crash Pass
vm-kernel-precomp-win-release-simarm64 vm/dart/il_round_trip_deserialization_test/6 Crash Pass
vm-kernel-precomp-win-release-simarm64 vm/dart/il_round_trip_deserialization_test/7 Crash Pass
vm-kernel-precomp-win-release-x64 vm/dart/il_round_trip_deserialization_test/2 Crash Pass
vm-kernel-precomp-win-release-x64 vm/dart/il_round_trip_deserialization_test/3 Crash Pass
vm-kernel-precomp-win-release-x64 vm/dart/il_round_trip_deserialization_test/6 Crash Pass
vm-kernel-precomp-win-release-x64 vm/dart/il_round_trip_deserialization_test/7 Crash Pass

Example logs:

Have approved for now, but will need to figure these out ASAP (or see if they're fixed by work in later CLs, in which case I may need to move that work forward and land separately first).

dart-bot pushed a commit that referenced this issue Sep 5, 2019
Adds support for:
  DebugStepCheck
  StoreInstanceField

Also:
* Adds context information to closure serialization.
* Adds parsing for serialized Closure and Slot values.
* Gets rid of some two-value enums in il.h that were acting as bools.

Results from compiling hello world program:

* Early round trip:
    * Contains unhandled instructions: 3433
    * Failed during deserialization: 2
    * Successful round trips: 745
* Late round trip:
    * Contains unhandled instructions: 3562
    * Failed during deserialization: 3
    * Successful round trips: 613

Bug: #36882
Change-Id: I409b232b64c4847623378462fc993460a858acb1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114840
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Sep 5, 2019
Adds support for:
  AllocateObject
  CheckNull
  LoadField
  StrictCompare

Results from compiling hello world program:

* Early round trip:
  * Contains unhandled instructions: 3065
  * Failed during deserialization: 6
  * Successful round trips: 1109
* Late round trip:
  * Contains unhandled instructions: 3085
  * Failed during deserialization: 7
  * Successful round trips: 1086

Bug: #36882
Change-Id: I09d25ca413a9facd27ba5a1b81e72a1fd126a3cc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114860
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@sstrickl
Copy link
Contributor

sstrickl commented Sep 5, 2019

Couple more issues approved after 7c8c197 (example log):

BOT TEST RESULT EXPECTED
vm-kernel-optcounter-threshold-linux-release-x64 vm/dart/il_round_trip_deserialization_test/2 Crash Pass
vm-kernel-optcounter-threshold-linux-release-x64 vm/dart/il_round_trip_deserialization_test/3 Crash Pass

At a good point to stop and look at these today, so doing that now.

@mkustermann
Copy link
Member

A few more from today:

BOT TEST RESULT EXPECTED
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/1 DartkCrash Pass
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/5 DartkCrash Pass
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/6 Crash Pass
vm-kernel-optcounter-threshold-linux-release-ia32 vm/dart/il_round_trip_deserialization_test/7 DartkCrash Pass

dart-bot pushed a commit that referenced this issue Sep 5, 2019
Also fix check that a new value hasn't been added to the
StoreBarrierType enum.

Bug: #36882
Change-Id: I8d781948082b45011466a9cc735ba2c94f130ea1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115708
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@ghost
Copy link

ghost commented Sep 10, 2019

FYI: dumping a bit more info on one of the failing tests:

$ tools/test.py -n dartk-optcounter-linux-debug-ia32 vm/dart/il_round_trip_deserialization_test/2

../../runtime/vm/compiler/backend/il_serializer.cc: 452: error: expected: instance_field_.is_final()
version=2.6.0-edge.b31566b29793247d0fdb2281a435e87fdae77cd5 (Thu Sep 5 12:56:49 2019 +0000) on "linux_ia32"
thread=256741, isolate=kernel-service(0x4184e00)
  pc 0x01ce123c fp 0xf6a3e678 dart::Profiler::DumpStackTrace(void*)
  pc 0x021f00a4 fp 0xf6a3e698 Dart_DumpNativeStackTrace
  pc 0x0194dbeb fp 0xf6a3e6c8 dart::Assert::Fail(char const*, ...)
  pc 0x01e5ac42 fp 0xf6a3e728 dart::FlowGraphSerializer::InstanceToSExp(dart::Instance const&)
  pc 0x01e5bb6e fp 0xf6a3e758 dart::FlowGraphSerializer::ObjectToSExp(dart::Object const&)
  pc 0x01e5949e fp 0xf6a3e7a8 dart::FlowGraphSerializer::ConstantPoolToSExp(dart::GraphEntryInstr*)
  pc 0x01e58017 fp 0xf6a3e7e8 dart::FlowGraphSerializer::FlowGraphToSExp()
  pc 0x01e57af3 fp 0xf6a3e868 dart::FlowGraphSerializer::SerializeToSExp(dart::Zone*, dart::FlowGraph const*)
  pc 0x01e2241a fp 0xf6a3e9c8 dart::FlowGraphDeserializer::RoundTripSerialization(dart::CompilerPassState*)
  pc 0x01ec1206 fp 0xf6a3ea08 out/DebugIA32/dart+0x1ac1206
  pc 0x01ebf625 fp 0xf6a3ea98 dart::CompilerPass::Run(dart::CompilerPassState*) const
  pc 0x01ebfcbc fp 0xf6a3eac8 dart::CompilerPass::RunPipeline(dart::CompilerPass::PipelineMode, dart::CompilerPassState*)
  pc 0x01f3da3a fp 0xf6a3ee38 dart::CompileParsedFunctionHelper::Compile(dart::CompilationPipeline*)
  pc 0x01f3ea0d fp 0xf6a3efc8 out/DebugIA32/dart+0x1b3ea0d
  pc 0x01f3f65f fp 0xf6a3f048 dart::Compiler::CompileOptimizedFunction(dart::Thread*, dart::Function const&, int)
  pc 0x01f407ed fp 0xf6a3f0c8 dart::BackgroundCompiler::Run()
  pc 0x01f418b7 fp 0xf6a3f0e8 out/DebugIA32/dart+0x1b418b7
  pc 0x01d93aaf fp 0xf6a3f138 dart::ThreadPool::Worker::Loop()
  pc 0x01d9364a fp 0xf6a3f188 dart::ThreadPool::Worker::Main(unsigned int)
  pc 0x01cdb4bf fp 0xf6a3f2d8 out/DebugIA32/dart+0x18db4bf
  pc 0xf7eb1982 fp 0xf6a3f3a8 /lib/i386-linux-gnu/libpthread.so.0+0x6982

@sstrickl
Copy link
Contributor

That dump strengthens a hypothesis I had for one possible reason for these failures, which is that we're getting non-canonical values in ConstantInstrs on some architectures/runs. Martin and I had seen this while trying to figure out an issue I could reproduce locally which led to 488a8ae, and looking at the results of CI after 488a8ae landed, it looks like some of these failures are indeed now passing.

That's not all the failures, but it's good to see some going Pass after at least.

@sstrickl
Copy link
Contributor

I believe the others are due to attempts to set instance fields in the background compiler while not at a safe point. (inst.SetField calls field.RecordStore, which both asserts IsOriginal on the field and calls guarded_cid(), which asserts !IsOriginal on the field if it is an instance field (always, in our case) and either not being done on the mutator thread or not currently at a safepoint.)

I've got a fix for this coming down the pipeline, will land as soon as it's checked off on, and hopefully that fixes the other current crashes.

@sstrickl
Copy link
Contributor

Related issue we should handle in the serializer: #38297

dart-bot pushed a commit that referenced this issue Sep 12, 2019
Adds support for:
  AssertBoolean
  InstanceCall

Also handles deserializing of TypeRefs and
serialization/deserialization of non-megamorphic
ICData.

Results from compiling hello world program:

* Early round trip:
  * Contains unhandled instructions: 2469
  * Failed during deserialization: 9
  * Successful round trips: 1772
* Late round trip:
  * Contains unhandled instructions: 2892
  * Failed during deserialization: 11
  * Successful round trips: 1268

Deserialization failure reasons:
* Early round trip:
  * no handling for local functions: 9
* Late round trip:
  * no handling for local functions: 6
  * out of range index for pushed argument: 5
    (see #38354)

Bug: #36882
Change-Id: I7a3731d4cc3df0bfa1e270adbdfc2faddd110af0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115219
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Sep 12, 2019
Adds support for:
  AssignAssertable
  BooleanNegate
  LoadClassId
  NativeCallInstr
  Throw

Results from compiling hello world program:

* Early round trip:
  * Contains unhandled instructions: 1565
  * Failed during deserialization: 10
  * Successful round trips: 2696
* Late round trip:
  * Contains unhandled instructions: 2404
  * Failed during deserialization: 12
  * Successful round trips: 1755

Deserialization failure reasons:
* Early round trip:
  * no handling for local functions: 10
* Late round trip:
  * no handling for local functions: 7
  * out of range index for pushed argument: 5
    (see #38354)

Bug: #36882
Change-Id: I56f875a68876b4d94ac1256be207df8b98ba0ea0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116320
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
@mkustermann
Copy link
Member

mkustermann commented Sep 12, 2019

After 420b292 landed, I've just approved the following failures:

BOT TEST RESULT EXPECTED
vm-kernel-optcounter-threshold-linux-release-x64 vm/dart/il_round_trip_deserialization_test/5 Crash Pass
vm-kernel-optcounter-threshold-linux-release-x64 vm/dart/il_round_trip_deserialization_test/7 Crash Pass

dart-bot pushed a commit that referenced this issue Sep 20, 2019
Printing out the flow graph before and after round trip serialization
revealed slight differences between the flow graphs that might lead
to either bugs or different results in compilation. Remove these
differences.

Also in this change:
* Remove cases where printing or serializing a flow graph could have
  the side effect of creating new `CompileType`s or `AbstractType`s
  not previously in the graph.
* Ensure reaching types for `Value`s are cloned if the binding
  definition changes and check that reaching types either have no
  owner or are owned by the `Value`'s definition with the
  FlowGraphChecker. (Before this change, we had `CompileType`s in
  the graph whose owner had long since been removed from it.)
* Adds `ASSERT`s to check that unexpected non-canonical `Instances`
  are not allowed as the value of `ConstantInstr`s and adds some
  canonicalization that did not previously occur.

Bug: #36882
Change-Id: I334ffa2a6383291a7cb318343c71bd55a41269a1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117143
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
dart-bot pushed a commit that referenced this issue Sep 20, 2019
Bug: #36882
Change-Id: Ief57fbd655767171d98aaf14002ff49d6da4fd1e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/118283
Reviewed-by: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Teagan Strickland <sstrickl@google.com>
@mkustermann
Copy link
Member

mkustermann commented Oct 11, 2019

Just an FYI: The recent enabling of bytecode has made the two vm/dart/il_round_trip_deserialization_test/{5,7} tests pass. Though the underlying bug is probably still there.

(In the meantime bytecode was turned off again)

dart-bot pushed a commit that referenced this issue Oct 24, 2019
Bug: #36882
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Change-Id: I20c5a8298f0f6444a326a63fe86ad106c3940317
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117543
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@mehmetf
Copy link
Contributor

mehmetf commented Apr 3, 2020

Note that 6b44c63 is causing {1,3,5,7} to crash again.

dart-bot pushed a commit that referenced this issue Oct 30, 2020
This test was added a while ago during development of IL serialization
and deserialization (#36882),
which wasn't completed.

Since development of serialization/deserialization was put on hold,
this test was failing with a lot of flakiness, flipping between
Crash/Timeout/RuntimeError on unrelated changes (#43617,
#43359, #36882),
and even turning bots purple (https://ci.chromium.org/p/dart/builders/ci.sandbox/vm-kernel-precomp-nnbd-linux-debug-simarm_x64/32).

There are no signs that underlying problems with IL serialization/deserialization
are going to be addressed, but this test requires constant attention.
If implementation of #36882 is
ever resumed, it would be possible to resurrect this test from git history.

Closes #43617
Closes #43359
Closes #42441

Issue: #36882
Change-Id: I5a8789397efcb69ad30257077aee0a3e195c8a69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169940
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
@mkustermann
Copy link
Member

The S-expression like IR serializer has been discontinued due to our main possible user going away. Closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants