Closed Bug 916662 Opened 6 years ago Closed 6 years ago
Fix Float32 assertions on ARM
Following bug 915495, comment 20, some assertions showed up on ARM after the Float32 landing. The assertions during Lowering weren't taking into account whether the frontend can generate Float32 or not. Doug, could you check that it solves most of the jit-test failures on ARM, please? I will flag for review only if it's the case.
(In reply to Benjamin Bouvier [:bbouvier] from comment #0) > Created attachment 805145 [details] [diff] [review] > arm-assertions.patch ... > Doug, could you check that it solves most of the jit-test failures on ARM, > please? I will flag for review only if it's the case. Looks like a useful improvement, thank you. Re-ran all the failing tests in this class and they all either pass or fail for other reasons now. In particular: tests/basic/testTypedArrays.js now crashes trying to re-target a near branch. tests/jaeger/testSetTypedFloatArray.js now crashes due to failure of the constant pools which bails out and cause a subsequent assertion failure. tests/ion/setelem-float32-typedarray-ic.js is still failing with numerical errors.
- Fixes assertions by checking that we can actually emit Float32. - Fixes a case where a conversion to Float was done twice, on ARM (IonCaches).
Comment on attachment 805586 [details] [diff] [review] fix Review of attachment 805586 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! An aside: I wonder if we should do some refactoring to unify the use of ScalarTypeRepresentation in typed array (and soon, typed object) related code to determine the action over MIRType?
Attachment #805586 - Flags: review?(shu) → review+
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 805586 [details] [diff] [review] fix [Approval Request Comment] Bug caused by (feature/regressing bug #): regression of bug 915495 User impact if declined: correctness issues on websites that use Float32Arrays on ARM (phones and tablets), that can lead to visual glitches or crashes. Testing completed (on m-c, etc.): no problem on m-c Risk to taking this patch (and alternatives if risky): very low, if not no risk at all String or IDL/UUID changes made by this patch: N/A
Attachment #805586 - Flags: approval-mozilla-aurora?
Attachment #805586 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.