Title: [206763] trunk/Source/_javascript_Core
Revision
206763
Author
[email protected]
Date
2016-10-03 17:35:52 -0700 (Mon, 03 Oct 2016)

Log Message

GetMapBucket node should speculate on the type of its 'key' child
https://bugs.webkit.org/show_bug.cgi?id=161638

Reviewed by Filip Pizlo.

This eliminates type-check branches when we've already
proven the type of the incoming key. Also, it reduces
the branches we emit when type checking the bucket's key.

This is a 2-3% speedup on ES6SampleBench/Basic.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetMapBucket):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (206762 => 206763)


--- trunk/Source/_javascript_Core/ChangeLog	2016-10-04 00:32:41 UTC (rev 206762)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-10-04 00:35:52 UTC (rev 206763)
@@ -1,3 +1,23 @@
+2016-10-03  Saam Barati  <[email protected]>
+
+        GetMapBucket node should speculate on the type of its 'key' child
+        https://bugs.webkit.org/show_bug.cgi?id=161638
+
+        Reviewed by Filip Pizlo.
+
+        This eliminates type-check branches when we've already
+        proven the type of the incoming key. Also, it reduces
+        the branches we emit when type checking the bucket's key.
+
+        This is a 2-3% speedup on ES6SampleBench/Basic.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMapBucket):
+
 2016-10-03  Christopher Reid  <[email protected]>
 
         Offline asm should not output masm assembly when using a x86_64 asm backend

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (206762 => 206763)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2016-10-04 00:32:41 UTC (rev 206762)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2016-10-04 00:35:52 UTC (rev 206763)
@@ -1587,7 +1587,22 @@
                 fixEdge<SetObjectUse>(node->child1());
             else
                 RELEASE_ASSERT_NOT_REACHED();
