compiler: add support for 'go' on func values #487
Conversation
compiler/func.go
Outdated
| @@ -32,7 +32,7 @@ const ( | |||
| // funcImplementation picks an appropriate func value implementation for the | |||
| // target. | |||
| func (c *Compiler) funcImplementation() funcValueImplementation { | |||
| if c.GOARCH == "wasm" { | |||
| if c.GOARCH == "wasm" || true { | |||
There was a problem hiding this comment.
I do not understand this line of code. Seems like the else cannot be reached?
There was a problem hiding this comment.
Oh thank you! That's an oversight, will take a look.
There was a problem hiding this comment.
Thinking about how to fix this, because this change is necessary for this PR to work at all, because the old way (the "doubleword" implementation) cannot allow go on function pointers right now.
I see two options:
- Using the switch implementation everywhere (basically as this PR does, but in a cleaner way). No extra work is needed, binary sizes are sometimes increased slightly (within 1%). An additional quick benefit is that some limitations in the current goroutine implementation will be lifted as well, such as not being able to use function pointers of functions that do some async operation. This would also be fixed by option 2 but that is going to take longer.
- Implementing goroutines in a more traditional way. This requires an overhaul of the scheduler. This is a significant amount of work, and is not a portable solution: it won't work on WebAssembly (but will probably work significantly better on bare metal).
Perhaps the best way to proceed is to pick option 1 and later work towards revamping the scheduler on supported platforms.
There was a problem hiding this comment.
Agreed, option 1 is the best option at this point.
There was a problem hiding this comment.
Pinging to bring this back to top of queue, hopefully can be wrapped up soon.
There was a problem hiding this comment.
Well, the best option would indeed have been 1, but then I got excited and implemented the stack-based scheduler in option 2 anyway.
aykevl
force-pushed
the
implement-go-func-value
branch
3 times, most recently
from
ae71543 to
238a91d
Compare
aykevl
force-pushed
the
implement-go-func-value
branch
from
238a91d to
b8fb88c
Compare
|
This should now be ready for review/merge. |
This commit allows starting a new goroutine directly from a func value, not just when the static callee is known. This is necessary to support the whole time package, not just the commonly used subset that was compiled with the SimpleDCE pass enabled.
aykevl
force-pushed
the
implement-go-func-value
branch
from
b8fb88c to
5fdbd3f
Compare
|
Tested successfully with some of my more complex code. Works as expected now with new scheduler and go routines on functions. Merging.. |
This commit allows starting a new goroutine directly from a func value,
not just when the static callee is known.
This is necessary to support the whole time package, not just the
commonly used subset that was compiled with the SimpleDCE pass enabled.
This is the first commit of #466, splitting it off because it is useful in general and makes the PR somewhat smaller.