Abstract/Separate Platform Specific Code

[sampaioletti]

I was playing around with Wagon and wanted to see if it would run on my Raspberry Pi, initially I couldn't get it to build so i looked around for what changes would need to happen to facilitate that. This PR is a POC and RFC only. It is just a rough hack to see what areas would need to be abstracted to enable cross compilation.

This branch builds and runs on GOOS=linux GOARCH=arm and can run the exec/testdata/basic.wasm no further testing has been attempted at this time.

I'm going to start looking at the abstractions a little bit but figured I would open it up to discussion to see if anyone had any thoughts about how the implementation should look

The main abstractions that need to occur separate the Backend from the VM

Portions of the code were split into _amd64.go files, but not all of it.

We could further split the code into build specific files with a default noop implementation, sort of what I did here...but better obviously (:

Or we could separate the vm from a backend compiler by doing a sort of BackendProvider in the middle

Or..?

Appreciate comments

Thanks for the great project!

[codecov-io]

Codecov Report

Merging #162 into master will not change coverage.
The diff coverage is 66.25%.

@@           Coverage Diff           @@
##           master     #162   +/-   ##
=======================================
  Coverage   69.54%   69.54%           
=======================================
  Files          43       44    +1     
  Lines        5007     5007           
=======================================
  Hits         3482     3482           
  Misses       1231     1231           
  Partials      294      294

Impacted Files	Coverage Δ
exec/internal/compile/native.go	0% <ø> (-54.55%)	⬇️
exec/internal/compile/native_exec.go	100% <ø> (ø)	⬆️
exec/internal/compile/allocator.go	86.2% <ø> (ø)	⬆️
exec/native_compile_nogae_amd64.go	100% <ø> (ø)
exec/internal/compile/backend_amd64.go	79.97% <ø> (ø)	⬆️
exec/internal/compile/scanner.go	0% <0%> (ø)	⬆️
exec/internal/compile/native_completion_amd64.go	100% <100%> (ø)
exec/native_compile_amd64.go	65.27% <65.27%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e64ad3...6ca31ac. Read the comment docs.

[twitchyliquid64]

Thanks! this is looking good.

My original intent for separating the different backends was to abstract on the pageAllocator and instructionBuilder interfaces. Can we keep that approach, and just use build tags inside internal/compile + enabling the correct backends?

[twitchyliquid64]

Wouldn't the allocator work on ARM?

[sampaioletti]

The problem was with the dependence on the asmBlock type in native_exec

wagon/exec/internal/compile/native_exec.go

Line 16 in 1e64ad3

return JITExitSignal(jitcall(unsafe.Pointer(&b.mem), stack, locals, globals, mem))

which depends on the amd64 generated function jitcall from native_exec_amd64.go

wagon/exec/internal/compile/native_exec_amd64.go

Line 11 in 1e64ad3

func jitcall(asm unsafe.Pointer, stack, locals, globals *[]uint64, mem *[]byte) uint64

wagon/exec/internal/compile/native_exec_amd64.s

Line 11 in 1e64ad3

TEXT ·jitcall(SB),NOSPLIT|NOFRAME,$0-48

So the allocator could...with some more abstraction.

[twitchyliquid64]

Okay, so we should add an implementation of jitcall that works on other arch's (or even just panicks), but the MMapAllocator & asmBlock types should be shareable, right?

[sampaioletti]

I believe so...i'll give that a try.

[twitchyliquid64]

What part of this file won't be applicable/compile on ARM?

[sampaioletti]

This one was due to a overrun from exitIndexMask

wagon/exec/internal/compile/native.go

Lines 36 to 38 in 1e64ad3

	func makeExitIndex(idx int) CompletionStatus {
	return CompletionStatus((idx << 8) & exitIndexMask)
	}

I didnt take the time to debug it..just took it out...i'll look into it more

[twitchyliquid64]

Did you run it on a 32bit system? We probably should use uint64 everywhere here instead of int, which will match the platforms register size.

[sampaioletti]

yes i tested it on a raspberry pi running raspbian so they run 32 bit arm

[twitchyliquid64]

Gotcha. I think if you change idx (and values adjacent to it) to uint64, it should work.