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.

Sign up for GitHub

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

Jump to bottom

Make JIT/AOT VM pipeline aware of closure annotations #45987

Closed
2 tasks done
mkustermann opened this issue May 12, 2021 · 2 comments
Closed
2 tasks done

Make JIT/AOT VM pipeline aware of closure annotations #45987

mkustermann opened this issue May 12, 2021 · 2 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@mkustermann
Copy link
Member

mkustermann commented May 12, 2021 •
edited by alexmarkov
Loading

Right now TFA as well as the kernel reader in the VM are not taking annotations for local closures into account. We should enable this to e.g. guide our inliner to recognize @pragma('vm:prefer-inline') or other related vm-specific annotations.

The kernel AST stores those annotations in FunctionDeclaration.variable.annotations.

Let's track this here with at least the sub-parts:

  • Make TFA + tree shaker aware of those annotations and handle them in a similar way to member annotations (see cl/199200)
  • Make flow graph builder recognize annotations on local functions when they are lazily created.

/cc @alexmarkov @mraleph

/cc @cskau-g Due to connection with #45710
@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label May 12, 2021
dart-bot pushed a commit that referenced this issue May 14, 2021
@alexmarkov
[vm/aot] Keep pragmas on VariableDeclaration nodes in tree shaker
d39e794 
VM can use pragmas on local functions, which are actually put on
VariableDeclaration nodes. This change teaches TFA tree shaker to
keep such pragmas.

TEST=pkg/vm/testcases/transformations/type_flow/transformer/pragmas.dart
Issue: #45987
Change-Id: Ic2db375a93b539a131eca2431bef0e317a4d1b2b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199520
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
dart-bot pushed a commit that referenced this issue May 18, 2021
[vm/debugger] Adds pragma support for func. decl.
6523c58 
This fixes issue #45710 by adding support for pragma
annotations on local functions which are lazily created
via the flow graph builder (as opposed to the kernel loader).

TEST=Added regression test to notify_debugger_on_exception_test.dart

Bug: #45710, #45987
Change-Id: I13f2f8d2b7d05ea1cb423c142537789e99a919d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199420
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@ghost
Copy link

ghost commented May 18, 2021

Possibly both the sub-parts are addressed now. Is there more that we need to address before closing this issue?

@mkustermann
Copy link
Member Author

Yes, seems like the inliner is also respecting the pragmas on closures now.

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

No branches or pull requests
1 participant
@mkustermann