-
-
Notifications
You must be signed in to change notification settings - Fork 848
Adding wasm64 target arch #5853
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
base: master
Are you sure you want to change the base?
Conversation
- Copied most settings from wasm64p32 and added memory64 - TODO: freestanding/wasi wasm64
- TODO: Fix i32 vs i64 in memory size and grow
- Need to double check this, but this is the only way to get it to work. The memory index needs to be 32 bit, but the page size is 64 bit - Also updated the wasm32 to explicitly take i32 instead of uintptr
|
Compile with
wasm_test.odin: package wasm_test
@(export)
test_pointers :: proc "contextless" () -> uintptr {
data := [4]u64{0x1234567890ABCDEF, 0xFEDCBA0987654321, 0xAAAABBBBCCCCDDDD, 0x1111222233334444}
ptr := raw_data(data[:])
base_addr := uintptr(ptr)
when ODIN_ARCH == .wasm64 {
large_addr := base_addr | 0x1234567800000000 // just to show we are outside of u32 range
return large_addr
}
return base_addr
}
@(export)
test_max_int :: proc "contextless" () -> int {
return max(int)
}
@(export)
get_pointer_size :: proc "contextless" () -> int {
return size_of(rawptr)
}
@(export)
get_int_size :: proc "contextless" () -> int {
return size_of(int)
}index.html: <html>
<body>
<h1>WASM 64 vs 32 bit Support</h1>
<div id="results"></div>
<script>
function toUnsignedHex(value, bits = 64) {
if (typeof value === 'bigint') {
if (value < 0n) {
return (value + (1n << BigInt(bits))).toString(16).toUpperCase().padStart(bits / 4, '0');
}
return value.toString(16).toUpperCase().padStart(bits / 4, '0');
}
return value.toString(16).toUpperCase().padStart(bits / 4, '0');
}
async function loadWasm(filename, title) {
const wasmModule = await WebAssembly.instantiateStreaming(fetch(filename));
const { exports } = wasmModule.instance;
const pointerSize = exports.get_pointer_size();
const intSize = exports.get_int_size();
const testInt = exports.test_64bit_int();
const testPointer = exports.test_64bit_pointers();
return `<h3>${title}</h3>
<pre>
Pointer size: ${pointerSize} bytes
Int size: ${intSize} bytes
64-bit int value: 0x${toUnsignedHex(testInt)}
64-bit pointer value: 0x${toUnsignedHex(testPointer)}
</pre>`;
}
async function loadBoth() {
const results = document.getElementById('results');
try {
const wasm64Result = await loadWasm('wasm64.wasm', 'WASM64');
const wasm32Result = await loadWasm('wasm32.wasm', 'WASM32');
results.innerHTML = wasm64Result + wasm32Result;
} catch (error) {
results.innerHTML = `<p>Error loading WASM files: ${error.message}</p>`;
}
}
loadBoth();
</script>
</body>
</html>Output: # WASM 64 vs 32 bit Support
### WASM64
Pointer size: 8 bytes
Int size: 8 bytes
64-bit int value: 0x7FFFFFFFFFFFFFFF
64-bit pointer value: 0x12345678000FFFE0
### WASM32
Pointer size: 4 bytes
Int size: 4 bytes
64-bit int value: 0x000000007FFFFFFF
64-bit pointer value: 0x00000000000FFFE0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! Some surface level comments before any actual testing.
runtime.HAS_HARDWARE_SIMDis missing a wasm64 check- all the JS needs to be updated/checked:
core/sys/wasm/js/odin.js,vendor/wgpu/wgpu.js. For exampleloadPtrof theWasmMemoryInterfaceis loading 4 bytes, but also more subtle things likegetSourcefor webgl doingi*stringSize + 4which on wasm64 would become + 8. Also I believe any pointer will now become a JS BigInt.
With wgpu being so big we can also just skip it for now, by adding a#+build !wasm64. But the correctodin.jsis going to be needed here.
| if (is_wasm64) { | ||
| // WASM64 uses i64 for memory operations | ||
| LLVMTypeRef types[1] = { | ||
| lb_type(p->module, t_i64), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this should be t_int then 32 bit for wasm32 and 64 bit for wasm64. Same thing with the result and page count, that can also become int. Then this entire if can just be one thing again.
Same thing with the wasm_memory_size proc.
Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had t_int initially but when I was trying to debug the memory indexing, I made it explicit. I think most of it can be combined again with t_int except for args[0].
I'm not sure if that is a bug as the wasm spec seems to indicate 64bit indexing, but only a 32bit args[0] compiled and ran successfully.
I'll do a thorough pass through Regarding Essentially, it would look like swapping "wgpu" with "webgl" in the example below: <script type="text/javascript" src="odin.js"></script>
<script type="text/javascript" src="wgpu.js"></script>
<script type="text/javascript">
// ...
const wgpuInterface = new odin.WebGPUInterface(memInterface);
odin.runWasm("triangle.wasm", null, { wgpu: wgpuInterface.getInterface() }, memInterface);
</script> |
I would say that's a good idea initially or if nobody was using it the way it is now. I don't think it's worth the breaking change, at least for now/the scope of this pr. |
I started this branch mainly to test some Wasm 3.0 features, but ended up implementing the
js_wasm64target. I have neither implemented nor tested thefreestanding/wasitargets for wasm64.As this branch currently stands, Odin files compile successfully to
js_wasm64. Outputting to.watwithwasm2wat test.wasm --enable-memory64 -o test.watshowsi64being used where expected. Browser testing confirms 64 bit integers and pointers.Everything compiles successfully, so hopefully I didn't miss anything in the compiler. Maybe some error messages? Spent quite a bit of time reading through the llvm files and with CTRL+F. Most of the wasm64 implementation is adapted from the existing wasm64p32 arch. I also made sure wasm's
memory.sizeandmemory.growuse the correct sized integers inllvm_backend_proc.cpp. According to the wasm spec, memory size and grow instructions are supposed to "use the address type instead ofi32". Getting this to compile cleaning without llvm/wasm errors required keeping the memory index as 32 bits.I will add some test files here. Let me know what work needs to be done to bring this up to spec.