Skip to content

Gabor/memcpy - #3009

Closed
ggreif wants to merge 15 commits into
masterfrom
gabor/memcpy
Closed

Gabor/memcpy#3009
ggreif wants to merge 15 commits into
masterfrom
gabor/memcpy

Conversation

@ggreif

@ggreif ggreif commented Dec 16, 2021

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

Comparing from 4cbc7f8 to 89b9016:
The produced WebAssembly code seems to be completely unchanged.

@ggreif

ggreif commented Dec 16, 2021

Copy link
Copy Markdown
Contributor Author

@nomeata the drun tests seem to fail. I cannot see enough log for linux, could you paste it? Thanks!

I have a Mac, and it looks like recent wasmtime releases need macOS 10.13, but nixpkgs don't support that version yet in the libraries universally. (I guess nixpkgs wants to support older Macs than that too.)

@nomeata

nomeata commented Dec 16, 2021

Copy link
Copy Markdown
Contributor

Does the replica already support bulk memory operations?

Anyways, for me the tests fail already with validating:

~ $ nix-build -E '(import (builtins.fetchTarball "https://github.com/dfinity/motoko/archive/gabor/memcpy.tar.gz") {}).tests.drun'
these derivations will be built:
  /nix/store/yk6vk49pclxl53228y1j3arfbpfs93hi-test-drun.drv
building '/nix/store/yk6vk49pclxl53228y1j3arfbpfs93hi-test-drun.drv'...
unpacking sources
unpacking source archive /nix/store/jchjlxrdbyww96vqr538c594hssx10xm-test-run-drun-src
source root is test-run-drun-src
running tests
patching script interpreter paths in .
./compare-perf.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./run.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./compare-wat.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./drun-wrapper.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./diff-stats.pl: interpreter directive changed from "#!/usr/bin/env perl" to "/nix/store/kpzx6f97583zbjyyd7b17rbv057l4vn2-perl-5.34.0/bin/perl"
./test-reproducibility.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./profile-report.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./ic-ref-run-wrapper.sh: interpreter directive changed from "#!/usr/bin/env bash" to "/nix/store/v1d8l3zqnia3hccqd0701szhlx22g54z-bash-5.1-p8/bin/bash"
./check-error-codes.py: interpreter directive changed from "#!/usr/bin/env python3" to "/nix/store/wl02plhc6zf84m6x9984l42wnnnbly5m-python3-3.9.6/bin/python3"
/nix/store/zb2ka4ls5ja7rih29ikxb7qcwjb4p001-moc/bin/moc
Motoko compiler (source zb2ka4ls-5ja7rih2-9ikxb7qc-wjb4p001)
make: Entering directory '/build/test-run-drun-src/run-drun'
../run.sh -d *.mo *.drun
WARNING: Could not run ic-ref-run, will skip some tests
AST-64: [tc] [run] [run-ir] [run-low]
AST-66: [tc] [run] [run-ir] [run-low] [comp] [comp-ref] [valid] [valid-ref] [drun-run]
--- AST-66.valid.ret (expected)
+++ AST-66.valid.ret (actual)
@@ -0,0 +1 @@
+Return code 1
--- AST-66.valid (expected)
+++ AST-66.valid (actual)
@@ -0,0 +1 @@
+0000bb3: error: unexpected opcode: 0xfc 0xa
--- AST-66.valid-ref.ret (expected)
+++ AST-66.valid-ref.ret (actual)
@@ -0,0 +1 @@
+Return code 1
--- AST-66.valid-ref (expected)
+++ AST-66.valid-ref (actual)
@@ -0,0 +1 @@
+0000bb3: error: unexpected opcode: 0xfc 0xa
GIT-843: [tc] [run] [run-ir] [run-low] [comp] [comp-ref] [valid] [valid-ref] [drun-run]
--- GIT-843.valid.ret (expected)
+++ GIT-843.valid.ret (actual)
@@ -0,0 +1 @@
+Return code 1

@ggreif

ggreif commented Dec 16, 2021

Copy link
Copy Markdown
Contributor Author

Does the replica already support bulk memory operations?

By @ulan's statement today, the replica (engine) should support them. Whether the byte code validator (parity?) allows them is another question.

@nomeata

nomeata commented Dec 17, 2021

Copy link
Copy Markdown
Contributor

The validator above is wasm-validate from I think wabt. Maybe it needs a flag or a bump.

@ggreif

ggreif commented Dec 17, 2021

Copy link
Copy Markdown
Contributor Author

Maybe it needs a flag or a bump.

Indeed, a flag did the trick! THX!

@ggreif

ggreif commented Dec 17, 2021

Copy link
Copy Markdown
Contributor Author

@nomeata next stop is ic-ref:

+← rejected (RC_CANISTER_ERROR): Parsing failed: getMathPrefix: illegal op 10

I can file a PR for winter, ok?

@nomeata

nomeata commented Dec 17, 2021

Copy link
Copy Markdown
Contributor

Looks like someone needs to implement bulk memory operations in winter :-)
(This is where Andreas will point out that it’s not good to rely on that Wasm implementation here.)

@ggreif

ggreif commented Dec 17, 2021

Copy link
Copy Markdown
Contributor Author

Looks like someone needs to implement bulk memory operations in winter :-)

I have successfully nerd-sniped myself into it: dfinity-side-projects/winter#29

(This is where Andreas will point out that it’s not good to rely on that Wasm implementation here.)

Nope, we just have to improve on our own toolset.

@ggreif

ggreif commented Dec 19, 2021

Copy link
Copy Markdown
Contributor Author

Okay, the two easy bulk memory operations are now ready for review in dfinity-side-projects/winter#29. @nomeata

@ggreif

ggreif commented Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

Another fresh hell opening up:

       > /nix/store/4xnsw7gdv4b3zvrb3dq8pq70db3bsnxy-moc/bin/moc
       > Motoko compiler (source 4xnsw7gd-v4b3zvrb-3dq8pq70-db3bsnxy)
       > /nix/store/rj0nm1wfpdg495myc6lj2g49dyj3800x-drun/bin/drun
       > ic standalone interface
       > Profiling assetstorage...
       > Error: UnknownOpcode(252)
       For full logs, run 'nix log /nix/store/rckfadrcb77350bdk941z8z90wm9mn4z-test-profiling-graphs.drv'.
error (ignored): error: cannot unlink '/tmp/nix-build-test-drun-compacting-gc.drv-0/test-run-drun-src/run-drun/ok': Directory not empty

Resolved by the below commit, phew.

@ggreif
ggreif force-pushed the gabor/memcpy branch 2 times, most recently from d2f95a2 to f363f78 Compare December 20, 2021 20:36
@ggreif

ggreif commented Dec 20, 2021

Copy link
Copy Markdown
Contributor Author

@nomeata I guess I once again need the tests.drun traces or the tests.ic-ref-run ones, whichever is the failing one. I cannot see the relevant parts in the error logs.

@nomeata

nomeata commented Dec 21, 2021

Copy link
Copy Markdown
Contributor

@nomeata I guess I once again need the tests.drun traces or the tests.ic-ref-run ones, whichever is the failing one. I cannot see the relevant parts in the error logs.

#3019 should get us the logs we need

@luc-blaeser

Copy link
Copy Markdown
Contributor

Just doing some repo cleanup for better focussing on active work:
Closing PR as it seems inactive for a longer time. Please re-open if it is still relevant and after it has been updated with latest master changes.

@luc-blaeser luc-blaeser closed this Dec 3, 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.

3 participants