Changeset 163070 in webkit for trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

Timestamp:

Jan 29, 2014, 11:30:24 PM (11 years ago)

Author:

[email protected]

Message:

Merge final changesets from the jsCStack branch (r162969, r162975, r162992, r163004, r163069).

2014-01-29 Filip Pizlo <​[email protected]>

DFG ArrayPop double array mishandles the NaN hole installation
​https://bugs.webkit.org/show_bug.cgi?id=127813

Reviewed by Mark Rowe.

Our object model for arrays inferred double dictates that we use quiet NaN (QNaN) to
mark holes. Holes, in this context, are any entries in the allocated array buffer
(i.e. from index 0 up to the vectorLength) that don't currently hold a value. Popping
creates a hole, since it deletes the value at publicLength - 1.

But, because of some sloppy copy-and-paste, we were storing (int64_t)0 when creating
the hole, instead of storing QNaN. That's likely because for other kinds of arrays,
64-bit zero is the hole marker, instead of QNaN.

The attached test case illustrates the problem. In the LLInt and Baseline JIT, the
result returned from foo() is "1.5,2.5_{4.5", since array.pop() removes 3.5 and
replaces it with a hole and then the assignment "array[3] = 4.5" creates an element
just beyond that hole. But, once we tier-up to the DFG, the result previously became
"1.5,2.5,0,4.5", which is wrong. The 0 appeared because the IEEE double
interpretation of 64-bit zero is simply zero.}

This patch fixes that problem. Now the DFG agrees with the other engines.

This patch also fixes style. For some reason that copy-pasted code wasn't even
indented correctly.

• dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile):
• tests/stress/array-pop-double-hole.js: Added. (foo):

2014-01-28 Filip Pizlo <​[email protected]>

FTL should support ArrayPush
​https://bugs.webkit.org/show_bug.cgi?id=127748

Not reviewed, remove some debug code.

• ftl/FTLLowerDFGToLLVM.cpp: (JSC::FTL::LowerDFGToLLVM::compileArrayPush):

2014-01-27 Filip Pizlo <​[email protected]>

FTL should support ArrayPush
​https://bugs.webkit.org/show_bug.cgi?id=127748

Reviewed by Oliver Hunt.

• ftl/FTLAbstractHeapRepository.h: (JSC::FTL::AbstractHeapRepository::forArrayType):
• ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile):
• ftl/FTLIntrinsicRepository.h:
• ftl/FTLLowerDFGToLLVM.cpp: (JSC::FTL::LowerDFGToLLVM::compileNode): (JSC::FTL::LowerDFGToLLVM::compileArrayPush):
• tests/stress/array-push-contiguous.js: Added. (foo):
• tests/stress/array-push-double.js: Added. (foo):

2014-01-28 Filip Pizlo <​[email protected]>

FTL should support ArrayPop
​https://bugs.webkit.org/show_bug.cgi?id=127749

Reviewed by Geoffrey Garen.

• ftl/FTLCapabilities.cpp: (JSC::FTL::canCompile):
• ftl/FTLIntrinsicRepository.h:
• ftl/FTLLowerDFGToLLVM.cpp: (JSC::FTL::LowerDFGToLLVM::compileNode): (JSC::FTL::LowerDFGToLLVM::compileArrayPush): (JSC::FTL::LowerDFGToLLVM::compileArrayPop):
• tests/stress/array-pop-contiguous.js: Added. (foo):
• tests/stress/array-pop-double.js: Added. (foo):
• tests/stress/array-pop-int32.js: Added. (foo):

File:

• trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

@@ -416 +416 @@
         case PutByValDirect:
             compilePutByVal();
+            break;
+        case ArrayPush:
+            compileArrayPush();
+            break;
+        case ArrayPop:
+            compileArrayPop();
             break;
         case NewObject:
@@ -2182 +2188 @@
     }
     
+    void compileArrayPush()
+    {
+        LValue base = lowCell(m_node->child1());
+        LValue storage = lowStorage(m_node->child3());
+        
+        switch (m_node->arrayMode().type()) {
+        case Array::Int32:
+        case Array::Contiguous:
+        case Array::Double: {
+            LValue value;
+            LType refType;
+            
+            if (m_node->arrayMode().type() != Array::Double) {
+                value = lowJSValue(m_node->child2(), ManualOperandSpeculation);
+                if (m_node->arrayMode().type() == Array::Int32) {
+                    FTL_TYPE_CHECK(
+                        jsValueValue(value), m_node->child2(), SpecInt32, isNotInt32(value));
+                }
+                refType = m_out.ref64;
+            } else {
+                value = lowDouble(m_node->child2());
+                FTL_TYPE_CHECK(
+                    doubleValue(value), m_node->child2(), SpecFullRealNumber,
+                    m_out.doubleNotEqualOrUnordered(value, value));
+                refType = m_out.refDouble;
+            }
+            
+            IndexedAbstractHeap& heap = m_heaps.forArrayType(m_node->arrayMode().type());
+
+            LValue prevLength = m_out.load32(storage, m_heaps.Butterfly_publicLength);
+            
+            LBasicBlock fastPath = FTL_NEW_BLOCK(m_out, ("ArrayPush fast path"));
+            LBasicBlock slowPath = FTL_NEW_BLOCK(m_out, ("ArrayPush slow path"));
+            LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("ArrayPush continuation"));
+            
+            m_out.branch(
+                m_out.aboveOrEqual(
+                    prevLength, m_out.load32(storage, m_heaps.Butterfly_vectorLength)),
+                slowPath, fastPath);
+            
+            LBasicBlock lastNext = m_out.appendTo(fastPath, slowPath);
+            m_out.store(
+                value,
+                m_out.baseIndex(heap, storage, m_out.zeroExt(prevLength, m_out.intPtr)),
+                refType);
+            LValue newLength = m_out.add(prevLength, m_out.int32One);
+            m_out.store32(newLength, storage, m_heaps.Butterfly_publicLength);
+            
+            ValueFromBlock fastResult = m_out.anchor(boxInt32(newLength));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(slowPath, continuation);
+            LValue operation;
+            if (m_node->arrayMode().type() != Array::Double)
+                operation = m_out.operation(operationArrayPush);
+            else
+                operation = m_out.operation(operationArrayPushDouble);
+            ValueFromBlock slowResult = m_out.anchor(
+                vmCall(operation, m_callFrame, value, base));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(continuation, lastNext);
+            setJSValue(m_out.phi(m_out.int64, fastResult, slowResult));
+            return;
+        }
+            
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+            return;
+        }
+    }
+    
+    void compileArrayPop()
+    {
+        LValue base = lowCell(m_node->child1());
+        LValue storage = lowStorage(m_node->child2());
+        
+        switch (m_node->arrayMode().type()) {
+        case Array::Int32:
+        case Array::Double:
+        case Array::Contiguous: {
+            IndexedAbstractHeap& heap = m_heaps.forArrayType(m_node->arrayMode().type());
+            
+            LBasicBlock fastCase = FTL_NEW_BLOCK(m_out, ("ArrayPop fast case"));
+            LBasicBlock slowCase = FTL_NEW_BLOCK(m_out, ("ArrayPop slow case"));
+            LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("ArrayPop continuation"));
+            
+            LValue prevLength = m_out.load32(storage, m_heaps.Butterfly_publicLength);
+            
+            Vector<ValueFromBlock, 3> results;
+            results.append(m_out.anchor(m_out.constInt64(JSValue::encode(jsUndefined()))));
+            m_out.branch(m_out.isZero32(prevLength), continuation, fastCase);
+            
+            LBasicBlock lastNext = m_out.appendTo(fastCase, slowCase);
+            LValue newLength = m_out.sub(prevLength, m_out.int32One);
+            m_out.store32(newLength, storage, m_heaps.Butterfly_publicLength);
+            TypedPointer pointer = m_out.baseIndex(
+                heap, storage, m_out.zeroExt(newLength, m_out.intPtr));
+            if (m_node->arrayMode().type() != Array::Double) {
+                LValue result = m_out.load64(pointer);
+                m_out.store64(m_out.int64Zero, pointer);
+                results.append(m_out.anchor(result));
+                m_out.branch(m_out.notZero64(result), continuation, slowCase);
+            } else {
+                LValue result = m_out.loadDouble(pointer);
+                m_out.store64(m_out.constInt64(bitwise_cast<int64_t>(QNaN)), pointer);
+                results.append(m_out.anchor(boxDouble(result)));
+                m_out.branch(m_out.doubleEqual(result, result), continuation, slowCase);
+            }
+            
+            m_out.appendTo(slowCase, continuation);
+            results.append(m_out.anchor(vmCall(
+                m_out.operation(operationArrayPopAndRecoverLength), m_callFrame, base)));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(continuation, lastNext);
+            setJSValue(m_out.phi(m_out.int64, results));
+            return;
+        }
+
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+            return;
+        }
+    }
+    
     void compileNewObject()
     {