Bug 1039993 - Don't try to re-use the input on float32 -> double conversions, it can go wrong on ARM due to deep-seated reasons. r=jandem, r=terrence, a=sledru
authorMarty Rosenberg <mrosenberg@mozilla.com>
Wed, 24 Sep 2014 02:26:00 -0400
changeset 216889 b4d40427d6e8
parent 216882 0780dce35e25
child 216890 041c9b3d66b1
push id3956
push userryanvm@gmail.com
push date2014-09-30 12:51 +0000
treeherdermozilla-beta@b4d40427d6e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, terrence, sledru
bugs1039993
milestone33.0
Bug 1039993 - Don't try to re-use the input on float32 -> double conversions, it can go wrong on ARM due to deep-seated reasons. r=jandem, r=terrence, a=sledru
js/src/jit/LiveRangeAllocator.cpp
js/src/jit/Lowering.cpp
--- a/js/src/jit/LiveRangeAllocator.cpp
+++ b/js/src/jit/LiveRangeAllocator.cpp
@@ -646,33 +646,41 @@ LiveRangeAllocator<VREG, forLSRA>::build
             // Calls may clobber registers, so force a spill and reload around the callsite.
             if (ins->isCall()) {
                 for (AnyRegisterIterator iter(allRegisters_); iter.more(); iter++) {
                     if (forLSRA) {
                         if (!addFixedRangeAtHead(*iter, inputOf(*ins), outputOf(*ins)))
                             return false;
                     } else {
                         bool found = false;
+
                         for (size_t i = 0; i < ins->numDefs(); i++) {
                             if (ins->getDef(i)->isFixed() &&
                                 ins->getDef(i)->output()->aliases(LAllocation(*iter))) {
                                 found = true;
                                 break;
                             }
                         }
                         if (!found && !addFixedRangeAtHead(*iter, outputOf(*ins), outputOf(*ins).next()))
                             return false;
                     }
                 }
             }
 
+            DebugOnly<bool> hasDoubleDef = false;
+            DebugOnly<bool> hasFloat32Def = false;
             for (size_t i = 0; i < ins->numDefs(); i++) {
                 if (ins->getDef(i)->policy() != LDefinition::PASSTHROUGH) {
                     LDefinition *def = ins->getDef(i);
-
+#ifdef DEBUG
+                    if (def->type() == LDefinition::DOUBLE)
+                        hasDoubleDef = true;
+                    if (def->type() == LDefinition::FLOAT32)
+                        hasFloat32Def = true;
+#endif
                     CodePosition from;
                     if (def->policy() == LDefinition::FIXED && def->output()->isRegister() && forLSRA) {
                         // The fixed range covers the current instruction so the
                         // interval for the virtual register starts at the next
                         // instruction. If the next instruction has a fixed use,
                         // this can lead to unnecessary register moves. To avoid
                         // special handling for this, assert the next instruction
                         // has no fixed uses. defineFixed guarantees this by inserting
@@ -790,17 +798,20 @@ LiveRangeAllocator<VREG, forLSRA>::build
                         if (use->usedAtStart()) {
                             if (!IsInputReused(*ins, use))
                                 hasUseRegisterAtStart = true;
                         } else {
                             hasUseRegister = true;
                         }
                     }
 
-                    JS_ASSERT(!(hasUseRegister && hasUseRegisterAtStart));
+                    JS_ASSERT_IF(hasUnaliasedDouble() && hasFloat32Def && vregs[use].type() == LDefinition::DOUBLE,
+                                 !use->usedAtStart());
+                    JS_ASSERT_IF(hasMultiAlias() && hasDoubleDef && vregs[use].type() == LDefinition::FLOAT32,
+                                 !use->usedAtStart());
 #endif
 
                     // Don't treat RECOVERED_INPUT uses as keeping the vreg alive.
                     if (use->policy() == LUse::RECOVERED_INPUT)
                         continue;
 
                     CodePosition to;
                     if (forLSRA) {
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -1752,17 +1752,25 @@ LIRGenerator::visitToDouble(MToDouble *c
       case MIRType_Int32:
       {
         LInt32ToDouble *lir = new(alloc()) LInt32ToDouble(useRegister(opd));
         return define(lir, convert);
       }
 
       case MIRType_Float32:
       {
+        // Bug 1039993: this used to be useRegisterAtStart, and theoreticall, it
+        // should still be, however, there is a bug in LSRA's implementation of
+        // *AtStart, which is quite fundamental. This should be reverted when that
+        // is fixed, or lsra is deprecated.
+#if defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
+        LFloat32ToDouble *lir = new(alloc()) LFloat32ToDouble(useRegister(opd));
+#else
         LFloat32ToDouble *lir = new(alloc()) LFloat32ToDouble(useRegisterAtStart(opd));
+#endif
         return define(lir, convert);
       }
 
       case MIRType_Double:
         return redefine(convert, opd);
 
       default:
         // Objects might be effectful.