Skip to content

WASI: Initial Support - #1497

Merged
certik merged 26 commits into
lcompilers:mainfrom
ubaidsk:wasi_initial
Feb 10, 2023
Merged

certik merged 26 commits into
lcompilers:mainfrom
ubaidsk:wasi_initial

Conversation

@ubaidsk

@ubaidsk ubaidsk commented Feb 3, 2023 •

Copy link
Copy Markdown
Collaborator

This PR implements initial support of WASI in the wasm and wasm_x64 backends.

@ubaidsk

ubaidsk commented Feb 3, 2023 •

Copy link
Copy Markdown
Collaborator Author

This is currently a WIP (Work in progress).

At the moment, #1461 (comment) seems to work.

(lp) lpython$ cat examples/expr2.py 
from sys import exit
def main0():
    a: i32 = 67
    exit(a)

main0()
(lp) lpython$ lpython examples/expr2.py --backend wasm -o tmp
(lp) lpython$ wasmtime tmp
(lp) lpython$ echo $?
67
(lp) lpython$ 

Comment thread src/libasr/codegen/wasm_assembler.h Resolved
@certik

certik commented Feb 3, 2023

Copy link
Copy Markdown
Contributor

Beautiful. I think this approach will force us to "clean up" our code generation to make it "standardized".

@ubaidsk
ubaidsk force-pushed the wasi_initial branch 2 times, most recently from 565b13a to 2f4f4c6 Compare February 6, 2023 17:19
@ubaidsk

ubaidsk commented Feb 6, 2023

Copy link
Copy Markdown
Collaborator Author

Current Output:

(lp) lpython$ cat examples/expr2.py 
def main0():
    print("hi", "1234", "jldakfjs!jdlkasf#9u843*)(")
    print("abcdefgh1234")
    print(-1234)
    print(6579352)
    print("hello")

main0()
(lp) lpython$ lpython examples/expr2.py --backend wasm -o tmp
(lp) lpython$ wasmtime tmp
hi, 1234, jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-####
#######
hello
(lp) lpython$ 
  • String printing currently works
  • For integers, we are currently printing # instead of the exact digit.

Comment thread src/libasr/codegen/asr_to_wasm.cpp Outdated
avail_mem_loc += str.length();
no_of_data_segments++;

avail_mem_loc = (int(ceil(avail_mem_loc / 4.0)) << 2); /* align mem location to 4 bytes */

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Line 1812: It seems that fd_write works only when its iov vector (it stores the start location of the string and the size of the string) is aligned by 4. It doesn't seem to print any output when the location of the iov vector is not a multiple of 4.

We previously had memory aligned by single byte. It seems we might need the memory alignment as 4 bytes in future.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good point.

@ubaidsk

ubaidsk commented Feb 6, 2023

Copy link
Copy Markdown
Collaborator Author

doubts/concerns:

  1. Is the current approach the expected way to print integers?
  2. Is the current way easily maintainable?
    • It seems that in case of X86Assembler, we have a good abstraction, i.e. if we have required function instructions, we can use them and we need not dive into bytes much.
    • In case of wasm, it seems that, as a programmer, we are needing to touch bytes. For example: when implementing if-else, loop, the return instruction or for defining a whole new function.
    • To define a function in x86, it seems simpler as we add a label and start defining the function. In case of wasm, we are needing to define the signature of the function in the type section, then reference the signature in the function section, add the function code in code section and then export the function in export section. For each section, whenever we add a new element, we need to (without missing) increment the variable no_of_element corresponding to that section. Any minor change could lead to incorrect binary which could be tricky to debug.
    • I am unsure if the current approach is easily maintainable/extendable.

@ubaidsk

ubaidsk commented Feb 6, 2023

Copy link
Copy Markdown
Collaborator Author

Printing integers seems to work:

(lp) lpython$ cat examples/expr2.py               
def main0():
    print("hi", "1234", "jldakfjs!jdlkasf#9u843*)(")
    print("abcdefgh1234")
    print(-1234)
    print(6579352, -123, "hi", 45897, "bye", 0, -1, 1, -10, 10, "done.")
    print("hello")
    print(2, -2, 10, -10, 1000, -10000)

main0()
(lp) lpython$ lpython examples/expr2.py --backend wasm -o tmp
(lp) lpython$ wasmtime tmp                                   
hi, 1234, jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-1234
6579352, -123, hi, 45897, bye, 0, -1, 1, -10, 10, done.
hello
2, -2, 10, -10, 1000, -10000
(lp) lpython$ 

@certik

certik commented Feb 6, 2023 •

Copy link
Copy Markdown
Contributor

Great job.

I think this is maintainable for now. I think we might later need to add a class that handles the WASM as a format, and exposes the API using capabilities like "add a function" and it internally takes care of the details. That way one cannot make a mistake.

Regarding:

    void emit_if_else(std::function<void()> test_cond, std::function<void()> if_block, std::function<void()> else_block) {
        test_cond();
        wasm::emit_b8(m_code_section, m_al, 0x04);  // emit if start
        wasm::emit_b8(m_code_section, m_al, 0x40);  // empty block type
        nesting_level++;
        if_block();
        wasm::emit_b8(m_code_section, m_al, 0x05);  // starting of else
        else_block();
        nesting_level--;
        wasm::emit_expr_end(m_code_section, m_al);  // emit if end
    }

This can probably later be moved to the wasm namespace. If the nesting_level must be a state that we have to maintain (not clear to me currently), then we can create a WASMAssembler, that takes care of this. In ASR->WASM you just use the assembler.

Comment thread src/libasr/codegen/asr_to_wasm.cpp Resolved
Comment on lines +477 to +478
(data (;14;) (i32.const 184) "
")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

newline character (which is needed/printed by print after printing all arguments) is being shown as above in the generated WAT.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This has been fixed. The newline character is now escaped and is printed in its byte form (\0a) as follows:

(data (;3;) (i32.const 24) "\0a   ")

@ubaidsk

ubaidsk commented Feb 7, 2023

Copy link
Copy Markdown
Collaborator Author

This might still fail at the CI.

Currently:

  • All the wasm tests pass.
  • Support for wasm_x64 (and wasm_x86) is needed.

@certik

certik commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

This looks good, just the WASM->x64 needs to be fixed so that the CI passes and we can merge this.

Comment thread src/libasr/codegen/wasm_to_x64.cpp Resolved
@ubaidsk

ubaidsk commented Feb 8, 2023

Copy link
Copy Markdown
Collaborator Author

Now, printing partially works in wasm_x64 backend (string printing works, integer and float printing yet to be supported/fixed).

@certik

certik commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

Nice, great progress!

@ubaidsk

ubaidsk commented Feb 9, 2023

Copy link
Copy Markdown
Collaborator Author

It works now!

WASM:

Binary:

(lp) lpython$ lpython integration_tests/print_05.py --backend wasm -o tmp
(lp) lpython$ wasmtime tmp
hi 1234 jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-1234
6579352 -123 hi 45897 bye 0 -1 1 -10 10 done.
hello
2 -2 10 -10 1000 -10000
3.14000000
-1000.10000000
-1000.00099999
abcdefgh1234
0 0.000000000 abcd 0 efghijk 0.000000000 1^3 -1.000000000 pqrswxyz 1.000000000 mno 2.000000000 p -5.09999999 -5 zz
pi = 3.14000000

WAT Text:

(lp) lpython$ lpython integration_tests/print_05.py --show-wat > main.wat
(lp) lpython$ wasmtime main.wat
hi 1234 jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-1234
6579352 -123 hi 45897 bye 0 -1 1 -10 10 done.
hello
2 -2 10 -10 1000 -10000
3.14000000
-1000.10000000
-1000.00099999
abcdefgh1234
0 0.000000000 abcd 0 efghijk 0.000000000 1^3 -1.000000000 pqrswxyz 1.000000000 mno 2.000000000 p -5.09999999 -5 zz
pi = 3.14000000

ASM:

Binary:

(wasm_asm) lpython$ lpython integration_tests/print_05.py --backend wasm_x64 -o tmp > main.asm
(wasm_asm) lpython$ ./tmp
hi 1234 jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-1234
6579352 -123 hi 45897 bye 0 -1 1 -10 10 done.
hello
2 -2 10 -10 1000 -10000
3.14000000
-1000.10000000
-1000.00099999
abcdefgh1234
0 0.000000000 abcd 0 efghijk 0.000000000 1^3 -1.000000000 pqrswxyz 1.000000000 mno 2.000000000 p -5.09999999 -5 zz
pi = 3.14000000

ASM Text:

(wasm_asm) lpython$ nasm -fbin main.asm && chmod +x main && ./main
hi 1234 jldakfjs!jdlkasf#9u843*)(
abcdefgh1234
-1234
6579352 -123 hi 45897 bye 0 -1 1 -10 10 done.
hello
2 -2 10 -10 1000 -10000
3.14000000
-1000.10000000
-1000.00099999
abcdefgh1234
0 0.000000000 abcd 0 efghijk 0.000000000 1^3 -1.000000000 pqrswxyz 1.000000000 mno 2.000000000 p -5.09999999 -5 zz
pi = 3.14000000

@ubaidsk

ubaidsk commented Feb 9, 2023

Copy link
Copy Markdown
Collaborator Author

I disabled testing for the wasm_x86 backend in CI: Disable testing of WASM_X86 backend. Our current wasm-wasi implementation uses 64-bit integers (at some places) and the wasm_x86 backend supports only 32-bit integers.

@ubaidsk

ubaidsk commented Feb 9, 2023

Copy link
Copy Markdown
Collaborator Author

The CI seems to pass. I will structure the commits and push here.

@ubaidsk
ubaidsk requested a review from certik February 9, 2023 20:02
@certik

certik commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

I reviewed all the changes I think this is beautiful, everything seems to work.

Is the wasm_x86 completely broken, or do some tests work? I would keep the backend running, but maybe disable the particular tests that do not work.

Will it be difficult to update the online GUI to work with these changes?

@ubaidsk

ubaidsk commented Feb 9, 2023 •

Copy link
Copy Markdown
Collaborator Author

Is the wasm_x86 completely broken, or do some tests work?

wasm always includes the print_int/print_i64 function (for printing to stdout using fd_write()) which uses variables of type i64 which is not supported in the wasm_x86 backend.

PS: If the wasm_x86 is needed, I think we can get it working (probably with some hacks). For example, it can skip defining the print_i64 supplied by wasm and instead use its own print_i32 for printing integers.

emit_print_int(m_a, "print_i32");

@ubaidsk

ubaidsk commented Feb 9, 2023 •

Copy link
Copy Markdown
Collaborator Author

Will it be difficult to update the online GUI to work with these changes?

It could present some challenges (and would thus be a great opportunity to learn and grow). We need to see if there are any WASI polyfills available for browser (https://github.com/bjorn3/browser_wasi_shim seems to be one).

I am currently concerned about making the wasm and wasm_x64 backends more robust. Especially, simplifying the wasm backend and abstracting away the details. For example: When someone adds a new type, he/she should not need to increment/update the variable no_of_types. Another concern is related to accessing memory in assembly. The linear memory in wasm is editable (we can store values in it). We need to see if there is such memory (apart from the execution stack) available in assembly.

@certik

certik commented Feb 10, 2023 •

Copy link
Copy Markdown
Contributor

I would get wasm_x86 working, not fully, but in some limited capacity, so that we can keep testing it and ensuring it works (in its limited capacity). The way I would do it is implement WASM 64bit integers in wasm_x86 (you have to use pair of registers or operate on memory in pairs of 32bit operations), then you can just use the existing WASM code. That can be done later. For now, why don't you get some simple tests that only print strings (but not numbers) working again? That way we have at least something. Or if that is too complicated, then at least returning zero / nonzero return value. Then we can merge this PR and implement the rest later.

Regarding the web UI, I would implement the functions that we use ourselves. That way the implementation will be small and clean.

Regarding accessing memory that you can modify in x64 assembly: I think it's called "heap", and you have to request the memory from the system. In C you would call malloc, say 1GB, and then you get a pointer to the beginning and manage the memory yourself just like you would in webassembly. Internally, I think malloc calls the sbrk system (linux) call. In fact, I think Linux might give you some memory to start with, I think it's called data segment. I think when you create the binary, you can create this data segment and then use it for the read/write memory, so you probably don't even need to call sbrk to grow it.

@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

I would get wasm_x86 working, not fully, but in some limited capacity, so that we can keep testing it and ensuring it works (in its limited capacity). The way I would do it is implement WASM 64bit integers in wasm_x86 (you have to use pair of registers or operate on memory in pairs of 32bit operations), then you can just use the existing WASM code. That can be done later.

Sure, Sir. Using pairs of 32-bit registers/instructions is a great idea.

For now, why don't you get some simple tests that only print strings (but not numbers) working again? That way we have at least something. Or if that is too complicated, then at least returning zero / nonzero return value. Then we can merge this PR and implement the rest later.

Got it.

Thank you for the guidance.

@ubaidsk

ubaidsk commented Feb 10, 2023 •

Copy link
Copy Markdown
Collaborator Author

If it is fine, shall we tackle the WebUI and the assembly (heap) memory later?

Comment thread src/libasr/codegen/wasm_to_x86.cpp Resolved
Comment thread src/libasr/codegen/wasm_to_x86.cpp Resolved
@certik

certik commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

I think this looks good, let's see if tests pass.

@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

This PR is ready.

I think this looks good, let's see if tests pass.

Sure.

@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

The integration_tests for wasm_x86 seem to pass on my system locally.

There is an integer overflow when printing the value of w in the following.

w: i32 = -2147483648
print(x)
print(y)
print(z)
print(w)

On my system locally, it just prints the - symbol but does not fail/terminate with an error. I guess it could be failing at the CI.

(lp) lpython$ lpython integration_tests/print_03.py --backend wasm_x86 -o tmp
(lp) lpython$ ./tmp
-25
100
2147483647
-
Hi
Hello
21
Bye

@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

I am sorry about #1497 (comment). I see it was the x86 backend (instead of wasm_x86) that was failing.

What shall we do about TEST: Slightly modify test to support it in wasm_x86?

@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

This is ready.

@certik certik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is good to go in!

I reviewed most of it, I don't see any major issue now. Great job, this is excellent progress.

@certik
certik merged commit a868f5f into lcompilers:main Feb 10, 2023
@ubaidsk
ubaidsk deleted the wasi_initial branch February 10, 2023 22:08
@ubaidsk

ubaidsk commented Feb 10, 2023

Copy link
Copy Markdown
Collaborator Author

Thank you for the approval and merging!

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