search

robert-strandh / SICL

A fresh implementation of Common Lisp
Other
1.07k stars 79 forks source link

Inline all closures #142

Closed karlosz closed 5 years ago

karlosz commented 5 years ago

Consider the code:

(loop for x from 0 to 5 collect (let ((i x)) (lambda () (setq i i) i))))

Right now, there is a trapper criterion preventing the function created by LET from being inlined. However, the inliner can inline the LET by creating an assignment from x into i. However, this introduces a bug, because x and i would share their values, which is incorrect. Therefore, this change makes it so that the inliner marks the created assignment as a special subclass of assignment-instruction, then fixes up these special subclasses at the end of the inlining pass by allocating cells for those variables which need them. The upshot is that functions can be inlined regardless of how their subfunctions escape.

Incidentally, while this change chiefly benefits generated code, there is a slight built time improvement for clasp.

karlosz commented 5 years ago
  1. Correct. I can add a comment about it.
  2. Let n be the number of functions to be inlined. It's okay to run reinitialize-instance a constant number of times (in this case once). It is not okay to run it in a loop which will run n times. In fact, discern-trappers was being run in the main loop, so the replacement of discern-trappers, an operation that in total ran O(n|G|) amount of time, by one use of reinitialize-data (O(|G|)) gives a favorable performance payout. Verified by empirical timing, as noted before, as well as flamegraphing.