Closed Bug 1436955 Opened 2 years ago Closed 2 years ago

Make ARM64 simulator support wasm

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

ARM64
All
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file, 1 obsolete file)

The parts needed for wasm are support for illegal-instruction and out-of-bounds faults, and a couple of new callouts.  There's also a bug that needs fixing.
See commit msg for most details; nothing here should be surprising.  The one thing worth noting is that I have not fixed the SIMD memory accesses for OOB faults because they take a different path; I'm not in the mood to do the reengineering of that now since we still won't support asm.js on the platform and wasm does not yet have SIMD.
Attachment #8949659 - Flags: review?(bbouvier)
Comment on attachment 8949659 [details] [diff] [review]
bug1436955-arm64-simulator-fixes.patch

Review of attachment 8949659 [details] [diff] [review]:
-----------------------------------------------------------------

Pretty mechanic. Thanks!

::: js/src/jit/arm64/vixl/MozSimulator-vixl.cpp
@@ +274,2 @@
>    if (!fp)
>        return;

This can be removed, now that the jit->wasm patch has landed...

@@ +280,2 @@
>  
> +  startWasmInterrupt(act, pc, fp);

... and this now returns bool, thus needs testing.

@@ +285,5 @@
> +bool
> +Simulator::handle_wasm_seg_fault(uintptr_t addr, unsigned numBytes)
> +{
> +    uint8_t* pc = (uint8_t*)get_pc();
> +    uint8_t* fp = (uint8_t*)xreg(29);

a get_fp() method would be nice, to avoid remembering what's the register number each time? (and would have prevented the error you fixed above)

@@ +301,5 @@
> +        return false;
> +
> +    const js::wasm::MemoryAccess* memoryAccess = instance->code().lookupMemoryAccess(pc);
> +    if (!memoryAccess) {
> +        startWasmInterrupt(act, pc, fp);

MOZ_ALWAYS_TRUE

::: js/src/jit/arm64/vixl/Simulator-vixl.cpp
@@ +1027,5 @@
> +    return dummy;
> +}
> +
> +template<typename T> T
> +Simulator::Read(uintptr_t address_)

nit: is the trailing underscore needed? otherwise might be nice to just get rid of it?

@@ +1036,5 @@
> +    return Memory::Read<T>(address_);
> +}
> +
> +template <typename T> void
> +Simulator::Write(uintptr_t address_, T value)

ditto
Attachment #8949659 - Flags: review?(bbouvier) → review+
Rebased, picked up a few bug fixes & changes to ARM simulator, ready to land.  Carrying r+.
Attachment #8949659 - Attachment is obsolete: true
Attachment #8954024 - Flags: review+
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b24d7a6d720a
ARM64 Simulator, bugfixes and wasm support. r=bbouvier
https://hg.mozilla.org/mozilla-central/rev/b24d7a6d720a
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.