Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rewrite goroutines, add support for channels #130

Merged
merged 6 commits into from
Jan 21, 2019
Merged

Rewrite goroutines, add support for channels #130

merged 6 commits into from
Jan 21, 2019

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented Jan 11, 2019

This PR increases the amount of code, but makes goroutine lowering more independent of the compiler which makes it somewhat easier to maintain.

@aykevl aykevl force-pushed the goroutine-pass branch 2 times, most recently from eec5740 to 61b376e Compare January 11, 2019 22:29
@aykevl
Copy link
Member Author

aykevl commented Jan 12, 2019

Note: I've found at least one bug, so this PR is not ready yet.

@aykevl aykevl force-pushed the goroutine-pass branch 3 times, most recently from 750cab8 to a8acd59 Compare January 13, 2019 16:08
@aykevl aykevl changed the title Split coroutine creation into a separate pass Rewrite goroutines, add support for channels Jan 13, 2019
@aykevl aykevl force-pushed the goroutine-pass branch 4 times, most recently from d0f2648 to 9a231cf Compare January 13, 2019 18:16
@aykevl
Copy link
Member Author

aykevl commented Jan 13, 2019

I think it is now ready for merging. The bug I mentioned should be fixed, and wasm output that doesn't need a scheduler is the same size as before.

@aykevl
Copy link
Member Author

aykevl commented Jan 13, 2019

The bug wasn't entirely fixed :( need to work on that a bit more.
Basically, splitting a basic block is difficult, because phi nodes need to be kept consistent. LLVM has helper functions for this, but of course those aren't exposed in the C/Go bindings.

I measured the channel implementation with a quick benchmark and it's only 1.5x slower than with the main Go compiler. I think that's great. It may be possible to improve it even more if needed.

This was referenced Jan 14, 2019
Closed
Closed
@deadprogram
Copy link
Member

Please let me know when this feature is ready for testing after whatever further changes are needed.

@aykevl aykevl force-pushed the goroutine-pass branch 2 times, most recently from b6ff24b to b00009f Compare January 18, 2019 16:15
@aykevl
Copy link
Member Author

aykevl commented Jan 18, 2019

I think I finally figured out the bug and fixed it, so this PR is ready for review.

@aykevl aykevl mentioned this pull request Jan 21, 2019
Merged
@aykevl
Copy link
Member Author

aykevl commented Jan 21, 2019

@deadprogram can you maybe take a look?

@deadprogram
Copy link
Member

I was unable to build this version:

$ make tgo
go build -o build/tgo -i .
# github.com/aykevl/tinygo/compiler
compiler/llvm.go:46:7: inst.RemoveFromParentAsInstruction undefined (type llvm.Value has no field or method RemoveFromParentAsInstruction)
Makefile:100: recipe for target 'build/tgo' failed
make: *** [build/tgo] Error 2

Is there some updated dependency I am missing?

@aykevl
Copy link
Member Author

aykevl commented Jan 21, 2019

Yes, go-llvm has been updated to add this function.

@deadprogram
Copy link
Member

Tested with the blinky2 on the Microbit, and it runs as expected.

Tested on the SAMD21 and it locks each Go routine on the time.Sleep() call. Then after some investigation that is also the case on the current master branch for SAMD21. I will open a separate issue for that.

@aykevl
all: rewrite goroutine lowering
602c264 
Before this commit, goroutine support was spread through the compiler.
This commit changes this support, so that the compiler itself only
generates simple intrinsics and leaves the real support to a compiler
pass that runs as one of the TinyGo-specific optimization passes.

The biggest change, that was done together with the rewrite, was support
for goroutines in WebAssembly for JavaScript. The challenge in
JavaScript is that in general no blocking operations are allowed, which
means that programs that call time.Sleep() but do not start goroutines
also have to be scheduled by the scheduler.
@aykevl
compiler: add support for channel operations
2e4dd09 
Support for channels is not complete. The following pieces are missing:

  * Channels with values bigger than int. An int in TinyGo can always
    contain at least a pointer, so pointers are okay to send.
  * Buffered channels.
  * The select statement.
@aykevl
Copy link
Member Author

aykevl commented Jan 21, 2019 •
edited

I needed to apply a small change to runtime_atsamd21.go that may have been the cause. Let's see what Travis CI thinks now.

@aykevl aykevl merged commit 2e4dd09 into master Jan 21, 2019
@aykevl aykevl deleted the goroutine-pass branch January 21, 2019 22:25
@aykevl
Copy link
Member Author

aykevl commented Jan 21, 2019

I guess that means it can be merged?
It is now merged with the runtime_atsamd21.go change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deadprogram deadprogram Awaiting requested review from deadprogram

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aykevl @deadprogram