-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
Implement WASI system calls #112
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good; just a couple of comments.
@@ -1,4 +1,5 @@ | |||
#!/usr/bin/env node | |||
#!/bin/sh | |||
// 2>/dev/null; exec /usr/bin/env node --experimental-wasm-bigint "$0" "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh. Basically, we need the experimental-wasm-bigint
flag for @wasmer/wasi to properly pass int64s to/from JS, but Linux machines don't allow you to pass more than one argument to a shebang.
https://unix.stackexchange.com/questions/65235/universal-node-js-shebang/65295#65295
This is temporary until it's not experimental anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, that makes sense. Maybe a comment to explain so that we don't forget?
compiler/src/codegen/compcore.ml
Outdated
@@ -129,6 +129,9 @@ let wrap_int64 n = add_dummy_loc (Values.I64Value.to_value n) | |||
let wrap_float32 n = add_dummy_loc (Values.F32Value.to_value n) | |||
let wrap_float64 n = add_dummy_loc (Values.F64Value.to_value n) | |||
|
|||
let grain_number_max = 0x3fffffff | |||
let grain_number_min = -0x3fffffff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to write this without the negative? Or at least a comment with the full hex of the negative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The native numbers in OCaml extend past 32 bits, so we can't write out the literal just here, but we could do Int32.to_int 0xC0000001l
if you'd like that better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Maybe just putting (* 0xC0000001 *)
next to it would be sufficient. I mostly just want it so that we can quickly identify what the memory layout of the number is, so it's not important that the actual source has the proper constant.
@@ -96,6 +108,8 @@ rule token = parse | |||
| comment { process_newlines lexbuf; token lexbuf } | |||
| blank { token lexbuf } | |||
| newline_chars { process_newlines lexbuf; EOL } | |||
| (signed_int as x) 'l' { INT32 (Int32.of_string x) } | |||
| (signed_int as x) 'L' { INT64 (Int64.of_string x) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's the inspiration for this syntax from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same syntax as C/C++/Java/OCaml. JavaScript uses the n
suffix for BigInt
but that doesn't feel quite right since other languages use n
for whatever the system's native int size is.
Some references:
https://docs.microsoft.com/en-us/cpp/c-language/c-floating-point-constants?view=vs-2019
https://en.wikiversity.org/wiki/Advanced_Java/Declaring_float_and_long_Literals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, cool. Looks good.
@@ -105,6 +119,7 @@ rule token = parse | |||
| "else" { ELSE } | |||
| "true" { TRUE } | |||
| "false" { FALSE } | |||
| "void" { VOID } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😨
This PR implements system calls for Grain using the WebAssembly System Interface. This includes things like accessing system clocks, reading environment variables, and reading/writing to files.
We use @wasmer/wasi to make WASI calls within Node.js. While writing this PR, Node.js 14 was released, which ships with built-in support for WASI, with a similar API. After this lands, I'll make an issue to use the built-in WASI implementation for our Node runtime, but we'll still need to rely on
@wasmer/wasi
for the browser runtime.New Modules
sys.gr
This module is meant to contain fairly thin wrappers around the WASI calls. It's not expected for users to interact with these directly, but rather with libraries built on top of these.
Environment
Clocks
Files
Process
Random
stdint64.gr
This module provides some basic operations over the Int64 type.
Updated Modules
lists.gr
Added
for_each
andfor_each_i
.arrays.gr
Added
map
.Improvements
Hex, Octal, and Binary literals
Visual Number Separators
Underscores are now allowed in number literals.
Int32 and Int64 Literals
Void constant
Bug Fixes
if
statements now workThe files foo and bar make inconsistent assumptions over interface baz
error while running tests no longer occurs. NOTE: This is not yet fully fixed, and could still happen when running compiling and running normal programs, though this will no longer happen while running the tests. A bug will be filed once this PR lands.Other Noteworthy Changes
To support running in WASI environments,
GRAIN$MAIN
has been renamed to_start
.memory
is now exported from all Grain modules under the namememory
.Everything Else
While a fair number of these new system calls will work as-is, many of them rely on some bugfixes I made that have yet to be released. As soon as a new release is cut, we'll upgrade to the proper version. In the meantime, say, to develop libraries on top of these calls, the user will need to manually build and link in wasmer-js.