Bug 957504: Fix mis-refactoring, and add some asserts to let debug users know that float32 can be broken (r=sunfish)
authorMarty Rosenberg <mrosenberg@mozilla.com>
Wed, 12 Feb 2014 08:41:59 -0500
changeset 168386 280aa953c868001373461d5fc4ed7a7b98474aa2
parent 168385 6245232c114d14f3b25dd54da74af80c28e5f7bb
child 168387 41b068af7b8dd6d146736f371e1c266a1ab00772
push id26203
push userryanvm@gmail.com
push dateWed, 12 Feb 2014 20:48:00 +0000
treeherdermozilla-central@18e7634d4094 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs957504
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 957504: Fix mis-refactoring, and add some asserts to let debug users know that float32 can be broken (r=sunfish)
js/src/jit/BaselineIC.cpp
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm/MacroAssembler-arm.h
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -1921,17 +1921,16 @@ ICCompare_Fallback::Compiler::generateSt
     masm.pushValue(R0);
     masm.pushValue(R1);
 
     // Push arguments.
     masm.pushValue(R1);
     masm.pushValue(R0);
     masm.push(BaselineStubReg);
     masm.pushBaselineFramePtr(BaselineFrameReg, R0.scratchReg());
-
     return tailCallVM(DoCompareFallbackInfo, masm);
 }
 
 //
 // Compare_String
 //
 
 bool
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -3488,16 +3488,17 @@ MacroAssemblerARMCompat::setupABICall(ui
     JS_ASSERT(!inCall_);
     inCall_ = true;
     args_ = args;
     passedArgs_ = 0;
     passedArgTypes_ = 0;
 #ifdef JS_CODEGEN_ARM_HARDFP
     usedIntSlots_ = 0;
     usedFloatSlots_ = 0;
+    usedFloat32_ = false;
     padding_ = 0;
 #else
     usedSlots_ = 0;
 #endif
     floatArgsInGPR[0] = MoveOperand();
     floatArgsInGPR[1] = MoveOperand();
     floatArgsInGPRValid[0] = false;
     floatArgsInGPRValid[1] = false;
@@ -3530,41 +3531,59 @@ MacroAssemblerARMCompat::passABIArg(cons
 {
     MoveOperand to;
     ++passedArgs_;
     if (!enoughMemory_)
         return;
     switch (type) {
       case MoveOp::FLOAT32:
       case MoveOp::DOUBLE: {
+        // N.B. this isn't a limitation of the ABI, it is a limitation of the compiler right now.
+        // There isn't a good way to handle odd numbered single registers, so everything goes to hell
+        // when we try.  Current fix is to never use more than one float in a function call.
+        // Fix coming along with complete float32 support in bug 957504.
+        JS_ASSERT(!usedFloat32_);
+        if (type == MoveOp::FLOAT32)
+            usedFloat32_ = true;
         FloatRegister fr;
         if (GetFloatArgReg(usedIntSlots_, usedFloatSlots_, &fr)) {
             if (from.isFloatReg() && from.floatReg() == fr) {
                 // Nothing to do; the value is in the right register already
+                usedFloatSlots_++;
+                if (type == MoveOp::FLOAT32)
+                    passedArgTypes_ = (passedArgTypes_ << ArgType_Shift) | ArgType_Float32;
+                else
+                    passedArgTypes_ = (passedArgTypes_ << ArgType_Shift) | ArgType_Double;
                 return;
             }
             to = MoveOperand(fr);
         } else {
             // If (and only if) the integer registers have started spilling, do we
             // need to take the register's alignment into account
-            uint32_t disp = GetFloatArgStackDisp(usedIntSlots_, usedFloatSlots_, &padding_);
+            uint32_t disp = INT_MAX;
+            if (type == MoveOp::FLOAT32)
+                disp = GetFloat32ArgStackDisp(usedIntSlots_, usedFloatSlots_, &padding_);
+            else
+                disp = GetDoubleArgStackDisp(usedIntSlots_, usedFloatSlots_, &padding_);
             to = MoveOperand(sp, disp);
         }
         usedFloatSlots_++;
         if (type == MoveOp::FLOAT32)
             passedArgTypes_ = (passedArgTypes_ << ArgType_Shift) | ArgType_Float32;
         else
             passedArgTypes_ = (passedArgTypes_ << ArgType_Shift) | ArgType_Double;
         break;
       }
       case MoveOp::GENERAL: {
         Register r;
         if (GetIntArgReg(usedIntSlots_, usedFloatSlots_, &r)) {
             if (from.isGeneralReg() && from.reg() == r) {
                 // Nothing to do; the value is in the right register already
+                usedIntSlots_++;
+                passedArgTypes_ = (passedArgTypes_ << ArgType_Shift) | ArgType_General;
                 return;
             }
             to = MoveOperand(r);
         } else {
             uint32_t disp = GetIntArgStackDisp(usedIntSlots_, usedFloatSlots_, &padding_);
             to = MoveOperand(sp, disp);
         }
         usedIntSlots_++;
--- a/js/src/jit/arm/MacroAssembler-arm.h
+++ b/js/src/jit/arm/MacroAssembler-arm.h
@@ -443,16 +443,17 @@ class MacroAssemblerARMCompat : public M
     // The actual number of arguments that were passed, used to assert that
     // the initial number of arguments declared was correct.
     uint32_t passedArgs_;
     uint32_t passedArgTypes_;
 
 #ifdef JS_CODEGEN_ARM_HARDFP
     uint32_t usedIntSlots_;
     uint32_t usedFloatSlots_;
+    bool usedFloat32_;
     uint32_t padding_;
 #else
     // ARM treats arguments as a vector in registers/memory, that looks like:
     // { r0, r1, r2, r3, [sp], [sp,+4], [sp,+8] ... }
     // usedSlots_ keeps track of how many of these have been used.
     // It bears a passing resemblance to passedArgs_, but a single argument
     // can effectively use between one and three slots depending on its size and
     // alignment requirements