Skip to content
New issue

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

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

Already on GitHub? # to your account

CMake: Add option to build with WAVM #266

Merged
merged 4 commits into from
Jul 31, 2018
Merged

CMake: Add option to build with WAVM #266

merged 4 commits into from
Jul 31, 2018

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Jul 24, 2018

This adds -DHERA_WAVM option what for now will build WAVM and link hera with it. Because Hera does not uses WAVM it will not do anything useful, but we can check if the build works.

Also the HERA_WAVM=1 macro is predefined for build with WAVM.

This depends on LLVM 6.0 so you have to install libllvm-6.0-dev. See http://apt.llvm.org.

The WAVM produces a number of libraries. In this PR only the libruntime is used, probably more is going to be needed when we start using WAVM.

Part of #158.

@chfast chfast requested review from axic and jakelang July 24, 2018 14:32
@chfast
Copy link
Collaborator Author

chfast commented Jul 24, 2018

I think I should test the build with WAVM on any CI servers.

@jakelang
Copy link
Member

jakelang commented Jul 24, 2018

I think there is an existing option in the abstraction layer PR called -DWAVM_SUPPORTED. I will adjust to use this one instead.


if(HERA_WAVM)
target_compile_definitions(hera PRIVATE HERA_WAVM=1)
target_link_libraries(hera PRIVATE wavm::runtime)
Copy link
Member

@jakelang jakelang Jul 24, 2018

Choose a reason for hiding this comment

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

In addition to the Runtime lib (and the dependencies pulled in implicitly) we also need the WASM library to convert the bytecode into WAVM's internal IR.

@chfast
Copy link
Collaborator Author

chfast commented Jul 26, 2018

Ready to merge?

@jakelang
Copy link
Member

If the WAVM include root is added to Hera's include search list then yeah, I think it is ready.

@axic
Copy link
Member

axic commented Jul 29, 2018

Added a sample code to pull in all the required bits for loading & running (in dc50760) and it seems to compile.

@axic
Copy link
Member

axic commented Jul 29, 2018

My bad, used the wrong ifdef, however now it does complain:

In file included from /home/builder/project/src/hera.cpp:41:
In file included from deps/src/wavm/Include/IR/Module.h:3:
In file included from deps/src/wavm/Include/Inline/Assert.h:3:
deps/src/wavm/Include/Platform/Platform.h:89:2: error: unknown type name 'DLL_IMPORT'
        PLATFORM_API Uptr getPageSizeLog2();
        ^
deps/src/wavm/Include/Platform/Platform.h:31:23: note: expanded from macro 'PLATFORM_API'
        #define PLATFORM_API DLL_IMPORT
                             ^
deps/src/wavm/Include/Platform/Platform.h:89:19: error: expected ';' after top level declarator
        PLATFORM_API Uptr getPageSizeLog2();
                         ^

Resolution: manually added #define DLL_IMPORT before the includes, but not sure if there's a cmake solution for this.

@axic
Copy link
Member

axic commented Jul 29, 2018

@chfast So it seems the sample code compiles and links, but there is some kind of runtime linking error:

testeth 1.4.0.dev0
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::system::system_error> >'
  what():  boost::dll::shared_library::load() failed (dlerror system message: /home/builder/build/src/libhera.so: undefined symbol: _ZN4llvm17JITSymbolResolver6anchorEv): Bad file descriptor
Received 'aborted' signal

which resolves to llvm::JITSymbolResolver::anchor()

@axic
Copy link
Member

axic commented Jul 29, 2018

Lastly it fails to build if not built as shared library.

@axic
Copy link
Member

axic commented Jul 31, 2018

This seems to be working on dynamic loading now (tests fail because the code will throw intentionally).

Static linking however doesn't work because the flag -Werror=maybe-uninitialized is passed down from aleth.

@chfast
Copy link
Collaborator Author

chfast commented Jul 31, 2018

It should build with GCC 8 now.

@axic
Copy link
Member

axic commented Jul 31, 2018

The new sed doesn't work on mac:

[ 10%] Performing patch step for 'wavm'
sed: 1: "CMakeLists.txt": invalid command code C

@axic
Copy link
Member

axic commented Jul 31, 2018

It still fails on mac:

[ 10%] Performing patch step for 'wavm'
sed: 1: "CMakeLists.txt": invalid command code C
sed: 1: "CMakeLists.txt": invalid command code C

@axic
Copy link
Member

axic commented Jul 31, 2018

Moving them back to -iE fixes it.

@chfast
Copy link
Collaborator Author

chfast commented Jul 31, 2018

sed is still failing now?

@axic
Copy link
Member

axic commented Jul 31, 2018

Yes, changing your last commit to sed -iE makes everything to work.

@chfast
Copy link
Collaborator Author

chfast commented Jul 31, 2018

But I removed regex from sed.

@axic
Copy link
Member

axic commented Jul 31, 2018

sed on Mac is kind of weird

@axic
Copy link
Member

axic commented Jul 31, 2018

On Mac, though -iE workarounds that by somehow making it to ignore the extension parameter:

     -i extension
             Edit files in-place, saving backups with the specified extension.  If a zero-length extension is given, no backup will be saved.  It is not recommended to give a zero-
             length extension when in-place editing files, as you risk corruption or partial content in situations where disk space is exhausted, etc.

@chfast
Copy link
Collaborator Author

chfast commented Jul 31, 2018

I will replace it with CMake script not to have problems with it on Windows.

@axic
Copy link
Member

axic commented Jul 31, 2018

I will replace it with CMake script not to have problems with it on Windows.

I think I will merge this broken sed -iE version so we can get going and you can fix it in a new PR.

chfast and others added 4 commits July 31, 2018 18:37
This adds -DHERA_WAVM option what for now will build WAVM and link hera with it. Because Hera does not uses WAVM it will not do anything useful, but we can check if the build works.

Also the `HERA_WAVM=1` macro is predefined for build with WAVM.

This depends on LLVM 6.0 so you have to install libllvm-6.0-dev. See http://apt.llvm.org.
@axic axic merged commit aa241f5 into master Jul 31, 2018
@axic axic deleted the wavm-project branch July 31, 2018 18:02
@axic axic removed the in progress label Jul 31, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants