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

codegen: don't use PSym for labels #940

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Oct 4, 2023

Summary

In the code generators, represent block labels with a dedicated type
and
node, instead of using PSym for that. This is an internal-only
change,
and another step towards removing all PSym usage from the code
generators.

Details

  • add the cnkLabel node kind to CgNode
  • it stores a BlockId , and appears as sub-node for both
    cnkBlockStmt
    and cnkBreakStmt
  • a BlockId is not a unique ID within a Body -- instead it
    identifies the enclosing block for a break. This matches how the
    code generators previously used skLabel symbols
  • cgirgen now no longer queries or modifies the identifier cache
    (IdentCache), so all usages of the type are removed. The idents
    import is turned into a narrow import

The code generators are updated accordingly:

  • cgen uses its blocks stack not only for cnkBlockStmts, but for
    C blocks ({}) in general. Therefore, TBlock stores the BlockId
    offset by 1, leaving '0' to mean "not corresponding to a
    cnkBlockStmt"
  • jsgen uses the BlockId as an index into the blocks stack, which
    is now a seq[int] (the previously used TBlock type had only a
    single field)
  • vmgen also uses the BlockId as an index into its blocks stack,
    making the previous search logic obsolete. The withBlock and
    popBlocks routines were only used in a single place -- they're
    inlined and removed. In addition, the blocks stack embeds the
    ending list directly, removing the need for the TBlock type

Summary
=======

In the code generators, represent block labels with a dedicated type and
node, instead of using `PSym` for that. This is an internal-only change,
and another step towards removing all `PSym` usage from the code
generators.

Details
=======

* add the `cnkLabel` node kind to `CgNode`
* it stores a `BlockId`, and appears as sub-node for both `cnkBlockStmt`
  and `cnkBreakStmt`
* a `BlockId` is not a unique ID within a `Body` -- instead it
  identifies the enclosing `block` for a `break`. This matches how the
  code generators previously used `skLabel` symbols
* `cgirgen` now no longer queries or modifies the identifier cache
  (`IdentCache`), so all usages of the type are removed. The `idents`
  import is turned into a narrow import

The code generators are updated accordingly:

* `cgen` uses its `blocks` stack not only for `cnkBlockStmt`s, but for
  C blocks (`{}`) in general. Therefore, `TBlock` stores the  `BlockId`
  offset by 1, leaving '0' to mean "not corresponding to a
  `cnkBlockStmt`"
* `jsgen` uses the `BlockId` as an index into the `blocks` stack, which
  is now a `seq[int]` (the previously used `TBlock` type had only a
  single field)
* `vmgen` also uses the `BlockId` as an index into its `blocks` stack,
  making the previous search logic obsolete. The `withBlock` and
  `popBlocks` routines were only used in a single place -- they're
  inlined and removed. In addition, the `blocks` stack embeds the fixup
  list directly, removing the need for the `TBlock` type
@zerbina zerbina added refactor Implementation refactor compiler/backend Related to backend system of the compiler labels Oct 4, 2023
@zerbina zerbina added this to the MIR phase milestone Oct 4, 2023
@saem
Copy link
Collaborator

saem commented Oct 4, 2023

/merge

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Merge requested by: @saem

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 Oct 4, 2023
Merged via the queue into nim-works:devel with commit b1b0b92 Oct 4, 2023
18 checks passed
@zerbina zerbina deleted the codegen-dont-use-psym-for-labels branch October 4, 2023 22:13
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