Skip to content

Makefile: add tinygo-test-wasi; like tinygo-test but with -target wasi #2373

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 24, 2021

Conversation

dkegel-fastly
Copy link
Contributor

Skips the standard library modules whose tests don't pass yet in wasi (see recently filed bugs).

@dgryski
Copy link
Member

dgryski commented Dec 10, 2021

Also dgryski/tinygo-test-corpus#1

@aykevl
Copy link
Member

aykevl commented Dec 11, 2021

Looks reasonable to me. But also see #2031.

@dgryski
Copy link
Member

dgryski commented Dec 12, 2021

Also if we're going to start splitting out stdlib package tests, another nice section might be "Packages which pass but fail in CI due to CPU or memory requirements." So far this includes: sort (needs -v to pass until t.Skip is implemented), compress/lzw, and regexp/syntax.

@dkegel-fastly
Copy link
Contributor Author

Looks like crypto/ed25519 go/format net are also passing?

@aykevl
Copy link
Member

aykevl commented Dec 17, 2021

Also if we're going to start splitting out stdlib package tests, another nice section might be "Packages which pass but fail in CI due to CPU or memory requirements." So far this includes: sort (needs -v to pass until t.Skip is implemented), compress/lzw, and regexp/syntax.

Possible, although I'd like to fix those issues instead ;)

@aykevl
Copy link
Member

aykevl commented Dec 17, 2021

Looks like crypto/ed25519 go/format net are also passing?

The net package passes for me, but the other two don't:

$ tinygo test crypto/ed25519
ld.lld-11: error: undefined symbol: crypto/ed25519/internal/edwards25519/field.feSquare
>>> referenced by fe.go:305 (/usr/local/go1.17/src/crypto/ed25519/internal/edwards25519/field/fe.go:305)
>>>               /tmp/tinygo3886713202/main.o:((*crypto/ed25519/internal/edwards25519/field.Element).Square)

ld.lld-11: error: undefined symbol: crypto/ed25519/internal/edwards25519/field.feMul
>>> referenced by fe.go:299 (/usr/local/go1.17/src/crypto/ed25519/internal/edwards25519/field/fe.go:299)
>>>               /tmp/tinygo3886713202/main.o:((*crypto/ed25519/internal/edwards25519/field.Element).Multiply)

ld.lld-11: error: undefined symbol: syscall.RawSyscall
>>> referenced by zsyscall_linux_amd64.go:1661 (/usr/local/go1.17/src/syscall/zsyscall_linux_amd64.go:1661)
>>>               /tmp/tinygo3886713202/main.o:(os.Pipe)
error: failed to link /tmp/tinygo3886713202/main: exit status 1
$ tinygo test go/format
panic: go/parser internal error: unbalanced scopes
FAIL	go/format	0.002s
FAIL

Really hoping for go/format support because it would be a nice feature for play.tinygo.org: formatting code directly in the browser without a server roundtrip.

@aykevl
Copy link
Member

aykevl commented Dec 17, 2021

Ah, I found one: regexp/syntax. And the culprit is mkCharClass.

@aykevl
Copy link
Member

aykevl commented Dec 17, 2021

I've profiled it a bit and it looks like the main culprit is heap allocation. I couldn't quickly find a single obvious performance bottleneck (apart from mem.store which takes up 5.26s out of ~30s - too big, but not an obvious bottleneck).
I think it might be possible to make the run function mostly free of heap allocations, at least in all the hot code paths. But that will require some significant refactoring of the interp package.

@dgryski
Copy link
Member

dgryski commented Dec 17, 2021

compress/zlib should pass now.

@dgryski
Copy link
Member

dgryski commented Dec 18, 2021

compress/bzip2 should pass once #2387 has landed.

@dgryski
Copy link
Member

dgryski commented Dec 18, 2021

reflect should be back to passing on wasi now that #2395 is in.

@dgryski
Copy link
Member

dgryski commented Dec 18, 2021

debug/macho failed for the same reason bzip2 did, and will pass once #2387 is in.

@deadprogram
Copy link
Member

Both #2387 and also #2395 have been merged.

Looks like this PR here needs merge conflict resolution/rebase, please.

@deadprogram
Copy link
Member

Thanks everyone for working on this! Now merging.

@deadprogram deadprogram merged commit 0aed62e into tinygo-org:dev Dec 24, 2021
@dkegel-fastly dkegel-fastly deleted the dkegel-add-wasi-test branch December 25, 2021 04:05
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