-
Notifications
You must be signed in to change notification settings - Fork 731
feat: unbox inductives in the compiler #11873
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
base: master
Are you sure you want to change the base?
Conversation
yes, I'm aware this is an ugly fix just you wait until I introduce the struct RC pass which must not happen before the interpreter runs the code (a compiled only pass!) anyways I hope there are no memory leaks
|
Great, seems to work! @hargoniX, could you !bench this? |
|
Mathlib CI status (docs):
|
|
Reference manual CI status:
|
|
!bench |
|
Benchmark results for 7d9cc32 against 1ca4faa are in! @Rob23oba Runs (1🟥)
Large changes (1🟥)
Medium changes (21✅)
Small changes (2049✅, 26🟥)
|
|
That's a nice first result. Though there does seem to be some (probabilistic?) segfault in the microbenchmarks here? I'm generally a little worried, given that most applications of I think there's also a couple of interesting open questions in the design space that are not obvious:
|
e11f2d7 to
0e86c81
Compare
…lean4 into compiler-unbox-test
…piler-unbox-test
0e86c81 to
14717c3
Compare
14717c3 to
2634c96
Compare
2634c96 to
a9068e3
Compare
a9068e3 to
d1af1a8
Compare
|
!bench |
|
Benchmark results for 852e108 against 11e4e44 are in! @Rob23oba Large changes (74✅, 4🟥)Too many entries to display here. View the full report on radar instead. Medium changes (240✅, 2🟥)Too many entries to display here. View the full report on radar instead. Small changes (1847✅, 28🟥)Too many entries to display here. View the full report on radar instead. |
852e108 to
f2da84f
Compare
|
!bench mathlib |
|
Benchmark results for leanprover-community/mathlib4-nightly-testing@cf601f6 against leanprover-community/mathlib4-nightly-testing@c31e3a7 are in! @Rob23oba No significant changes detected. |
|
@Garmelon "No significant changes detected." feels weird considering almost every file has roughly -6% in instructions |
|
Yeah, it's not great. Maybe I can re-use mathlib thresholds (based on historical data) for nightly-testing. More detailsRadar uses thresholds based on historical data, and nightly-testing's linearized history consists of all commits tagged If I use mathlib's thresholds, then I'll need to turn off (or at least scale by some factor) significance detection for the history, else most commits will turn out to be significant. |
f2da84f to
cad66e6
Compare
This PR implements the
@[unbox]attribute: In compiled code, values from inductives tagged@[unbox]are stored asstructs on the stack rather than the heap.This takes a bit of different approach than previously outlined in the documentation, allowing unboxed structs to occur anywhere (including variables and parameters and not just return values). To make this work, the existing infrastructure for boxing/unboxing scalar types has been extended to work for
structtypes. Notable changes:IRType.structnow takesobjs : Array IRTypeandusize ssize : Natinstead of justArray IRType.objsis meant to mirror the projections from the regular constructor layout andusizeandssizecorrespond to the same fields in the constructor layoutIRType.uniontakes an array oftypes : Array IRTypethat now should all beIRType.structsExpr.unboxalso works forstruct/unionExpr.boxnow has two roles: boxing scalars and structs and "reshaping" structs (that is, translating between two unboxed struct/union types)FnBody.inc/FnBody.decon an unboxed struct corresponds to incrementing/decrementing every field.Expr.proj,Expr.uproj,Expr.sproj,Expr.usetandExpr.ssetnow take acidx : Natparameter to indicate which part of the union should be used.projandctor