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

cgen: improve code generation for seq types #1272

Merged
merged 3 commits into from
Apr 9, 2024

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Apr 8, 2024

Summary

  • only emit a single seq payload type definition per C file
  • only emit the full C type definition for a seq if not used in a
    pointer-like type (e.g.: ptr seq[T], ref seq[T])

Together, these improvements reduce the amount of C code output by the
NimSkull compiler, slightly improving compile times.

Details

Move the tySequence handling from getTypeDescWeak to
getTypeDescAux, preventing different instances referring to the same
type, or the same instance, being pushed to the type stack multiple
times within a single module.

For each tySequence type instance pushed to the type stack, a payload
is emitted, with preventing duplicates previously being deferred to the
C compiler through use of a C pre-processor guard (#ifdef). Since the
type instance is now only pushed to the type stack once per module, the
pre-processor guard is unnecessary and thus removed.

In addition, for ptr seq, ref seq, etc. types, only a forward-
declaration of the sequence type is requested, preventing the full type
and all its dependencies being pulled in.

Finally, the trecursive_table test, intended to catch C code
generator bugs with recursive seq types, is fixed. Since p was
neither used nor exported, the T type was never actually processed by
the code generator.

Move the `tySequence` handling from `getTypeDescWeak` to `getTypeDesc`.
Behaviour is largely the same, but with the important difference that
the `tySequence` type is only pushed to the type stack *once*, removing
the need to use a C `#ifdef` pre-processor block around the payload
struct definition.
* for pointer indirections, only a forward declaration is requested
* in field contexts, `getTypeDescAux` can be called directly
The `T` object type wasn't actually used anywhere in alive code,
meaning no code was generated for it, and thus that the regression it
intended to catch would have never been caught.

The `p` procedure is now exported, forcing code generation to run for
it (and subsequently the `T` type).
@zerbina zerbina added the compiler/backend Related to backend system of the compiler label Apr 8, 2024
@zerbina zerbina added this to the C backend rework milestone Apr 8, 2024
@saem
Copy link
Collaborator

saem commented Apr 9, 2024

/merge

Copy link

github-actions bot commented Apr 9, 2024

Merge requested by: @saem

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


Notes for Reviewers

  • a small improvement noticed while introducing the new type IR for the MIR

@chore-runner chore-runner bot added this pull request to the merge queue Apr 9, 2024
Merged via the queue into nim-works:devel with commit b8a4bea Apr 9, 2024
31 checks passed
@zerbina zerbina deleted the cgen-improve-seq-codegen branch April 9, 2024 18:19
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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants