WAMR upgrade #734
WAMR upgrade #734
Conversation
0b21630 to
439f8e4
Compare
d020c48 to
a603582
Compare
|
|
||
| // Set dummy argv to capture return value | ||
| std::vector<uint32_t> argv = { 0 }; | ||
| bool success = wasm_runtime_call_wasm(execEnv, func, 0, argv.data()); |
There was a problem hiding this comment.
The only reason I change this here is because aot_create_exec_env_and_call_function is not even exposed as an API function anymore, so this file would not compile.
| @@ -32,7 +28,7 @@ int doRunner(int argc, char* argv[]) | |||
| usleep(1000 * 500); | |||
|
|
|||
| for (const auto& m : req->messages()) { | |||
| faabric::Message result = sch.getFunctionResult(m.id(), 20000); | |||
| faabric::Messa 8000 ge result = sch.getFunctionResult(m.id(), 20000 * 100); | |||
There was a problem hiding this comment.
For the local pool runner it makes sense to have longer timeouts, as we may want to run some longer running functions for debug purposes.
As a reminder, the local pool runner is a pool_runner that can be invoked from the command line. For example, the following runs a LAMMPS simulation without requiring any changes to faasm (i.e. any new lammps_runner etc):
WASM_VM=wamr inv run.pool lammps main --cmdline '-in faasm://lammps-data/in.controller.wall'| @@ -11,6 +11,7 @@ set(WAMR_BUILD_SPEC_TEST 0) | |||
| add_definitions(-DWAMR_FAASM=1) | |||
|
|
|||
| # Set AOT mode and JIT for code generation | |||
| set(WAMR_BUILD_INTERPRETER 1) | |||
There was a problem hiding this comment.
This was set by default implicitly through setting WAMR_BUILD_AOT, but I prefer to make it explicit.
| uint32_t returnValue = argv[0]; | ||
|
|
||
| // Check function result |
There was a problem hiding this comment.
Luckily, WAMR's error checking has improved since we first wrote this code, so we don't need such cumbersome checks.
| // Note, for some reason WAMR sets the return value in the argv array you | ||
| // pass it, therefore we should provide a single integer argv even though | ||
| // it's not actually used | ||
| std::vector<uint32_t> argv = { 0 }; | ||
|
|
||
| // Invoke the function | ||
| bool success = aot_create_exec_env_and_call_function( |
There was a problem hiding this comment.
This function was never part of the public API, and I think its not even available anymore.
We use the public API function wasm_runtime_call_wasm.
…mory layout protection call
095c6bb to
c203496
Compare
| { | ||
| module->validateNativePointer(requestPtrPtr, sizeof(MPI_Request)); | ||
| MPI_Request* requestPtr = reinterpret_cast<MPI_Request*>(requestPtrPtr); | ||
| faabric::util::unalignedWrite<faabric_request_t*>( |
There was a problem hiding this comment.
This unalignedWrite was actually smashing other values in the WASM stack, what was making LAMMPS hang mid execution, and what made me have a very entretaining week 🤣
src/wasm/WasmModule.cpp
Outdated
| SPDLOG_WARN("MEM - unable to reclaim unmapped memory {} at {}", | ||
| pageAligned, | ||
| offset); | ||
| // TODO - this log statement should be a warning, but for some reason |
There was a problem hiding this comment.
Unfortunately, I don't know what's going on with this, but I will look into it.
There was a problem hiding this comment.
This is now fixed as per faasm/cpp#113
| { | ||
| auto moduleInstance = this->underlying().getModuleInstance(); | ||
| uint32_t wasmOffset = | ||
| wasm_runtime_module_malloc(moduleInstance, size, nativePtr); |
There was a problem hiding this comment.
This essentially calls the malloc symbol in wasi-libc. Hence why we need to export it here.
In this PR I bump the WAMR reference, further simplify our diff with upstream WAMR, and use the newest public API calls to invoke wasm functions.
Most importantly, as part of the diff simplification, I remove some places in the WAMR codebase where we were hardcoding the memory layout. This was to prevent WAMR from doing some optimisations to the WASM's module memory. Instead of hacking WAMR (harder to maintain), I move to using a workaround in our compilation toolchain. I agree that all Faasm-compiled WASM modules will have this workaround (even if they don't run in WAMR), but the overhead is minimal (just a very small function that isn't even called), and really helps maintaining alignment with upstream WAMR. See faasm/cpp#113
I also make a couple of fixes here and there that make LAMMPS work on WAMR (!).
To-Do before merge:
This PR makes LAMMPS work on WAMR including migration (!).