Closed Bug 1749671 Opened 2 years ago Closed 2 years ago

[Arm64] Ion exhausts virtual registers on web.autocad.com

Categories

(Core :: JavaScript Engine: JIT, defect, P2)

ARM64
All
defect

Tracking

()

RESOLVED FIXED
98 Branch
Tracking Status
firefox98 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

In 98 Nightly on an Apple M1, disable wasm_baselinejit and ensure wasm_optimizingjit is enabled. Go to web.autocad.com and open a sample. Observe loading of the sample never completes. Open the console and observe the OOM error. (It's possible wasm_verbose has to be enabled and wasm_caching has to be disabled for best results.)

With the compilers reversed it works OK for me but this could be a fluke. Also, any combination of switches seems to work OK on x86_64, both Linux and macOS.

I think this may be because each executable memory blob on ARM64 is limited to 128MB (see ProcessExecutableMemory.h) so as to avoid having to patch branches that go beyond the instruction's branch offset field, but I need to investigate further in a build I control. It's not a great explanation because baseline code should be larger than Ion code, and we do have some logic that tries to cope with this problem.

It does not look like a code allocation OOM but it's definitely related to using Ion and not baseline.

Code blobs are definitely larger than 128MB but I think the reason this does not become a problem is that it's only the assembler buffer that's limited to 128MB, not the final code allocation. Since we compile on multiple threads no single assembler buffer will be that large. As we paste these together we insert branch islands as necessary, and that's good enough. (Single-threaded compilation might fail, however, so on low-core systems or when thread pools are sharply limited there may be mysterious failures. This is a little worrisome but not what I'm seeing here.)

(Not related: web.autocad.com is pretty awful in the sense that it has a worker than uses an entire core for something unspecified, the worker runs at 100% and never lets up, see about:performance. This seems much worse on the M1 than on an older x86-64 MBP, however, where perf impact is "only" 25%. Another mystery. Also, this worker persists for ~ 1 minute after the autocad tab is closed.)

Hm, the OOM comes from the content that's being loaded - it's an emscripten OOM bug - and it happens also on the folder view, not just on the detail view. There are a bunch of JS errors followed by the OOM message. These JS errors happen also with baseline but then no OOM. This suggests that we have a differential bug somewhere. It's not tiering per se, since it's only Ion. The oom is not thrown during normal tiered operation either. Apart from ion/codegen bugs it could be an optimized entry/exit stub problem.

Summary: [Arm64] Ion OOM when compiling wasm on web.autocad.com → [Arm64] Content OOM when Ion-only-compiling wasm on web.autocad.com
Summary: [Arm64] Content OOM when Ion-only-compiling wasm on web.autocad.com → [Arm64] Ion exhausts virtual registers on web.autocad.com

Ion aborts because it exhausts the virtual register space on arm64; the oom trickles up and is reported.

The reason this happens only on arm64 is that the virtual register space is one bit smaller on this platform, as a bit was stolen for the register field when the arm64 port was created. Also, arm64 may use more virtual registers because we try to avoid reusing registers when generating code, to remove regalloc constraints.

This is pretty tragic, really, because on 64-bit systems the underlying datum in LAllocation/LUse is 64-bit (so that it can hold a pointer) but we only use 32 bits of it for the bitfields. We should make the bitfields larger on 64-bit systems. The virtual register will still be limited to a uint32_t but this will be much more than enough.

It's easy to expand the field by three bits (from 18 to 21) on 64-bit, and this resolves the issue on web.autocad.com. I'll offer a patch for that.

It would be useful to expand it further - all the way to 32 bits - but this will require a bit more care. Julian reports that for Cranelift, they tried to fit the field into 22 bits (4M virtual registers).

Severity: -- → S3
OS: Unspecified → All
Priority: -- → P2

This is a simple fix for a too-small field for the virtual register number on arm64,
where the field gets only 18 bits. This change expands the field to 21 bits. This
is enough to solve compilation problems on web.autocad.com.

Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1be73998d9b0
Make more data bits available in LAllocation on 64-bit. r=nbp
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: