MLton backend #156
MLton backend #156
Conversation
b-studios
force-pushed
the
feature/mlton
branch
from
7a798fb
to
4d578ae
Compare
…dering definitions
|
@JonathanStarup Is there anything that you planned to do before we can merge this PR? |
|
Nothing except some cleaup, the changes to effekt.sh and the build should be reverted right? |
I have to look into this, maybe for now. We still have the open ticket on how to support, unix, and macos at the same time with the same convenience. |
|
|
||
| def name(id: Symbol): MLName = MLName(uniqueName(id)) | ||
|
|
||
| def name(id: Name): MLName = MLName(id.name) |
There was a problem hiding this comment.
Maybe move those into a companion object on MLName? Then we do not need this file?
| // lift inference infers two lifts here, which is not necessary! | ||
| // but how can we tell functions like `triple` apart from capabilities? | ||
| // what if a local function closes over some capabilities and receives a few others? | ||
| // what if that function is used under a handler? |
There was a problem hiding this comment.
Is this still the case? Maybe we can describe this a bit more precisely in a ticket and drop the comment here?
|
|
||
| def trace(pos: Point, dir: Vector, scene: Scene, depth: Int): ColorV = { | ||
|
|
||
| def surface(pos: Point, dir: Vector, scene: Scene, depth: Int, intersection: Intersection): ColorV = { |
There was a problem hiding this comment.
Compared to the original raytracer example in examples/pos I had to manually reorder (and partially re-nest) the function definitions in order to be sorted topologically. @JonathanStarup Could you maybe open up a ticket so that we remember implementing this as a preprocessing step before emitting ML?
There was a problem hiding this comment.
On 640x480, the example takes around 200ms on my machine:
Benchmark 1: ./out/raytracer 640 480
Time (mean ± σ): 207.7 ms ± 0.8 ms [User: 111.2 ms, System: 95.9 ms]
Range (min … max): 205.9 ms … 209.8 ms 30 runs
There was a problem hiding this comment.
I also rewrite the List stdlib to use tailrecursive functions instead of local mutable state since we do not know how fast the latter is in ML at the moment.
Had to solve a lot of conflicts, so this merge commit contains semantic content! Conflicts: effekt/shared/src/main/scala/effekt/generator/chez/ChezSchemeLift.scala effekt/shared/src/main/scala/effekt/lifted/LiftInference.scala examples/neg/scoped.effekt
|
@JonathanStarup Can you tell me whether you can think of something that blocks the merge? I would like to click merge tomorrow, so that we can maintain the backend in all subsequent PRs :) It is not necessary that everything works; it is more important that all tests pass (which is the case) and no impediments for other backends or contributors are created. |
This is a draft PR for discussion and tracking the changes on this branch :)
TODO
Limitations #185