awfy regression on misc-typedobj-write-struct-field-typedobj.js

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

unspecified
mozilla33
x86_64
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Created attachment 8447536 [details] [diff] [review]
boundscheck-foldsto.patch

misc-typedobj-write-struct-field-typedobj.js slowed down as a result of the new GVN (bug 1004363). There were speedups in more important benchmarks, so reverting is undesirable.

It looks like the new GVN is deleting the same number of instructions as the old one, but it's deleting different instructions; when two instructions are redundant and both dominate all uses of the other, either can be kept, and the new GVN chooses differently than the old in some cases.

Looking at the generated code, the first thing I noticed is that there are several arithmetic instructions with all constant operands which aren't being folded away. This happens because there are MBoundsCheck instructions with constant operands which aren't folded away until Bounds Check Elimination, which is very late, and there's no follow-up folding.

Attached is a patch which adds a foldsTo implementation for constant-operand MBoundsCheck to fold it during GVN, which can do follow-up folding. This allows several more instructions in misc-typedobj-write-struct-field-typedobj.js to be folded away.
Attachment #8447536 - Flags: review?(jdemooij)

Updated

4 years ago
Attachment #8447536 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 1

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8704f33d10d7

On AWFY, this mostly makes up for the regression, though not completely. I've looked at the output of GVN before and after this patch, and the only difference is that it makes some arbitrary decisions differently. Since this is the only known testcase at this point that regressed, I'm inclined to leave it at that.
https://hg.mozilla.org/mozilla-central/rev/8704f33d10d7
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.