Refactor compiler #746
Refactor compiler #746
Conversation
aykevl
force-pushed
the
refactor-compiler
branch
from
06c602a to
1de0b8c
Compare
|
This requires #773 to be merged first (and probably requires some other changes), to make this a clean PR. |
aykevl
force-pushed
the
refactor-compiler
branch
2 times, most recently
from
9b78cdb to
fcd9f17
Compare
aykevl
force-pushed
the
refactor-compiler
branch
2 times, most recently
from
d4d2959 to
9840800
Compare
aykevl
force-pushed
the
refactor-compiler
branch
from
9840800 to
0401f5c
Compare
This is the first commit in a series to refactor the compiler. The
intention is to make sure every function to be compiled eventually has
its own IR builder. This will make it much easier to do other
refactorings in the future:
* Most code won't depend (directly) on the central Compiler object,
perhaps making it possible to eliminate it in the future. Right now
it's embedded in the `builder` struct but individual fields from the
`Compiler` can easily be moved into the `builder` object.
* Some functions are not directly exposed in Go SSA, they are wrapper
functions for something. At the moment they are included in the list
of functions to be compiled with the reachability analysis
(SimpleDCE) in the ir package, but eventually this reachability
analys will be removed. At that point, it would be very convenient
to be able to simply build a function with a new IR builder.
The `compilerContext` struct makes sure that it is not possible for
`builder` methods to accidentally use global state such as the global IR
builder. It is a transitional mechanism and may be removed when
finished.
aykevl
force-pushed
the
refactor-compiler
branch
from
0401f5c to
fc35d10
Compare
|
Finally, this PR is ready for review. It doesn't fix the entire public interface of the compiler package (it still exposes It is a very large PR but it is split into many relatively small commits. The last time I tested, all but one commit can be compiled and tested individually, making it easy to spot regressions if there are any. |
This commit unfortunately introduces a significant amount of code duplication. However, all that duplicate code should be removed once this refactor is done.
This is a fairly big commit, but it actually changes very little. getValue should really be a property of the builder (or frame), where the previously created instructions are kept.
Move to the builder object, and rename to createTypeAssert.
Now that most of the utility compiler methods are ported over to the builder or compilerContext, it is possible to avoid having to do the wrapper creation in two steps. A new builder is created just to create the wrapper. This is a small reduction in line count (and a significant reduction in complexity!), even though more documentation was added.
parseExpr (now createExpr) and all callers (recursively) are switched over to the new builder object!
A few functions were duplicated during the refactor. They can now be deleted.
This commit merges NewCompiler and Compile into one simplifying the external interface. More importantly, it does away with the entire Compiler object so the public API becomes a lot smaller. The refactor is not complete: eventually, the compiler should just compile a single package without trying to load it first (that should be done by the builder package).
aykevl
force-pushed
the
refactor-compiler
branch
from
fc35d10 to
b283224
Compare
|
I spent a while reviewing the changes, which indeed were quite numerous. That said, it was a true refactoring and not a rewrite, so although tedious to review, it was consistent. Thanks for that @aykevl I then turned my attentions to some empirical testing using my latest collection of demos plus a couple of test programs/boards I have been using lately. That also took a while, but all of the code/hardware functioned exactly as expected. Therefore, I am going to merge this PR. Hopefully it will help current and future contributors navigate the compiler code. |
Work-in-progress to refactor the compiler.
Not nearly finished, but sharing here for initial review to make sure this is the right direction. The main goal is to remove
Compiler.builder, which is almost like a global variable (shared throughout the entire compiler instance). Instead, an IR builder should be made per function. This will make the structure of the compiler a lot simpler and should avoid lots of jumping around, for example to create wrapper functions. The last time I tried to remove theSimpleDCEmechanism (a blocker for building packages independently), wrapper functions were a major pain and resulted in needing to add a worklist for functions to be built after the current one is finished.This change will be huge, so I decided to split it in many steps to make it reviewable. It could even be done in multiple PRs (if needed I could also split this PR up in multiple pieces). Most commits should be easy to review as the vast majority of the changes are mechanical (search+replace).
Some code is duplicated while the refactor is in progress. This duplication will be removed once it is finished.