diff options
author | Yuriy Glukhov <yglukhov@users.noreply.github.com> | 2024-07-03 22:49:30 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-03 22:49:30 +0200 |
commit | 05df263b84de9008266b3d53e2c28b009890ca61 (patch) | |
tree | 3616ac90794a37d4a7840136d29c36524a516dcd /tests/iter/tyieldintry.nim | |
parent | 051a536275c8851adec4742f90c518fbbb65d13c (diff) | |
download | Nim-05df263b84de9008266b3d53e2c28b009890ca61.tar.gz |
Optimize closure iterator locals (#23787)
This pr redefines the relation between lambda lifting and closureiter transformation. Key takeaways: - Lambdalifting now has less distinction between closureiters and regular closures. Namely instead of lifting _all_ closureiter variables, it lifts only those variables it would also lift for simple closure, i.e. those not owned by the closure. - It is now closureiter transformation's responsibility to lift all the locals that need lifting and are not lifted by lambdalifting. So now we lift only those locals that appear in more than one state. The rest remains on stack, yay! - Closureiter transformation always relies on the closure env param created by lambdalifting. Special care taken to make lambdalifting create it even in cases when it's "too early" to lift. - Environments created by lambdalifting will contain `:state` only for closureiters, whereas previously any closure env contained it. IMO this is a more reasonable approach as it simplifies not only lambdalifting, but transf too (e.g. freshVarsForClosureIters is now gone for good). I tried to organize the changes logically by commits, so it might be easier to review this on per commit basis. Some ugliness: - Adding lifting to closureiters transformation I had to repeat this matching of `return result = value` node. I tried to understand why it is needed, but that was just another rabbit hole, so I left it for another time. @Araq your input is welcome. - In the last commit I've reused currently undocumented `liftLocals` pragma for symbols so that closureiter transformation will forcefully lift those even if they don't require lifting otherwise. This is needed for [yasync](https://github.com/yglukhov/yasync) or else it will be very sad. Overall I'm quite happy with the results, I'm seeing some noticeable code size reductions in my projects. Heavy closureiter/async users, please give it a go.
Diffstat (limited to 'tests/iter/tyieldintry.nim')
-rw-r--r-- | tests/iter/tyieldintry.nim | 23 |
1 files changed, 23 insertions, 0 deletions
diff --git a/tests/iter/tyieldintry.nim b/tests/iter/tyieldintry.nim index 47ba6efa6..04409795b 100644 --- a/tests/iter/tyieldintry.nim +++ b/tests/iter/tyieldintry.nim @@ -504,3 +504,26 @@ block: # void iterator except: discard var a = it + +block: # Locals present in only 1 state should be on the stack + proc checkOnStack(a: pointer, shouldBeOnStack: bool) = + # Quick and dirty way to check if a points to stack + var dummy = 0 + let dummyAddr = addr dummy + let distance = abs(cast[int](dummyAddr) - cast[int](a)) + const requiredDistance = 300 + if shouldBeOnStack: + doAssert(distance <= requiredDistance, "a is not on stack, but should") + else: + doAssert(distance > requiredDistance, "a is on stack, but should not") + + iterator it(): int {.closure.} = + var a = 1 + var b = 2 + var c {.liftLocals.} = 3 + checkOnStack(addr a, true) + checkOnStack(addr b, false) + checkOnStack(addr c, false) + yield a + yield b + test(it, 1, 2) |