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.

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

support WASI target #1373

Merged
merged 15 commits into from
Sep 29, 2020
Merged

support WASI target #1373

merged 15 commits into from
Sep 29, 2020

Conversation

mathetake
Copy link
Contributor

@mathetake mathetake commented Sep 12, 2020

This PR adds wasi target

Screenshot from 2020-09-23 10-28-29

Changes in this PR

  • add targets/wasi.json
  • implement runtime.ticks
  • implement runtime.sleepTicks
  • add the emulator/test

Future TODOs

  • add WASI systemcall tests in tests/wasi/*
  • documentation
  • support file system related syscalls

I will create an issue for tracking these TODOs after merging this PR


Motivation

we in the Envoyproxy community are working on proxy-wasm project. The objective here is to provide web developers with the way to write Envoy extensions through WASM <-> Proxy ABI with their own favorite languages.

The host environment is not Go ABI (as in wasm_exec.js) at all and is kind of the extension of WASI, and therefore we would like to produce WASI compatible wasm binaries with TinyGo. We can compile proxy-wasm ABI compatible WASM binaries with the current TinyGo but we cannot import almost all of existing libraries due to the syscall/js centric syscall implementation. That is too bad for developer experience with proxy-wasm extensions.

Consideration

I know there are ongoing WASI issues golang/go#31105 and golang/go#38248 in the official compiler, but I think it's going to take multiple months (even years) for them to be completed. Once they're completed, I will do the integration with TinyGo for consistency and remove TinyGo specific wasi syscall implementation.


This is not completed yet but I would like to have opinions and direction from TinyGo maintainers before I work on the detail. Now ready for review 😄

P.S. Thanks for this awesome project! Without TinyGo, there would be no potential for Go developers to write proxy-wasm extensions.

@mathetake mathetake changed the title WASI non js-centric wasi target support WASI target Sep 12, 2020
@mathetake
Copy link
Contributor Author

@deadprogram Could you please take a look at? If there's no concern, I am going to work on the detail.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I wanted to wait until upstream Go supports WASI. Unfortunately, that doesn't seem to be happening (at least not in the near future) so it does make sense to add support in TinyGo independent of upstream support.

This PR contains a lot of code duplication. While some duplication might be necessary, it would be best to avoid as much as possible. Also, I'm missing a targets/wasi.json to be able to use this.
(Yes I know that this PR is a draft and needs more work, just wanted to point out a few things that will be needed for review).

src/runtime/runtime_wasi.go Outdated Show resolved Hide resolved
src/syscall/syscall_wasi.go Outdated Show resolved Hide resolved
@mathetake
Copy link
Contributor Author

mathetake commented Sep 17, 2020

time.Now has become available

Screenshot from 2020-09-17 21-47-18

@mathetake
Copy link
Contributor Author

mathetake commented Sep 19, 2020

runtime.sleepTicks is now working with poll_oneoff API:)

Screenshot from 2020-09-19 11-09-28

@mathetake mathetake marked this pull request as ready for review September 19, 2020 06:06
@mathetake
Copy link
Contributor Author

@deadprogram @aykevl I've made this PR ready for review so PTAL when you have time! 😄

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a good start, the main problem is that WASI support here is trying to also include JS/WebAssembly build tags. This won't work, as this is not supported by the standard library. Therefore, there are two options:

  • Match js/wasm support exactly, including support for the syscall/js package. I do not think this is a good path to take, as the things that work for JS don't work so well for WASI.
  • Pretend to be something else. This is the path that baremetal support in TinyGo takes: it pretends to be GOOS=linux GOARCH=arm even on other architectures such as AVR and RISC-V. While this may seem odd, it means that we can use the existing standard library with very few patches (mainly the syscall package).

transform/gc.go Outdated Show resolved Hide resolved
transform/gc.go Outdated Show resolved Hide resolved
targets/wasi.json Outdated Show resolved Hide resolved
@mathetake
Copy link
Contributor Author

it pretends to be GOOS=linux GOARCH=arm

OK, that makes sense. I'll follow this path and refactor the code 👍

@mathetake
Copy link
Contributor Author

mathetake commented Sep 23, 2020

@aykevl Thanks you for kind review. I followed your comments and made the following changes:

  • use GOARCH=arch and GOOS=linux instead of wasm/js
  • set InternalLinkage instead of HiddenVisibility, and enabled this by default

Indeed, these changes reduced the complexity of this PR! Please take another look 😄 Thanks!

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!
While the GOOS=linux GOARCH=arm is unfortunate, I really don't see a better way to integrate this in a sane way. So let's hope upstream Go eventually adds wasi support natively that we can rely on.

@aykevl aykevl changed the base branch from release to dev September 29, 2020 13:11
@mathetake
Copy link
Contributor Author

mathetake commented Sep 29, 2020

give me a sec to resolve conflict:)

loader/loader.go Outdated Show resolved Hide resolved
* merge "time" package with wasi build tag
* override syscall package with wasi build tag
* create runtime_wasm_{js,wasi}.go files
* create syscall_wasi.go file
* create time/zoneinfo_wasi.go file as the replacement of zoneinfo_js.go
* add targets/wasi.json target
Accodring to the WASI docs (https://github.com/WebAssembly/WASI/blob/master/design/application-abi.md#current-unstable-abi),
none of exports of WASI executable(Command) should no be accessed.

v0.19.0 of bytecodealliance/wasmetime, which is often refered to as the reference implementation of WASI,
does not accept any exports except functions and the only limited variables like "table", "memory".
Signed-off-by: mathetake <takeshi@tetrate.io>
Signed-off-by: mathetake <takeshi@tetrate.io>
Signed-off-by: mathetake <takeshi@tetrate.io>
Signed-off-by: mathetake <takeshi@tetrate.io>
@aykevl aykevl merged commit f50ad35 into tinygo-org:dev Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants