Created attachment 8887000 [details] [diff] [review] Patch On x86 without SSE 4.1, boxDouble can clobber the source double register. This is surprising and breaks some CacheIR code. evilpie's patch for bug 1344483 triggers a jit-test failure. This patch makes this more explicit by adding an additional temp argument to boxDouble. Most callers can pass the same register as the source register, but callers that want to preserve the source can pass a different register.
Attachment #8887000 - Flags: review?(evilpies)
We should probably backport this to 55. Fortunately this only affects pretty old CPUs (x86 without SSE 4.1).
status-firefox54: --- → wontfix
status-firefox55: --- → affected
status-firefox56: --- → affected
status-firefox-esr52: --- → unaffected
tracking-firefox55: --- → ?
tracking-firefox56: --- → ?
Comment on attachment 8887000 [details] [diff] [review] Patch Review of attachment 8887000 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks for looking into this, I would probably not have found this.
Attachment #8887000 - Flags: review?(evilpies) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e73020a1e023 Fix boxDouble to not clobber the source register on x86 CPUs without SSE 4.1. r=evilpie
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Please request Beta approval on this when you're comfortable doing so.
Comment on attachment 8887000 [details] [diff] [review] Patch Approval Request Comment [Feature/Bug causing the regression]: Bug 1341067. [User impact if declined]: Broken websites for users with old CPUs. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Patch is mechanical for the most part, only changes behavior for users on 32-bit x86 CPUs that don't support SSE 4.1 [String changes made/needed]: None.
Attachment #8887000 - Flags: approval-mozilla-beta?
Comment on attachment 8887000 [details] [diff] [review] Patch jit fix for some x86 cpus, beta55+
Attachment #8887000 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox55: affected → fixed
You need to log in before you can comment on or make changes to this bug.