You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/11/09 02:27:56 UTC

[GitHub] [incubator-tvm] kazum opened a new pull request #6886: [CI] Pin wasmtime version to 0.16.0

kazum opened a new pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886


   The wasm32 test [doesn't work](https://github.com/apache/incubator-tvm/issues/6816#issue-734113134) with the current wasmtime (after the commit https://github.com/bytecodealliance/wasmtime/pull/1565).  It seems that we cannot generate a wasm binary which is compatible with the new WASI ABI.
   
   This PR pins the wasmtime version to v0.16.0, the latest stable version which can work with our wasm32 test, and makes #6871 pass the CI.  This also installs libc6-dev-i386 needed for wasm32 compilation.
   
   @jroesch @tqchen @nhynes 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6886: [CI] Install libc6-dev-i386 to compile wasm32

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-729262365


   @jroesch still need to update the images to reflect the change


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6886: [CI] Install libc6-dev-i386 to compile wasm32

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-729310802


   Right 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6886: [CI] Pin wasmtime version to 0.16.0

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-726090951


   Thanks @kazum what you said makes sense. We could actually change the LLVM codegen. Under the systemlib mode, it is not necessary to export these two global variables and since they are registered via a startup function. While it is not good to keep them as InternalLinkage (we might still need to make them weak). 
   
   But we can skip the set dll export part of the codegen


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] jroesch commented on pull request #6886: [CI] Install libc6-dev-i386 to compile wasm32

Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-729257691


   Looks good to me, was on vacation for a few days, thanks for the fix!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen merged pull request #6886: [CI] Install libc6-dev-i386 to compile wasm32

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] tqchen commented on pull request #6886: [CI] Pin wasmtime version to 0.16.0

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-724026759


   @kazum it would be great if we can still generate wasm32 that is compatible with the new WASI ABI. Is it possible to look a bit into it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] kazum commented on pull request #6886: [CI] Pin wasmtime version to 0.16.0

Posted by GitBox <gi...@apache.org>.
kazum commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-726236977


   @tqchen Thanks, it makes sense to me :)  I'll send the change in #6871 later.
   
   Installing libc6-dev-i386 is necessary to compile wasm32 anyway.  Could you merge the change if it's okay?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] jroesch commented on pull request #6886: [CI] Install libc6-dev-i386 to compile wasm32

Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-729304036


   @tqchen these tests are only ran on CPU so I should only have to update that image right?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-tvm] kazum commented on pull request #6886: [CI] Pin wasmtime version to 0.16.0

Posted by GitBox <gi...@apache.org>.
kazum commented on pull request #6886:
URL: https://github.com/apache/incubator-tvm/pull/6886#issuecomment-725851854


   @tqchen There are two kinds of WASI binaries, command and reactor, and what we generate in the test is a command.  A WASI command may not export global variables, but we export two variables, `__tvm_module_ctx` and `__tvm_main__`.
   
   If I changed the llvm linkage type of [this line](https://github.com/apache/incubator-tvm/blob/main/src/target/llvm/codegen_cpu.cc#L229) and [this line](https://github.com/apache/incubator-tvm/blob/main/src/target/llvm/codegen_cpu.cc#L361) to InternalLinkage, I could produce a compatible wasm32 which is runnable with the latest wasmtime, but I think the change is not acceptable for other targets.
   
   Another fix I came up with was localizing those variables with llvm-objcopy after we generate the wasm library.  But the current llvm-objcopy has only initial support for wasm, so we cannot do it yet.
   
   I think it's a reasonable workaround to pin the wasmtime version until llvm-objcopy supports hiding global variables for wasm32. What do you think of it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org