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

vmjit: use MIR-based dependency discovery #810

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jul 22, 2023

Summary

Move the JIT-related state from TCtx into a standalone type and
replace usage of the custom PNode-based dependency discovery with that
used by the backends.

The intention is to bring the processing in vmjit closer to that of
the backends (because vmjit can also be considered a backend) and to
prepare for the introduction of a dedicated code-generator/backend IR by
removing a usage of PNode. In addition, decoupling the JIT state from
TCtx is part of the on-going project of splitting up TCtx.

For lifted globals used in code running at compile-time or in the
context of NimScript, there's a small difference in behaviour:

proc p() =
  var x {.global.}: int
  ...

x is now reset to its default value when control-flow reaches the
declaration -- previously it wasn't. This is likely going to change
again in the future.

Details

The key changes related to TCtx:

  • introduce the JitState type, which represents the non-temporal JIT
    state. The compile-time evaluation context (PEvalContext) stores an
    instance of it
  • remove LinkState and usages thereof; it is replaced by the
    DiscoveryData type also used by the backends
  • as an interim solution until vmbackend.nim uses its own context
    type, add the collectedGlobals field to TCtx and replace usages of
    LinkState in vmbackend with it

The processing logic in vmjit now performs the following steps:

  1. generate MIR code for the input
  2. scan for definitions of globals and register them
  3. rewrite definitions of globals into assignment
  4. discover used constants and procedures

Apart from the missing handling of lifted globals, this is the same as
what the unified backend processing does. Step 2 and 4 were previously
implemented by the now-removed gatherDependencies.

Before invoking vmgen, the newly discovered entities are registered
in vmgen's link table, with the entities' index in the DiscoveryData
sequences being used as the link indices. In case of an error during
code generation, the DiscoveryData sequences are rolled back to their
state prior to code generation, which ensures that only entities used in
valid code are registered with DiscoveryData. Committing to the
entities only happens when they're loaded into the VM's execution
environment.

Rewriting the definitions of globals into assignments means that var
statements involving globals don't reach vmgen anymore and thus
that the corresponding logic in vmgen can be dropped.

Finally, the documentation of the vmjit module is updated to better
describe the module's purpose.

Summary
=======

Move the JIT-related state from `TCtx` into a standalone type and
replace usage of the custom `PNode`-based dependency discovery with that
used by the backends.

The intention is to bring the processing in `vmjit` closer to that of
the backends (because `vmjit` can also be considered a backend) and to
prepare for the introduction of a dedicated code-generator/backend IR by
removing a usage of `PNode`. In addition, decoupling the JIT-state from
`TCtx` is part of the on-going project of splitting up `TCtx`.

For lifted globals used in code running at compile-time or in the
context of NimScript, there's a small difference in behaviour: they are
now reset to their default value when control-flow reaches the
definition statement, for now making them work like locals.

Details
=======

The key changes related to `TCtx`:
* introduce the `JitState` type, which represents the non-temporal JIT
  state. The compile-time evaluation context (`PEvalContext`) stores an
  instance of it
* remove `LinkState` and usages thereof; it is replaced by the
  `DiscoveryData` type also used by the backends
* as an interim solution until `vmbackend.nim` uses its own context
  type, add the `collectedGlobals` field to `TCtx` and replace usages of
  `LinkState` in `vmbackend` with it

The processing logic in `vmjit` now performs the following steps:
1. generate MIR code for the input
2. scan for definitions of globals and register them
3. rewrite definitions of globals into assignment
4. discover used constants and procedures

Apart from the missing handling of lifted globals, this is the same as
what the unified backend processing does. Step 2 and 4 were previously
implemented by the now-removed ``gatherDependencies``.

Before invoking `vmgen`, the newly discovered entities are registered
in `vmgen`'s link table, with the entities' index in the `DiscoveryData`
sequences being used as the link indices. In case of an error during
code generation, the `DiscoveryData` sequences are rolled back to their
state prior to code generation, which ensures that only entities used in
valid code are registered with `DiscoveryData`. Committing to the
entities only happens when they're loaded into the VM's execution
environment.

Rewriting the definitions of globals into assignments means that `var`
statements involving globals don't reach `vmgen` anymore and thus
that the corresponding logic in `vmgen` can be dropped.

Finally, the documentation of the `vmjit` module is updated to better
describe the module's purpose.
@zerbina zerbina added compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features. labels Jul 22, 2023
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers and the question isn't pressing.

The bearing out of VM state is especially nice. The bigger questions about globals are still fuzzy. We should discuss them separately, they cause fairly substantial issues with modularity.

compiler/vm/compilerbridge.nim Outdated Show resolved Hide resolved
#
# const c = block: (var x = 0; x)
#
# If `c` is defined at the top-level, then `x` is a "global" variable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, that's busted. I guess not from a scope perspective but a point of declaration perspective (owner)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, although, to my knowledge, this doesn't cause any direct issues at the moment. I'm still unsure about what the "correct" owner would be for x here.

Declarations in compile-time-only contexts like the left-hand side of a const declaration, static expressions, static blocks, etc. generally have the problem that they're treated like normal expression and statements by semantic analysis (with detection of "not accessible in this context" errors being pushed into vmgen).

@zerbina zerbina marked this pull request as draft July 24, 2023 14:14
Co-authored-by: Saem Ghani <[email protected]>
@zerbina zerbina marked this pull request as ready for review July 24, 2023 14:46
@zerbina
Copy link
Collaborator Author

zerbina commented Jul 24, 2023

I've updated the PR description to make it clearer what changes regarding lifted globals, namely that only the behaviour in case of no initializer is now different.

@zerbina
Copy link
Collaborator Author

zerbina commented Jul 25, 2023

/merge

@github-actions
Copy link

Merge requested by: @zerbina

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Jul 25, 2023
Merged via the queue into nim-works:devel with commit 963542e Jul 25, 2023
@zerbina zerbina deleted the vmjit-use-mir-based-discovery branch July 25, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants