IonMonkey: (ARM) Assertion failure: tempFloat32 != InvalidFloatReg, at js/src/jit/IonCaches.cpp:3748

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dougc, Assigned: dougc)

Tracking

Trunk
mozilla34
ARM
All
Points:
---

Firefox Tracking Flags

(firefox33 fixed, firefox34 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

With bug 1046016 patched, hasUnaliasedDouble() can now return false when there are only 16 DP registers, and this hits an assertion failure in the SetElementCache path.
Would this be an appropriate fix?

Was the intent to avoid allocating an extra float32 register in this case?
Attachment #8464695 - Flags: review?(mrosenberg)
Comment on attachment 8464695 [details] [diff] [review]
Correct handling of 16 DP float registers in for SetElementCache code generation.

Review of attachment 8464695 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/IonCaches.cpp
@@ -3744,5 @@
>      int width = Scalar::byteSize(arrayType);
>      BaseIndex target(elements, index, ScaleFromElemWidth(width));
>  
>      if (arrayType == Scalar::Float32) {
> -        JS_ASSERT(tempFloat32 != InvalidFloatReg);

I'd probably change this assert to JS_ASSERT_IF(hasUnaliasedDouble(), ...);
and rather than checking tempFloat32 != InvalidFloatReg, just checking hasUnaliasedDouble()
Attachment #8464695 - Flags: review?(mrosenberg) → review+
Thank you for the quick review. All suggestions have been integrated. Carrying forward the r+. Try run: https://tbpl.mozilla.org/?tree=Try&rev=f83c32358d06
Attachment #8464695 - Attachment is obsolete: true
Attachment #8465367 - Flags: review+
And fix the patch description. Carrying forward r+.
Attachment #8465367 - Attachment is obsolete: true
Attachment #8465369 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/736a5f1a92b5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Comment on attachment 8465369 [details] [diff] [review]
Correct handling of 16 DP float registers for SetElementCache code generation.

Approval Request Comment
[Feature/regressing bug #]: Bug 991153.

[User impact if declined]: Crashes, or bad code generation, on ARM devices without vfp3.

[Describe test coverage new/current, TBPL]: Local testing, and TBPL.

[Risks and why]: Low. It's a small patch that only affects the code path for the affected devices and these had jit-tests failures that this patch corrects.

[String/UUID change made/needed]: n/a
Attachment #8465369 - Flags: approval-mozilla-aurora?
Attachment #8465369 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.