-            fixEdge<UntypedUse>(node->child2());
+
+            if (node->child2()->shouldSpeculateBoolean())
+                fixEdge<BooleanUse>(node->child2());
+            else if (node->child2()->shouldSpeculateInt32())
+                fixEdge<Int32Use>(node->child2());
+            else if (node->child2()->shouldSpeculateSymbol())
+                fixEdge<SymbolUse>(node->child2());
+            else if (node->child2()->shouldSpeculateObject())
+                fixEdge<ObjectUse>(node->child2());
+            else if (node->child2()->shouldSpeculateString())
+                fixEdge<StringUse>(node->child2());
+            else if (node->child2()->shouldSpeculateCell())
+                fixEdge<CellUse>(node->child2());
+            else
+                fixEdge<UntypedUse>(node->child2());
+
             fixEdge<Int32Use>(node->child3());
             break;
 

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (206762 => 206763)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-10-04 00:32:41 UTC (rev 206762)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2016-10-04 00:35:52 UTC (rev 206763)
@@ -4717,7 +4717,7 @@
     }
     case GetMapBucket: {
         SpeculateCellOperand map(this, node->child1());
-        JSValueOperand key(this, node->child2());
+        JSValueOperand key(this, node->child2(), ManualOperandSpeculation);
         SpeculateInt32Operand hash(this, node->child3());
         GPRTemporary mask(this);
         GPRTemporary index(this);
@@ -4741,6 +4741,9 @@
         else
             RELEASE_ASSERT_NOT_REACHED();
 
+        if (node->child2().useKind() != UntypedUse)
+            speculate(node, node->child2());
+
         m_jit.loadPtr(MacroAssembler::Address(mapGPR, node->child1().useKind() == MapObjectUse ? JSMap::offsetOfHashMapImpl() : JSSet::offsetOfHashMapImpl()), bufferGPR);
         m_jit.load32(MacroAssembler::Address(bufferGPR, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfCapacity()), maskGPR);
         m_jit.loadPtr(MacroAssembler::Address(bufferGPR, HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::offsetOfBuffer()), bufferGPR);
@@ -4763,40 +4766,79 @@
         m_jit.load64(MacroAssembler::Address(bucketGPR, HashMapBucket<HashMapBucketDataKey>::offsetOfKey()), bucketGPR);
 
         // Perform Object.is()
-        done.append(m_jit.branch64(MacroAssembler::Equal, bucketGPR, keyGPR)); // They're definitely the same value, we found the bucket we were looking for!
-        auto _oneIsntCell_ = m_jit.branchIfNotCell(JSValueRegs(bucketGPR));
-        // first is a cell here.
-        loopAround.append(m_jit.branchIfNotCell(JSValueRegs(keyGPR)));
-        // Both are cells here.
-        loopAround.append(m_jit.branch8(JITCompiler::NotEqual,
-            JITCompiler::Address(bucketGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
-        // The first is a string here.
-        slowPathCases.append(m_jit.branch8(JITCompiler::Equal,
-            JITCompiler::Address(keyGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
-        // The first is a string, but the second is not, we continue to loop around.
-        loopAround.append(m_jit.jump());
+        switch (node->child2().useKind()) {
+        case BooleanUse:
+        case Int32Use:
+        case SymbolUse:
+        case ObjectUse: {
+            done.append(m_jit.branch64(MacroAssembler::Equal, bucketGPR, keyGPR)); // They're definitely the same value, we found the bucket we were looking for!
+            // Otherwise, loop around.
+            break;
+        }
+        case CellUse: {
+            done.append(m_jit.branch64(MacroAssembler::Equal, bucketGPR, keyGPR));
+            loopAround.append(m_jit.branchIfNotCell(JSValueRegs(bucketGPR)));
+            loopAround.append(m_jit.branch8(JITCompiler::NotEqual,
+                JITCompiler::Address(bucketGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
+            loopAround.append(m_jit.branch8(JITCompiler::NotEqual,
+                JITCompiler::Address(keyGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
+            // They're both strings.
+            slowPathCases.append(m_jit.jump());
+            break;
+        }
+        case StringUse: {
+            done.append(m_jit.branch64(MacroAssembler::Equal, bucketGPR, keyGPR)); // They're definitely the same value, we found the bucket we were looking for!
+            loopAround.append(m_jit.branchIfNotCell(JSValueRegs(bucketGPR)));
+            loopAround.append(m_jit.branch8(JITCompiler::NotEqual,
+                JITCompiler::Address(bucketGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
+            slowPathCases.append(m_jit.jump());
+            break;
+        }
+        case UntypedUse: { 
+            done.append(m_jit.branch64(MacroAssembler::Equal, bucketGPR, keyGPR)); // They're definitely the same value, we found the bucket we were looking for!
+            auto _oneIsntCell_ = m_jit.branchIfNotCell(JSValueRegs(bucketGPR));
+            // first is a cell here.
+            loopAround.append(m_jit.branchIfNotCell(JSValueRegs(keyGPR)));
+            // Both are cells here.
+            loopAround.append(m_jit.branch8(JITCompiler::NotEqual,
+                JITCompiler::Address(bucketGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
+            // The first is a string here.
+            slowPathCases.append(m_jit.branch8(JITCompiler::Equal,
+                JITCompiler::Address(keyGPR, JSCell::typeInfoTypeOffset()), TrustedImm32(StringType)));
+            // The first is a string, but the second is not, we continue to loop around.
+            loopAround.append(m_jit.jump());
 
-        oneIsntCell.link(&m_jit);
-        // We've already done a 64-bit compare at this point, so if one is not a number, they're definitely not equal.
-        loopAround.append(m_jit.branchIfNotNumber(bucketGPR));
-        loopAround.append(m_jit.branchIfNotNumber(keyGPR));
-        // Both are definitely numbers. If we see a double, we go to the slow path.
-        slowPathCases.append(m_jit.branchIfNotInt32(bucketGPR));
-        slowPathCases.append(m_jit.branchIfNotInt32(keyGPR));
-        
-        loopAround.link(&m_jit);
+            oneIsntCell.link(&m_jit);
+            // We've already done a 64-bit compare at this point, so if one is not a number, they're definitely not equal.
+            loopAround.append(m_jit.branchIfNotNumber(bucketGPR));
+            loopAround.append(m_jit.branchIfNotNumber(keyGPR));
+            // Both are definitely numbers. If we see a double, we go to the slow path.
+            slowPathCases.append(m_jit.branchIfNotInt32(bucketGPR));
+            slowPathCases.append(m_jit.branchIfNotInt32(keyGPR));
+            break;
+        }
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
+
+            
+        if (!loopAround.empty())
+            loopAround.link(&m_jit);
+
         m_jit.add32(TrustedImm32(1), indexGPR);
         m_jit.jump().linkTo(loop, &m_jit);
 
-        slowPathCases.link(&m_jit);
-        silentSpillAllRegisters(indexGPR);
-        if (node->child1().useKind() == MapObjectUse)
-            callOperation(operationJSMapFindBucket, resultGPR, mapGPR, keyGPR, hashGPR);
-        else
-            callOperation(operationJSSetFindBucket, resultGPR, mapGPR, keyGPR, hashGPR);
-        silentFillAllRegisters(indexGPR);
-        m_jit.exceptionCheck();
-        done.append(m_jit.jump());
+        if (!slowPathCases.empty()) {
+            slowPathCases.link(&m_jit);
+            silentSpillAllRegisters(indexGPR);
+            if (node->child1().useKind() == MapObjectUse)
+                callOperation(operationJSMapFindBucket, resultGPR, mapGPR, keyGPR, hashGPR);
+            else
+                callOperation(operationJSSetFindBucket, resultGPR, mapGPR, keyGPR, hashGPR);
+            silentFillAllRegisters(indexGPR);
+            m_jit.exceptionCheck();
+            done.append(m_jit.jump());
+        }
 
         notPresentInTable.link(&m_jit);
         m_jit.move(TrustedImmPtr(nullptr), resultGPR);

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (206762 => 206763)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-10-04 00:32:41 UTC (rev 206762)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2016-10-04 00:35:52 UTC (rev 206763)
@@ -6509,14 +6509,6 @@
         LBasicBlock notPresentInTable = m_out.newBlock();
         LBasicBlock notEmptyValue = m_out.newBlock();
         LBasicBlock notDeletedValue = m_out.newBlock();
-        LBasicBlock notBitEqual = m_out.newBlock();
-        LBasicBlock bucketKeyNotCell = m_out.newBlock();
-        LBasicBlock bucketKeyIsCell = m_out.newBlock();
-        LBasicBlock bothAreCells = m_out.newBlock();
-        LBasicBlock bucketKeyIsString = m_out.newBlock();
-        LBasicBlock bucketKeyIsNumber = m_out.newBlock();
-        LBasicBlock bothAreNumbers = m_out.newBlock();
-        LBasicBlock bucketKeyIsInt32 = m_out.newBlock();
         LBasicBlock continuation = m_out.newBlock();
 
         LBasicBlock lastNext = m_out.insertNewBlocksBefore(loopStart);
@@ -6529,7 +6521,10 @@
         else
             RELEASE_ASSERT_NOT_REACHED();
 
-        LValue key = lowJSValue(m_node->child2());
+        LValue key = lowJSValue(m_node->child2(), ManualOperandSpeculation);
+        if (m_node->child2().useKind() != UntypedUse)
+            speculate(m_node->child2());
+
         LValue hash = lowInt32(m_node->child3());
 
         LValue hashMapImpl = m_out.loadPtr(map, m_node->child1().useKind() == MapObjectUse ? m_heaps.JSMap_hashMapImpl : m_heaps.JSSet_hashMapImpl);
@@ -6551,44 +6546,106 @@
         m_out.branch(m_out.equal(hashMapBucket, m_out.constIntPtr(HashMapImpl<HashMapBucket<HashMapBucketDataKey>>::deletedValue())),
             unsure(loopAround), unsure(notDeletedValue));
 
-        m_out.appendTo(notDeletedValue, notBitEqual);
+        m_out.appendTo(notDeletedValue, loopAround);
         LValue bucketKey = m_out.load64(hashMapBucket, m_heaps.HashMapBucket_key);
+
         // Perform Object.is()
-        m_out.branch(m_out.equal(key, bucketKey),
-            unsure(continuation), unsure(notBitEqual));
+        switch (m_node->child2().useKind()) {
+        case BooleanUse:
+        case Int32Use:
+        case SymbolUse:
+        case ObjectUse: {
+            m_out.branch(m_out.equal(key, bucketKey),
+                unsure(continuation), unsure(loopAround));
+            break;
+        }
+        case StringUse: {
+            LBasicBlock notBitEqual = m_out.newBlock();
+            LBasicBlock bucketKeyIsCell = m_out.newBlock();
 
-        m_out.appendTo(notBitEqual, bucketKeyIsCell);
-        m_out.branch(isCell(bucketKey),
-            unsure(bucketKeyIsCell), unsure(bucketKeyNotCell));
+            m_out.branch(m_out.equal(key, bucketKey),
+                unsure(continuation), unsure(notBitEqual));
 
-        m_out.appendTo(bucketKeyIsCell, bothAreCells);
-        m_out.branch(isCell(key),
-            unsure(bothAreCells), unsure(loopAround));
+            m_out.appendTo(notBitEqual, bucketKeyIsCell);
+            m_out.branch(isCell(bucketKey),
+                unsure(bucketKeyIsCell), unsure(loopAround));
 
-        m_out.appendTo(bothAreCells, bucketKeyIsString);
-        m_out.branch(isString(bucketKey),
-            unsure(bucketKeyIsString), unsure(loopAround));
+            m_out.appendTo(bucketKeyIsCell, loopAround);
+            m_out.branch(isString(bucketKey),
+                unsure(slowPath), unsure(loopAround));
+            break;
+        }
+        case CellUse: {
+            LBasicBlock notBitEqual = m_out.newBlock();
+            LBasicBlock bucketKeyIsCell = m_out.newBlock();
+            LBasicBlock bucketKeyIsString = m_out.newBlock();
 
-        m_out.appendTo(bucketKeyIsString, bucketKeyNotCell);
-        m_out.branch(isString(key),
-            unsure(slowPath), unsure(loopAround));
+            m_out.branch(m_out.equal(key, bucketKey),
+                unsure(continuation), unsure(notBitEqual));
 
-        m_out.appendTo(bucketKeyNotCell, bucketKeyIsNumber);
-        m_out.branch(isNotNumber(bucketKey),
-            unsure(loopAround), unsure(bucketKeyIsNumber));
+            m_out.appendTo(notBitEqual, bucketKeyIsCell);
+            m_out.branch(isCell(bucketKey),
+                unsure(bucketKeyIsCell), unsure(loopAround));
 
-        m_out.appendTo(bucketKeyIsNumber, bothAreNumbers);
-        m_out.branch(isNotNumber(key),
-            unsure(loopAround), unsure(bothAreNumbers));
+            m_out.appendTo(bucketKeyIsCell, bucketKeyIsString);
+            m_out.branch(isString(bucketKey),
+                unsure(bucketKeyIsString), unsure(loopAround));
 
-        m_out.appendTo(bothAreNumbers, bucketKeyIsInt32);
-        m_out.branch(isNotInt32(bucketKey),
-            unsure(slowPath), unsure(bucketKeyIsInt32));
+            m_out.appendTo(bucketKeyIsString, loopAround);
+            m_out.branch(isString(key),
+                unsure(slowPath), unsure(loopAround));
+            break;
+        }
+        case UntypedUse: {
+            LBasicBlock notBitEqual = m_out.newBlock();
+            LBasicBlock bucketKeyIsCell = m_out.newBlock();
+            LBasicBlock bothAreCells = m_out.newBlock();
+            LBasicBlock bucketKeyIsString = m_out.newBlock();
+            LBasicBlock bucketKeyNotCell = m_out.newBlock();
+            LBasicBlock bucketKeyIsNumber = m_out.newBlock();
+            LBasicBlock bothAreNumbers = m_out.newBlock();
+            LBasicBlock bucketKeyIsInt32 = m_out.newBlock();
 
-        m_out.appendTo(bucketKeyIsInt32, loopAround);
-        m_out.branch(isNotInt32(key),
-            unsure(slowPath), unsure(loopAround));
+            m_out.branch(m_out.equal(key, bucketKey),
+                unsure(continuation), unsure(notBitEqual));
 
+            m_out.appendTo(notBitEqual, bucketKeyIsCell);
+            m_out.branch(isCell(bucketKey),
+                unsure(bucketKeyIsCell), unsure(bucketKeyNotCell));
+
+            m_out.appendTo(bucketKeyIsCell, bothAreCells);
+            m_out.branch(isCell(key),
+                unsure(bothAreCells), unsure(loopAround));
+
+            m_out.appendTo(bothAreCells, bucketKeyIsString);
+            m_out.branch(isString(bucketKey),
+                unsure(bucketKeyIsString), unsure(loopAround));
+
+            m_out.appendTo(bucketKeyIsString, bucketKeyNotCell);
+            m_out.branch(isString(key),
+                unsure(slowPath), unsure(loopAround));
+
+            m_out.appendTo(bucketKeyNotCell, bucketKeyIsNumber);
+            m_out.branch(isNotNumber(bucketKey),
+                unsure(loopAround), unsure(bucketKeyIsNumber));
+
+            m_out.appendTo(bucketKeyIsNumber, bothAreNumbers);
+            m_out.branch(isNotNumber(key),
+                unsure(loopAround), unsure(bothAreNumbers));
+
+            m_out.appendTo(bothAreNumbers, bucketKeyIsInt32);
+            m_out.branch(isNotInt32(bucketKey),
+                unsure(slowPath), unsure(bucketKeyIsInt32));
+
+            m_out.appendTo(bucketKeyIsInt32, loopAround);
+            m_out.branch(isNotInt32(key),
+                unsure(slowPath), unsure(loopAround));
+            break;
+        }
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+        }
+
         m_out.appendTo(loopAround, slowPath);
         m_out.addIncomingToPhi(unmaskedIndex, m_out.anchor(m_out.add(index, m_out.int32One)));
         m_out.jump(loopStart);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to