SCP-3392 UPLC simplifier #4365
SCP-3392 UPLC simplifier #4365
Conversation
|
/benchmark plutus-benchmark:nofib |
|
The nofib benchmark is rebuilt every time, so it's a test of whether changes to the compiler make things faster. It would be nice to have size and budget outputs for them too, I'll think about adding those. |
|
Comparing benchmark results of '7a3e0a26d' (base) and 'b5e9d60e5' (PR)
|
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
b5e9d60 to
9a6e0d2
Compare
|
/benchmark plutus-benchmark:nofib |
|
Comparing benchmark results of '7a3e0a26d' (base) and '9a6e0d2ff' (PR)
|
|
Okay, this is looking very good in a lot of cases, including some of the size/budget tests we have. I think we should almost certainly do this. TODO:
|
|
I regenerated the |
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
|
Well that's extremely mysterious. The |
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
4666709 to
85b72f1
Compare
|
The mystery deepens: the nofib tests that got faster got (slightly) bigger (which I don't understand at all), and the ones that got slower got smaller! More digging required. |
|
Looks like the size bump is due to inlining some constructors. In particular, nullary constructors look like |
You can dump the compiled code using |
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
85b72f1 to
9e0b319
Compare
|
/benchmark plutus-benchmark:nofib |
|
Comparing benchmark results of 'plutus-benchmark:nofib' on 'a28a62d79' (base) and '9e0b319b3' (PR)
|
|
That's more like it. |
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
9e0b319 to
1ea2db1
Compare
|
Okay, I think this is ready for review now. Seems to be pretty good overall. The main sadness is that I basically had to duplicate the whole inliner, but I tried to keep the divergences to a minimum and label them, so hopefully it won't be too bad. I should probably write a few more tests before we merge, but I think the code is in a good state, and empirically it seems to work pretty well. |
| (force | ||
| (delay |
There was a problem hiding this comment.
Is it okay that forceDelayCancel doesn't happen in test/Lift tests?
There was a problem hiding this comment.
Yes: at the moment no optimization happens in Lift, this just makes that consistent. We probably should do some optimization in Lift, but that's a separate problem.
| defaultCompilationOpts :: CompilationOpts | ||
| defaultCompilationOpts = CompilationOpts True False False False 8 True True True False | ||
| defaultCompilationOpts :: CompilationOpts a | ||
| defaultCompilationOpts = CompilationOpts True False False False 12 True True True mempty False |
There was a problem hiding this comment.
Maybe extract _soMaxSimplifierIterations from defaultSimplifyOpts here?
There was a problem hiding this comment.
Mmm, but they're different. This is for the PIR compiler, the other is for the UPLC compiler, so (in principle) they shouldn't import each other. They end up being the same, but I think it's easier for now to just have them be quasi-independent. You could imagine wanting to tune them separately...
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
1ea2db1 to
3015edf
Compare
This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on #4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
michaelpj
force-pushed
the
mpj/uplc-inline
branch
2 times, most recently
from
532d790 to
4d17e7a
Compare
This adds essentially a copy of the PIR inliner for UPLC. This has pretty good results, with a 5-20% speedup and size decrease for many programs.
michaelpj
force-pushed
the
mpj/uplc-inline
branch
from
4d17e7a to
db20910
Compare
|
Hydra really doesn't like this PR, I'm going to open a new one. |
…ctMBO#4372) This has bothered me for a while. We rely on this to turn function applications into let-bindings so that we can inline them, but because of the way it was written it could only ever do this for at most one argument at a time! So we could only fully-inline a function of arity N with N rounds of the simplifier! Terrible! Now it handles any number at once. I wrote all the functions for this while working on IntersectMBO#4365, since there we *really* need it, since constructors and destructors appear like multiple function arguments rather than multiple nested immediately-applied-lambdas. But it applies well here too, so I thought I'd just do this improvement on the side.
Don't look here yet, just benchmarking something.