-
Notifications
You must be signed in to change notification settings - Fork 154
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
Various deriving bitrepr fixes #2209
Conversation
03420b0
to
28f62b8
Compare
Oh and btw unrelated to this issue. My data type was so large that it takes ages to compile. So I didn't notice this before. But do I need to do something in addition to using |
No, the |
d3c8607
to
a9870fa
Compare
a0f1a7a
to
6964085
Compare
This is ready for review and possible merge now. |
No, you shouldn't have to do anything. |
@leonschoorl I fixed that in this PR this morning, in addition with a few more things. |
Oh nice |
@leonschoorl That was one of the issues. So the updated top comment of this PR |
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.
Some of you commit (titles) are a bit long/wide, causing truncation in some tools, like github.
Specifically:
Clash.Annotations.BitRepresentation.Deriving.deriveAnnotation
no lo…
Just dropping the module name would help a lot.
clash-prelude/src/Clash/Annotations/BitRepresentation/Deriving.hs
Outdated
Show resolved
Hide resolved
clash-prelude/src/Clash/Annotations/BitRepresentation/Deriving.hs
Outdated
Show resolved
Hide resolved
6964085
to
2a2d1cf
Compare
Various deriving bitrepr fixes (copy #2209)
This PR fixes the following bit representation derivation issues:
1. Quadratic compilation time when deriving
The process of deriving
DataReprAnn
has quadratic complexity in the size of constructors and fields.For example this data type:
Going from 1 constructor to all 7 yields these compilation times on my machine:
The reason is that the TemplateHaskell in
deriveAnnotation
creates insanely large annotations. See this file where I dumbed the splice(-ddump-splices
) for the case with 4 constructors.This PR fixes this issue by introducing let-bindings of commonly used sub-expressions in the generated annotation. See here for the generated annotation after this PR.
A quick test shows these compilation times after this PR:
2. Promoted data cons and symbols
Promoted data cons and symbols weren't supported at all. Which lead to issues were you used promoted data constructed as a parameter to a type you wanted to derive a bitrepresentation for. Or if you wanted to for example use the domain somewhere in your data type.
3. Type synonyms weren't resolved
During the process of finding types annotation exist for there could be a mismatch because the types coming via GHC core had their type synonyms resolved. While the types coming in via the annotation hadn't. The result was that if you use a type synonym somewhere in the annotation it wouldn't be able to match it with any type in the Custom representation map. Now it always fully resolves that type during the Template Haskell phase at the annotation.