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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Douglas Crosher [:dougc]]

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.

Comment 1

[Douglas Crosher [:dougc]]

Attachment: Correct handling of 16 DP float registers in for SetElementCache code generation.

Would this be an appropriate fix?

Was the intent to avoid allocating an extra float32 register in this case?

Comment 2

[Marty Rosenberg [:mjrosenb]]

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()

Comment 3

[Douglas Crosher [:dougc]]

Attachment: Correct handling of 16 DP float registers in for SetElementCache code generation.

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

Comment 4

[Douglas Crosher [:dougc]]

Attachment: Correct handling of 16 DP float registers for SetElementCache code generation.

And fix the patch description. Carrying forward r+.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/736a5f1a92b5

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/736a5f1a92b5

Comment 7

[Douglas Crosher [:dougc]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/967ea1f65380