Wasm baseline: Generate less branchy code for cmp64Set() on ARM, x86_32

ASSIGNED
Assigned to

Status

()

Core
JavaScript Engine: JIT
P5
normal
ASSIGNED
a year ago
11 months ago

People

(Reporter: lth, Assigned: dannas, Mentored)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

a year ago
The current code is pretty branchy, and it is not yet obvious that we'll reduce the incidence of cmp64Set by boolean optimization for control (bug 1286816) since it'll lead to fairly hairy code in the compiler on 32-bit systems.  So it would be good to generate better code for cmp64Set.

Probably this comes down to breaking x86 and ARM apart and having custom code for both.  ARM is more important; it should be easy to generate good code on ARM.
(Reporter)

Updated

a year ago
See Also: → bug 1316848
(Assignee)

Comment 1

a year ago
I'd like to take a stab at this one. Cmp64Set is defined here http://searchfox.org/mozilla-central/source/js/src/wasm/WasmBaselineCompile.cpp#2987

Looks like cmp64Set() is used for reducing repeated code when emitting compare operations. http://searchfox.org/mozilla-central/source/js/src/wasm/WasmBaselineCompile.cpp#6247

First step: Figure out how to do the equivalent of the x64 case for arm.
(Assignee)

Comment 2

a year ago
Created attachment 8823828 [details] [diff] [review]
bug1316822_add_cmp64set_for_arm32.patch

WIP. Untested. 

The cmp ARM instruction sets the APSR register. According to the docs it's supposed to be equivalent to a subs (but without writing back the result). I wonder if there's any instruction that can be used for comparing 64 bit values on a 32 bit arm platform. That would simplify things a bit. So far I haven't found any.
Assignee: nobody → dannas
Status: NEW → ASSIGNED
(Assignee)

Comment 3

11 months ago
Created attachment 8826574 [details] [diff] [review]
bug1316822_add_cmp64set_for_arm32.patch
Attachment #8823828 - Attachment is obsolete: true
(Reporter)

Updated

11 months ago
Priority: P3 → P5
You need to log in before you can comment on or make changes to this bug.