compilePutByValForIntTypedArray() has a slow path in the middle of its processing
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Aug 2016 00:01:45 +0000 (00:01 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Aug 2016 00:01:45 +0000 (00:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160614

Reviewed by Keith Miller.

In compilePutByValForIntTypedArray() we were calling out to the slow path
operationToInt32() and then returning back to the middle of code to finish
the processing of writing the value to the array.  When we make the slow
path call, we trash any temporary registers that have been allocated.
In general slow path calls should finish the operation in progress and
continue processing at the beginning of the next node.

This was discovered while working on the register argument changes, when
we SpeculateStrictInt32Operand on the value child node.  That child node's
value was live in register with a spill format of DataFormatJSInt32.  In that
case we allocate a new temporary register and copy just the lower 32 bits from
the child register to the new temp register.  That temp register gets trashed
when we make the operationToInt32() slow path call.

I spent some time trying to devise a test with the current code base and wasn't
successful.  This case is tested with the register argument changes in progress.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204204 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

index 47cb368..9c1b204 100644 (file)
@@ -1,3 +1,30 @@
+2016-08-05  Michael Saboff  <msaboff@apple.com>
+
+        compilePutByValForIntTypedArray() has a slow path in the middle of its processing
+        https://bugs.webkit.org/show_bug.cgi?id=160614
+
+        Reviewed by Keith Miller.
+
+        In compilePutByValForIntTypedArray() we were calling out to the slow path
+        operationToInt32() and then returning back to the middle of code to finish
+        the processing of writing the value to the array.  When we make the slow
+        path call, we trash any temporary registers that have been allocated.
+        In general slow path calls should finish the operation in progress and
+        continue processing at the beginning of the next node.
+
+        This was discovered while working on the register argument changes, when
+        we SpeculateStrictInt32Operand on the value child node.  That child node's
+        value was live in register with a spill format of DataFormatJSInt32.  In that
+        case we allocate a new temporary register and copy just the lower 32 bits from
+        the child register to the new temp register.  That temp register gets trashed
+        when we make the operationToInt32() slow path call.
+
+        I spent some time trying to devise a test with the current code base and wasn't
+        successful.  This case is tested with the register argument changes in progress.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
+
 2016-08-05  Saam Barati  <sbarati@apple.com>
 
         Assertion failure when accessing TDZ variable in catch through eval
index ca0ad67..b0b8627 100644 (file)
@@ -2726,8 +2726,19 @@ void SpeculativeJIT::compilePutByValForIntTypedArray(GPRReg base, GPRReg propert
     Edge valueUse = m_jit.graph().varArgChild(node, 2);
     
     GPRTemporary value;
+#if USE(JSVALUE32_64)
+    GPRTemporary propertyTag;
+    GPRTemporary valueTag;
+#endif
+
     GPRReg valueGPR = InvalidGPRReg;
-    
+#if USE(JSVALUE32_64)
+    GPRReg propertyTagGPR = InvalidGPRReg;
+    GPRReg valueTagGPR = InvalidGPRReg;
+#endif
+
+    JITCompiler::JumpList slowPathCases;
+
     if (valueUse->isConstant()) {
         JSValue jsValue = valueUse->asJSValue();
         if (!jsValue.isNumber()) {
@@ -2798,20 +2809,36 @@ void SpeculativeJIT::compilePutByValForIntTypedArray(GPRReg base, GPRReg propert
                 value.adopt(result);
                 valueGPR = gpr;
             } else {
+#if USE(JSVALUE32_64)
+                GPRTemporary realPropertyTag(this);
+                propertyTag.adopt(realPropertyTag);
+                propertyTagGPR = propertyTag.gpr();
+
+                GPRTemporary realValueTag(this);
+                valueTag.adopt(realValueTag);
+                valueTagGPR = valueTag.gpr();
+#endif
                 SpeculateDoubleOperand valueOp(this, valueUse);
                 GPRTemporary result(this);
                 FPRReg fpr = valueOp.fpr();
                 GPRReg gpr = result.gpr();
                 MacroAssembler::Jump notNaN = m_jit.branchDouble(MacroAssembler::DoubleEqual, fpr, fpr);
                 m_jit.xorPtr(gpr, gpr);
-                MacroAssembler::Jump fixed = m_jit.jump();
+                MacroAssembler::JumpList fixed(m_jit.jump());
                 notNaN.link(&m_jit);
-                
-                MacroAssembler::Jump failed = m_jit.branchTruncateDoubleToInt32(
-                    fpr, gpr, MacroAssembler::BranchIfTruncateFailed);
-                
-                addSlowPathGenerator(slowPathCall(failed, this, operationToInt32, gpr, fpr, NeedToSpill, ExceptionCheckRequirement::CheckNotNeeded));
-                
+
+                fixed.append(m_jit.branchTruncateDoubleToInt32(
+                    fpr, gpr, MacroAssembler::BranchIfTruncateSuccessful));
+
+#if USE(JSVALUE64)
+                m_jit.or64(GPRInfo::tagTypeNumberRegister, property);
+                boxDouble(fpr, gpr);
+#else
+                m_jit.move(TrustedImm32(JSValue::Int32Tag), propertyTagGPR);
+                boxDouble(fpr, valueTagGPR, gpr);
+#endif
+                slowPathCases.append(m_jit.jump());
+
                 fixed.link(&m_jit);
                 value.adopt(result);
                 valueGPR = gpr;
@@ -2847,6 +2874,34 @@ void SpeculativeJIT::compilePutByValForIntTypedArray(GPRReg base, GPRReg propert
     JITCompiler::Jump done = jumpForTypedArrayIsNeuteredIfOutOfBounds(node, base, outOfBounds);
     if (done.isSet())
         done.link(&m_jit);
+
+    if (!slowPathCases.empty()) {
+#if USE(JSVALUE64)
+        if (node->op() == PutByValDirect) {
+            addSlowPathGenerator(slowPathCall(
+                slowPathCases, this,
+                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectStrict : operationPutByValDirectNonStrict,
+                NoResult, base, property, valueGPR));
+        } else {
+            addSlowPathGenerator(slowPathCall(
+                slowPathCases, this,
+                m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValStrict : operationPutByValNonStrict,
+                NoResult, base, property, valueGPR));
+        }
+#else // not USE(JSVALUE64)
+        if (node->op() == PutByValDirect) {
+            addSlowPathGenerator(slowPathCall(
+                slowPathCases, this,
+                m_jit.codeBlock()->isStrictMode() ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict,
+                NoResult, base, JSValueRegs(propertyTagGPR, property), JSValueRegs(valueTagGPR, valueGPR)));
+        } else {
+            addSlowPathGenerator(slowPathCall(
+                slowPathCases, this,
+                m_jit.codeBlock()->isStrictMode() ? operationPutByValCellStrict : operationPutByValCellNonStrict,
+                NoResult, base, JSValueRegs(propertyTagGPR, property), JSValueRegs(valueTagGPR, valueGPR)));
+        }
+#endif
+    }
     noResult(node);
 }