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

replaces implicit passes array registered at runtime with explicit function calls; simplify compilation pipeline #21444

Merged
merged 17 commits into from Mar 3, 2023

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Feb 28, 2023

ref nim-lang/RFCs#503
follow up #21379

The processModule should handle all the logics by calling parser, sem, transf, codegen in order.

I'm keeping the passes interface for nimsuggest, nimfix and third-party library for backwards compatibilities.

@ringabout
Copy link
Member Author

ringabout commented Feb 28, 2023

I have few questions.

It seems that I need to duplicate lots of code to replace passes. Could I only make the pipeline explicit for C like backends and keep the rest under the umbrella of the passes? And are there other changes I should make to make the pipeline explicit?

@arnetheduck
Copy link
Contributor

as far as I'm concerned, in an ideal world the c backend is something that would not be invoked at all until semantics and transformations have completed, more or less - ie we should not be calling the c backend at all until we know there's a sound nim ast - this way, we shield the semantic and transformation passes from what the C backend ultimately does - it would also mean splitting out any "useful" pre-c-generation work that the C backend does (ie calling transformations)... of course, there are degrees to this ideal and it's important that transformations don't lose significant optimization information that the backends can exploit, so there's a balance here..

Effectively, what we end up with is a "mid-level" IR comprising of a subset of the full AST, that still can be used for "nim-level" optimizations, ie where knowledge of nim semantics allows the implementation of useful code transformations - after that, the C pipeline takes or (or in the case of nlvm, the llvm pipeline which presents several opportunities for optimizations that the C backend might struggle with (specially for things like callbacks, assumption checking, defects etc).

@arnetheduck
Copy link
Contributor

ah, which brings me to the other point: it would be nice at some point to be able to replace the VM with a different one - in nlvm, the possibility exists to use the same IR for compile-time execution as for code generation - this means that if a function is used at compile time, it would be practical if llvm code generation could happen before the function is used in the VM: the benefit of this approach in nlvm is that the function only has to be transformed once to IR if the function is used both during compile and runtime - in particular, we can run llvm optimizers on the function before doing the compile-time execution.

This suggests that the pipeline might need some additional "plugin" points in the future, if this use case is to be covered - of course, this kind of stuff can also be maintained in an nlvm-specific fork of Nim.

@ringabout ringabout changed the title abolish using passes in the compiler; simplify compilation pipeline replaces implicit passes array registed at runtime with explicit function calls; simplify compilation pipeline Mar 1, 2023
@ringabout ringabout marked this pull request as ready for review March 2, 2023 04:24
@ringabout ringabout mentioned this pull request Mar 2, 2023
12 tasks
@Varriount Varriount requested a review from Araq March 2, 2023 19:26
@Varriount Varriount added the Requires Araq To Merge PR should only be merged by Araq label Mar 2, 2023
result = semWithPContext(PContext(context), n)

const semPass* = makePass(mySemOpen, mySemProcess, mySemClose,
isFrontend = true)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this still required?

Copy link
Member Author

@ringabout ringabout Mar 3, 2023

Choose a reason for hiding this comment

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

It is not required. It is kept for tools like nimsuggest, drnim and nimfix and backward compatibilities. I have removed passes imports from other modules in the compiler. I can remove passes from nimsuggest and nimfix as well.

Copy link
Member

Choose a reason for hiding this comment

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

Can be done in a followup PR.

@Araq Araq merged commit d51a392 into devel Mar 3, 2023
19 checks passed
@Araq Araq deleted the pr_passes_reword branch March 3, 2023 06:36
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from d51a392

Hint: mm: orc; opt: speed; options: -d:release
166204 lines; 8.110s; 612.531MiB peakmem

@ringabout ringabout changed the title replaces implicit passes array registed at runtime with explicit function calls; simplify compilation pipeline replaces implicit passes array registered at runtime with explicit function calls; simplify compilation pipeline Mar 6, 2023
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
…tion calls; simplify compilation pipeline (nim-lang#21444)

* abolish using passes in the compiler; simplify compilation pipeline

* duplicate code

* Really cool to have the same signature...

* haul

* unify other backends

* refactor process

* introduce PipelinePhase

* refactor compiler

* fixes passes

* fixes nimsuggest

* add a sentinel

* enable docs checkj

* activate doc testing

* clean up

* complete cleanups
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
…tion calls; simplify compilation pipeline (nim-lang#21444)

* abolish using passes in the compiler; simplify compilation pipeline

* duplicate code

* Really cool to have the same signature...

* haul

* unify other backends

* refactor process

* introduce PipelinePhase

* refactor compiler

* fixes passes

* fixes nimsuggest

* add a sentinel

* enable docs checkj

* activate doc testing

* clean up

* complete cleanups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires Araq To Merge PR should only be merged by Araq
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants