VM: Implement support for constants being evaluated by front end #36220

a-siva · 2019-03-14T19:34:07Z

This is the tracker issue for implementing support in the VM for constants being evaluated by the front end

Implement support in JIT mode
The bytecode compiler currently implements constant expression evaluation, with CFE doing it that becomes redundant
The AOT pipeline implements constant expression evaluation as a kernel to kernel transformation, with CFE doing it that becomes redundant

Language tracking issue: dart-lang/language#60
Frontend implementation tracking issue: #35079

a-siva · 2019-03-14T19:34:30Z

/cc @aartbik @alexmarkov who should own this issue?

aartbik · 2019-03-14T19:46:20Z

I marked us both as owner for now. Alex probably wants to do the bytecode side. I can help with the other stuff after consulting with Alex.

Rationale: The constant evaluator deals with two sorts of offsets, the true offset into the kernel data, and the offset of a constant relative to the start of the constant table (compensated for a variable-size int prefix at the true base). The DAG test was comparing the latter against the former (which was always true, since the true offsets are much larger). This fixes this omission. #36220 Change-Id: I71f37bfb2bd3432e52d5086e89a62115f0cacacd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97421 Commit-Queue: Aart Bik <[email protected]> Reviewed-by: Alexander Markov <[email protected]>

dgrove · 2019-04-03T00:32:16Z

Any updates on this issue?

aartbik · 2019-04-03T00:39:55Z

I am not aware of blocking VM work. For JIT, we could lazily instead of eagerly evaluate the constants in the constant table (which becomes larger) to reduce compile-time, and I have a prototype CL for that. For AOT, I don't think anything is not working, and removing code would be just a cleanup. I let Alex comment on bytecode status.

a-siva · 2019-04-03T00:50:14Z

I think this issue can be taken off from the 2.3 milestone list, it is more for clean up work to remove the redundant constant expressions evaluation code from the AOT and bytecode paths once CFE has solid support for constant expressions evaluation.

Rationale: Previously all constants were read from the constants table, whether they were needed (at that point or at all) or not. This CL replaces that eager reading with a lazy reading that makes the raw bytes of the constants table available through KernelProgramInfo and attaches a hashmap to KernelProgramInfo, which maps offsets into the constant table to evaluated constants. The maps starts empty and on a miss, the constant is evaluated, but no sooner than when it it needed. #36220 Change-Id: Ief4df3d70c950c4beb61d896632ce06cfd12d525 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/99088 Commit-Queue: Aart Bik <[email protected]> Reviewed-by: Alexander Markov <[email protected]>

aartbik · 2019-06-21T18:20:50Z

Lazy constant reading has been submitted. Items and ideas to follow up on:

(1) by setting the raw-bytes array and hash map from the start, we may be able to completely avoid the "pending" and "delayed" data structures; we should be evaluate every constant when needed first
(2) by peeking into pragma's we may avoid the cyclic detector altogether
(3) removed unused code and data structures when this stabilizes

aartbik · 2019-06-21T18:25:00Z

Approved fails to follow-up on:

|vm-kernel-precomp-bare-linux-release-x64 |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-linux-product-x64      |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-linux-release-simarm64 |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-linux-release-x64      |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-mac-release-simarm64   |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-win-release-simarm64   |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-win-release-x64        |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-linux-debug-x64 |language_2/const_inference_test    |RuntimeError
|vm-kernel-precomp-bare-linux-release-simarm   |language_2/const_inference_test | RuntimeError
|vm-kernel-precomp-bare-linux-release-simarm64 |language_2/const_inference_test | RuntimeError
|vm-kernel-precomp-linux-release-simarm        |language_2/const_inference_test | RuntimeError
|vm-kernel-precomp-android-release-arm |language_2/const_inference_test |RuntimeError
|vm-kernel-precomp-linux-debug-x64 | language_2/cyclic_metadata_test/02 |Crash

aartbik · 2019-06-21T22:16:40Z

A fix for the cyclic test crash is pending (see https://dart-review.googlesource.com/c/sdk/+/107000). As for the constant inference, the difference is right after canonicalize:

v16 <- Constant(#Closure: (Null) => Null from Function 'constFunction': static.) T{_Closure}
vs.
v16 <- Constant(#Closure: (dynamic) => dynamic from Function 'constFunction': static.) T{_Closure}

Rationale: Due to the cycle breaking reading of VM annotations, we should be prepared to see a class loaded inside class loading wrap-up. #36220 Change-Id: I46665989e489ada8ed2414a57a789b9c2d9c9f7a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107000 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Aart Bik <[email protected]>

aartbik · 2019-06-21T23:25:33Z

The canonicalize difference happens in LoadFieldInstr::Canonicalize() with the following code

   if (Evaluate(instance()->BoundConstant(), &result)) {
      if (result.IsSmi() || result.IsOld()) {
          return flow_graph->GetConstant(result);
      }
  }

with Evaluate() returning the different values

v15 <- Constant(#Instance of 'C<Null>') T{C}
Instance of 'C<Null>'
Closure: (Null) => Null from Function 'constFunction': static.

v15 <- Constant(#Instance of 'C<Null>') T{C}
Instance of 'C<Null>'
Closure: (dynamic) => dynamic from Function 'constFunction': static.

Indeed, this is a difference already at constant evaluation level:

C2 Closure: (Null) => Null from Function 'constFunction': static
C2 Closure: (dynamic) => dynamic from Function 'constFunction': static.

aartbik · 2019-06-22T00:58:35Z

Fix for crash confirmed:

The following tests are now succeeding:

BOT/CONFIG	TEST
vm-kernel-precomp-linux-debug-x64	language_2/cyclic_metadata_test/02

Fix for the failure is pending at https://dart-review.googlesource.com/c/sdk/+/107064

Rationale: As shown in the comment, I was already not sure about what argument to use. Clearly one of the two type arguments is not needed. This fixes the language_2/const_inference_test failure that was temporarily approved. #36220 Change-Id: I3dd8ddf0e3cf0e992164c005b08ffd59306041f7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107064 Reviewed-by: Aart Bik <[email protected]> Commit-Queue: Aart Bik <[email protected]> Auto-Submit: Aart Bik <[email protected]>

aartbik · 2019-06-22T04:06:08Z

Confirmed fixes for all vm bots. Sample. below.

The following tests are now succeeding:

BOT/CONFIG	TEST
vm-kernel-precomp-linux-product-x64	language_2/const_inference_test

Rationale: Only look for "external" and "pragma" to avoid cyclic or unloaded class evaluation #37533 #36220 Change-Id: I78ef5dcd2c7e7dee0d196394320e994f8aa8a695 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109300 Commit-Queue: Aart Bik <[email protected]> Reviewed-by: Alexander Markov <[email protected]>

aartbik · 2019-07-22T23:00:23Z

@a-siva any actionable items left for this?

a-siva · 2019-07-23T00:32:42Z

I think there are no more known items, let me close it. If we find bugs we can open new issues for the failures.

a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Mar 14, 2019

a-siva added this to the Dart2.3 milestone Mar 14, 2019

mit-mit mentioned this issue Mar 14, 2019

Implementation issue for constant changes Q4'18 dart-lang/language#60

Closed

18 tasks

a-siva mentioned this issue Mar 14, 2019

Dart vm cannot run dill file with precomputed constants in jit-mode #36153

Closed

aartbik assigned aartbik and alexmarkov Mar 14, 2019

a-siva added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Mar 28, 2019

dgrove removed this from the Dart2.3 milestone Apr 3, 2019

a-siva added P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels May 24, 2019

aartbik mentioned this issue Jun 21, 2019

Implement lazy evaluations of constants instead of eagerly reading the constant table and evaluating all constants while loading the kernel file #36584

Closed

aartbik mentioned this issue Jul 15, 2019

VM: Crash while handling reference from an instance constant to a class which is not loaded yet #37533

Closed

a-siva closed this as completed Jul 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VM: Implement support for constants being evaluated by front end #36220

VM: Implement support for constants being evaluated by front end #36220

a-siva commented Mar 14, 2019 •

edited

Loading

a-siva commented Mar 14, 2019

aartbik commented Mar 14, 2019

dgrove commented Apr 3, 2019

aartbik commented Apr 3, 2019 •

edited

Loading

a-siva commented Apr 3, 2019

aartbik commented Jun 21, 2019

aartbik commented Jun 21, 2019 •

edited

Loading

aartbik commented Jun 21, 2019

aartbik commented Jun 21, 2019 •

edited

Loading

aartbik commented Jun 22, 2019 •

edited

Loading

aartbik commented Jun 22, 2019

aartbik commented Jul 22, 2019

a-siva commented Jul 23, 2019

VM: Implement support for constants being evaluated by front end #36220

VM: Implement support for constants being evaluated by front end #36220

Comments

a-siva commented Mar 14, 2019 • edited Loading

a-siva commented Mar 14, 2019

aartbik commented Mar 14, 2019

dgrove commented Apr 3, 2019

aartbik commented Apr 3, 2019 • edited Loading

a-siva commented Apr 3, 2019

aartbik commented Jun 21, 2019

aartbik commented Jun 21, 2019 • edited Loading

aartbik commented Jun 21, 2019

aartbik commented Jun 21, 2019 • edited Loading

aartbik commented Jun 22, 2019 • edited Loading

aartbik commented Jun 22, 2019

aartbik commented Jul 22, 2019

a-siva commented Jul 23, 2019

a-siva commented Mar 14, 2019 •

edited

Loading

aartbik commented Apr 3, 2019 •

edited

Loading

aartbik commented Jun 21, 2019 •

edited

Loading

aartbik commented Jun 21, 2019 •

edited

Loading

aartbik commented Jun 22, 2019 •

edited

Loading