1791483 - js/src/jit/arm/Architecture-arm.cpp:533:16: error: instruction requires: data-barriers

Status: NEW

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

Description

[Mike Hommey [:glandium]]

When building for armv5t:

[task 2022-09-20T00:07:30.981Z] 00:07:30     INFO -  In file included from Unified_cpp_js_src_jit12.cpp:47:
[task 2022-09-20T00:07:30.981Z] 00:07:30    ERROR -  /builds/worker/checkouts/gecko/js/src/jit/arm/Architecture-arm.cpp:533:16: error: instruction requires: data-barriers
[task 2022-09-20T00:07:30.982Z] 00:07:30     INFO -    asm volatile("isb\n" : : : "memory");
[task 2022-09-20T00:07:30.982Z] 00:07:30     INFO -                 ^
[task 2022-09-20T00:07:30.982Z] 00:07:30     INFO -  <inline asm>:1:2: note: instantiated into assembly here
[task 2022-09-20T00:07:30.982Z] 00:07:30     INFO -          isb
[task 2022-09-20T00:07:30.983Z] 00:07:30     INFO -          ^

Interestingly, this is something GCC doesn't warn about, but ISB is, AFAICT, an instruction that is new to ARMv6.

I guess ideally the ISB should be behind something akin to mozilla::supports_armv6() (and the function compiled for at least armv6).

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

Well armv5 has never been officially supported …
I am not in favor of adding a runtime conditional around this instruction, which is here to ensure safety of the instruction caches.

However, I am fine with a configure flag such as --disable-instruction-cache-synchronization-because-this-would-only-run-on-a-single-thread.
On a similar note, we already have --enable-arm64-fjcvtzs.

Comment 2

[Jan de Mooij [:jandem]]

Maybe configure should just default to --disable-jit when compiling for ARM < 7 at this point.

Comment 3

[Nicolas B. Pierron [:nbp] {away}]

additional alternatives would be to:

• Ensure that all WASM compilations are happening on the main thread.
• Disabling WebAssembly.

Comment 4

[Mike Hommey [:glandium]]

(In reply to Nicolas B. Pierron [:nbp] from comment #1)

Well armv5 has never been officially supported …
I am not in favor of adding a runtime conditional around this instruction, which is here to ensure safety of the instruction caches.

However, I am fine with a configure flag such as --disable-instruction-cache-synchronization-because-this-would-only-run-on-a-single-thread.
On a similar note, we already have --enable-arm64-fjcvtzs.

mozilla::supports_armv6() expands to a constant true when building for >= armv6.

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/memory-access-ordering-part-3---memory-access-ordering-in-the-arm-architecture :

Before the Armv6 architecture, not much explicit was defined about the out-of-ordering of memory accesses - the Sequential Execution Model was assumed to apply to all instructions. Processors that implemented caches and write buffers could mark regions of memory as being cacheable or bufferable without greater side effects than the obvious ones.

I guess using the compiler elided branch is ok then, while adding the above comment next to the branch.