summary refs log tree commit diff stats
path: root/tests/iter/tyieldintry.nim
diff options
context:
space:
mode:
authorYuriy Glukhov <yglukhov@users.noreply.github.com>2024-07-03 22:49:30 +0200
committerGitHub <noreply@github.com>2024-07-03 22:49:30 +0200
commit05df263b84de9008266b3d53e2c28b009890ca61 (patch)
tree3616ac90794a37d4a7840136d29c36524a516dcd /tests/iter/tyieldintry.nim
parent051a536275c8851adec4742f90c518fbbb65d13c (diff)
downloadNim-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.nim23
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)