Skip to content

Conversation

@maht
Copy link

@maht maht commented Sep 30, 2022

Small improvements/fixes I suggest related to documented examples.

Note: this is my first PR in this project, so please let me know if there if something is wrong or needed.

For unknown reasons, the './' seems to be needed:

```
build> ./iwasm --native-lib=libtest_add.so --native-lib=libtest_sqrt.so wasm-app/test.wasm
[05:50:21:655 - 7FCE9C49FCC0]: warning: failed to load native library libtest_add.so
[05:50:21:707 - 7FCE9C49FCC0]: warning: failed to load native library libtest_sqrt.so
[05:50:21:755 - 7FCE9C49FCC0]: warning: failed to link import function (env, test_add)
[05:50:21:761 - 7FCE9C49FCC0]: warning: failed to link import function (env, test_sqrt)
Hello World!
Exception: failed to call unlinked import function (env, test_add)
build> ./iwasm --native-lib=./libtest_add.so --native-lib=./libtest_sqrt.so wasm-app/test.wasm
Hello World!
10 + 20 = 30
sqrt(10, 20) = 500
build>
```
In order to be properly debugged as in the documented
example:

 * `iwasm` has to be compiled using `Debug` build type.
 * `test.aot` has to be compiled without `wamrc` optimizations
This makes the example easier to reproduce.
This makes the guide easier to follow.
cd ${WAMR_ROOT}/product-mini/platforms/linux
mkdir build && cd build
cmake .. -DWAMR_BUILD_DEBUG_AOT=1
cmake .. -DCMAKE_BUILD_TYPE:STRING="Debug" -DWAMR_BUILD_DEBUG_AOT=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Must we build iwasm with Debug mode?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't think so.

i guess his intention was to match with the example below, where lldb is showing the source code of iwasm.
i think it's less confusing to add a note to the example.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, how about we add a note here to mention that the -DCMAKE_BUILD_TYPE:STRING="Debug" option is to make lldb be able to showing the source code of iwasm?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i guess it makes sense. @maht can you confirm?

4. Compile wasm module to AOT module
``` bash
wamrc -o test.aot test.wasm
${WAMR_ROOT}/wamr-compiler/build/wamrc --opt-level=0 --size-level=0 -o test.aot test.wasm
Copy link
Contributor

Choose a reason for hiding this comment

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

Must we add --opt-level=0 --size-level=0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

when i created the example in #1496,
i was using:

CFLAGS=-Os -g and

wamrc -o test.aot test.wasm

it was on x86-64 mac.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, @maht could you help confirm it? Since --opt-level=0 --size-level=0 flags for wamrc impact the AOT performance a lot, we had better remove them if they are not necessary.


The following `test.c` file is used in this document as example:

```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
```
``` C

```bash
cd build
./iwasm --native-lib=libtest_add.so --native-lib=libtest_sqrt.so wasm-app/test.wasm
./iwasm --native-lib=./libtest_add.so --native-lib=./libtest_sqrt.so wasm-app/test.wasm
Copy link
Collaborator

Choose a reason for hiding this comment

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

i picked this commit in #1643 as it conflicts otherwise.

@wenyongh
Copy link
Contributor

wenyongh commented Feb 2, 2024

Close the PR as it hasn't been updated for a long time.

@wenyongh wenyongh closed this Feb 2, 2024
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.

4 participants