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

internal: split vmgen/linker state from TCtx #843

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 16, 2023

Summary

Move much of the data and associated types for the VM code generator and
linker into vmgen and the new vmlinker module. Having the code-
generation-related state in the vmgen module, means that the VM code-
generator state can now depend on modules that should not be a
transitive dependency (through vmdef) of all other VM-related modules.

Details

The linker-related types IdentPattern and LinkIndex plus the
routines for working with them are moved to the vmlinker module -- the
linker-related state is moved into the new LinkerData type. Due
vmcompilerserdes currently requiring access to the link table, the
LinkerData instance has to be still stored with TCtx instead of
the more correct JitState.

For the VM code generator, all relevant types, except for the
diagnostics, are moved from vmdef into vmgen. The TCtx used by
vmgen is now an alias to the new CodeGenCtx type, which stores all
the input, output, and other contextual state required for code
generation.

When using vmjit, much of the data read or written to by the code
generator is part of the VM execution environment (TCtx), and in order
to not having to either use pointers or pass all of it along as
procedure parameters, the relevant data is swapped between TCtx and
CodeGenCtx both before and after invoking the code generator.
vmjit.genExpr is slightly adjusted for this to work uniformly across
the genX procedures: it now also emits an Eof instruction.

vmbackend now stores an instance of CodeGenCtx instead of TCtx.
Once code generation is finished, a temporary VM execution environment
is set-up, populated, and then serialized.

Finally, vmrunner is updated to not rely on (now non-existent)
TCtx.callbackKeys field. The list of expected override patterns is
instead taken from the packed context and passed to registerCallbacks
directly.

Move emission of the `Ret` instruction into `vmgen.genExpr` and have the
`genExpr` procedure in `vmjit` emit an 'eof' instruction. This brings
the shape of the latter closer to that of `genStmt`.
The state and required contextual data is moved into the new
`CodeGenCtx` type owned by `vmgen`. All types only referenced by the
now-moved code are moved with it.
The field doesn't exist anymore.
@zerbina zerbina added refactor Implementation refactor compiler/backend Related to backend system of the compiler labels Aug 16, 2023
@zerbina zerbina added this to the MIR phase milestone Aug 16, 2023
@zerbina zerbina mentioned this pull request Aug 16, 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.

Looks good, I like how vmdef is becoming more focused and the modules become more independent of each other.

I did find a comment typo, but it's non-blocking. The suggestion for documenting the invariant is also non-blocking, just occurred to me that the trick with more DOD style approaches is signalling cardinality/relationships of the data contained therein -- I might be mistaken.

compiler/vm/vmjit.nim Outdated Show resolved Hide resolved
compiler/vm/vmlinker.nim Show resolved Hide resolved
Co-authored-by: Saem Ghani <saemghani+github@gmail.com>
@saem
Copy link
Collaborator

saem commented Aug 17, 2023

/merge

@github-actions
Copy link

Merge requested by: @saem

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


Notes for Reviewers

  • part of the endeavor of splitting up TCtx
  • prepares for further improvements/simplifications of CgNode, as those will require new data types that should be imported into vmdef

@chore-runner chore-runner bot added this pull request to the merge queue Aug 17, 2023
Merged via the queue into nim-works:devel with commit 255c9a6 Aug 17, 2023
18 checks passed
@zerbina zerbina deleted the separate-vmgen-state-from-tctx branch August 21, 2023 17:34
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 refactor Implementation refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants