-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implement top-down pass for multiple specialization #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the bug revealed in test-cases/final-dump/multi_specz_cyclic_lib2.exp, and the minor updates I've suggested, this looks good.
src/Builder.hs
Outdated
-- | Run a top-down pass starting form the given module. | ||
-- It will find all required specialized versions and generate them. | ||
-- It also calls "blockTransformModule" to transform LPVM code to LLVM code. | ||
-- TODO: handle stdlib such as "wybe". It's read-only so we can't fill in specz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just stdlib, but any module for which you can't rewrite the .o file. In fact, since we have to handle this anyway, it's probably a good idea to only revise .o files in the current directory, and handle any object files in a different directory the same way we handle stdlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated todo.
-- collecting all required spec versions | ||
fixpointProcessSCC expandRequiredSpeczVersions ms | ||
|
||
-- generating required specz versions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use a TODO comment here (or maybe somewhere else?) saying that code is needed to select which specialisations to generate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in expandRequiredSpeczVersions
in Transform.hs
.
sbody' <- transformBody caller body (aliasMap, Map.empty) | ||
return (id, Just sbody') | ||
) (Map.toAscList speczBodies) | ||
let speczBodies' = Map.fromDistinctAscList speczBodiesList |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're going to want to run (or re-run) optimisations on the generated specialisations. I'm not sure if this is the right place for it, but it needs to be done somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added TODO in transformBody
in Transform.hs
. Even though we treat the standard and specialized versions a bit different, the "re-run optimization" strategy should be applied on both.
@@ -0,0 +1,2 @@ | |||
src/Expansion.hs:199:13-54: Irrefutable pattern failed for pattern ProcDefPrim proto body _ _ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that doesn't look good! That's a bug somewhere!
This PR is a part of #31. It includes the implementation of the top-down pass for multiple specialization.
In the old bottom-up pass, the compiler only generates the standard version of each proc and doesn't transform lpvm to llvm now.
It the new top-down pass, the compiler collects all required specialized versions and generates them, then transform lpvm to llvm.
Currently, we generates all needed specialized versions.
Some steps related to incremental build are disabled for now, but it shouldn't be too hard to be re-enabled. Around Builder.hs:946 and Builder.hs:980.
This PR also includes a small fix to the testing script.
test-cases/final-dump/multi_specz_cyclic_lib2.exp
has a weird output (it doesn't affect the whole test though). It outputs the same in master branch so I don't think it's related to this PR.