Changeset 160205 in webkit for trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp

Timestamp:

Dec 5, 2013, 5:47:19 PM (11 years ago)

Author:

[email protected]

Message:

FTL should use cvttsd2si directly for double-to-int32 conversions
​https://bugs.webkit.org/show_bug.cgi?id=125275

Source/JavaScriptCore:

Reviewed by Michael Saboff.

Wow. This was an ordeal. Using cvttsd2si was actually easy, but I learned, and
sometimes even fixed, some interesting things:

• The llvm.x86.sse2.cvttsd2si intrinsic can actually result in LLVM emitting a vcvttsd2si. I guess the intrinsic doesn't actually imply the instruction.

• That whole thing about branchTruncateDoubleToUint32? Yeah we don't need that. It's better to use branchTruncateDoubleToInt32 instead. It has the right semantics for all of its callers (err, its one-and-only caller), and it's more likely to take fast path. This patch kills branchTruncateDoubleToUint32.

• "a[i] = v; v = a[i]". Does this change v? OK, assume that 'a[i]' is a pure-ish operation - like an array access with 'i' being an integer index and we're not having a bad time. Now does this change v? CSE assumes that it doesn't. That's wrong. If 'a' is a typed array - the most sensible and pure kind of array - then this can be a truncating cast. For example 'v' could be a double and 'a' could be an integer array.

• "v1 = a[i]; v2 = a[i]". Is v1 === v2 assuming that 'a[i]' is pure-ish? The answer is no. You could have a different arrayMode in each access. I know this sounds weird, but with concurrent JIT that might happen.

This patch adds tests for all of this stuff, except for the first issue (it's weird
but probably doesn't matter) and the last issue (it's too much of a freakshow).

• assembler/MacroAssemblerARM64.h:
• assembler/MacroAssemblerARMv7.h:
• assembler/MacroAssemblerX86Common.h:
• dfg/DFGCSEPhase.cpp:

(JSC::DFG::CSEPhase::getByValLoadElimination):
(JSC::DFG::CSEPhase::performNodeCSE):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):

• ftl/FTLAbbreviations.h:

(JSC::FTL::vectorType):
(JSC::FTL::getUndef):
(JSC::FTL::buildInsertElement):

• ftl/FTLIntrinsicRepository.h:
• ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::LowerDFGToLLVM::doubleToInt32):
(JSC::FTL::LowerDFGToLLVM::doubleToUInt32):
(JSC::FTL::LowerDFGToLLVM::sensibleDoubleToInt32):

• ftl/FTLOutput.h:

(JSC::FTL::Output::insertElement):
(JSC::FTL::Output::hasSensibleDoubleToInt):
(JSC::FTL::Output::sensibleDoubleToInt):

LayoutTests:

Reviewed by Michael Saboff.

• js/regress/double-to-int32-typed-array-expected.txt: Added.
• js/regress/double-to-int32-typed-array-no-inline-expected.txt: Added.
• js/regress/double-to-int32-typed-array-no-inline.html: Added.
• js/regress/double-to-int32-typed-array.html: Added.
• js/regress/double-to-uint32-typed-array-expected.txt: Added.
• js/regress/double-to-uint32-typed-array-no-inline-expected.txt: Added.
• js/regress/double-to-uint32-typed-array-no-inline.html: Added.
• js/regress/double-to-uint32-typed-array.html: Added.
• js/regress/script-tests/double-to-int32-typed-array-no-inline.js: Added.

(foo):
(test):

• js/regress/script-tests/double-to-int32-typed-array.js: Added.

(foo):
(test):

• js/regress/script-tests/double-to-uint32-typed-array-no-inline.js: Added.

(foo):
(test):

• js/regress/script-tests/double-to-uint32-typed-array.js: Added.

(foo):
(test):

File:

• trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp

@@ -332 +332 @@
     }
     
-    Node* getByValLoadElimination(Node* child1, Node* child2)
+    Node* getByValLoadElimination(Node* child1, Node* child2, ArrayMode arrayMode)
     {
         for (unsigned i = m_indexInBlock; i--;) {
@@ -343 +343 @@
                 if (!m_graph.byValIsPure(node))
                     return 0;
-                if (node->child1() == child1 && node->child2() == child2)
+                if (node->child1() == child1
+                    && node->child2() == child2
+                    && node->arrayMode().type() == arrayMode.type())
                     return node;
                 break;
@@ -352 +354 @@
                 if (!m_graph.byValIsPure(node))
                     return 0;
-                if (m_graph.varArgChild(node, 0) == child1 && m_graph.varArgChild(node, 1) == child2)
+                // Typed arrays 
+                if (arrayMode.typedArrayType() != NotTypedArray)
+                    return 0;
+                if (m_graph.varArgChild(node, 0) == child1
+                    && m_graph.varArgChild(node, 1) == child2
+                    && node->arrayMode().type() == arrayMode.type())
                     return m_graph.varArgChild(node, 2).node();
                 // We must assume that the PutByVal will clobber the location we're getting from.
@@ -1217 +1224 @@
                 break;
             if (m_graph.byValIsPure(node))
-                setReplacement(getByValLoadElimination(node->child1().node(), node->child2().node()));
+                setReplacement(getByValLoadElimination(node->child1().node(), node->child2().node(), node->arrayMode()));
             break;
                 
@@ -1227 +1234 @@
             Edge child2 = m_graph.varArgChild(node, 1);
             if (node->arrayMode().canCSEStorage()) {
-                Node* replacement = getByValLoadElimination(child1.node(), child2.node());
+                Node* replacement = getByValLoadElimination(child1.node(), child2.node(), node->arrayMode());
                 if (!replacement)
                     break;