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

Timestamp:

Jan 30, 2014, 3:00:16 PM (11 years ago)

Author:

[email protected]

Message:

FTL should support GetById(Untyped:)
​https://bugs.webkit.org/show_bug.cgi?id=127750

Reviewed by Oliver Hunt.

This was supposed to be easy. Indeed, the actual GetById UntypedUse case was easy. But
then it expanded coverage by a lot and I got to deal with three bugs. So, this has
some additional changes:

Also make it safe for LLVM to duplicate calls to patchpoints and stackmaps. Previously
we incorrectly assumed that if we emitted a patchpoint, then there would only be one
copy of that patchpoint (with that ID) in the resulting machine code and in the
stackmaps section. That's obviously a bad assumption - LLVM is allowed to do anything
it wants so long as the outcome of executing the code has a semantically equivalent
meaning to the IR we gave it, and duplicating code is trivially OK under this rule. We
should be OK with it, too. The solution is to add Vectors in a bunch of places that
previously just thought they only had one value. For example, an InlineCacheDescriptor
now has a Vector of generators - one generator for each copy that LLVM stamped out.
Normally there will only be one copy, of course - since duplication is usually
unprofitable. But, if LLVM decides that copying would be groovy then we will no longer
barf.

Also fix SSA conversion. It turns out that we mishandled the case where a block had
multiple Phi functions for the same local. If any of those CPS Phis fail to trivialize
in the Aycock-Horspool fixpoint, we need to insert an SSA Phi. Previously, it was
assuming that so long as the head CPS Phi was trivial, we could forego SSA Phi
insertion. That's wrong if the head CPS Phi trivialized but ended up pointing to a
non-trivial CPS Phi in the same block. This madness with trees of Phis occurs because
we try to save on compile times: no Phi ever has more than three children even if the
block has more than three predecessors; we just build out a tree of Phis to satisfy
all predecessors. So weird.

And finally, fix DFG->FTL OSR entry's reconstruction of 'this' in a constructor. That
reconstruction code, JITCode::reconstruct(), had a work-around for the case where we
were entering into a constructor at the prologue. In that case, 'this' is definitely
unavailable. But the OSR code does reconstructions at LoopHints, which aren't at the
prologue, and so 'this' should totally be available.

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::dump):

• dfg/DFGJITCode.cpp:

(JSC::DFG::JITCode::reconstruct):

• dfg/DFGNode.h:

(JSC::DFG::Node::tryGetVariableAccessData):

• dfg/DFGSSAConversionPhase.cpp:

(JSC::DFG::SSAConversionPhase::run):

• ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

• ftl/FTLCompile.cpp:

(JSC::FTL::generateICFastPath):
(JSC::FTL::fixFunctionBasedOnStackMaps):

• ftl/FTLInlineCacheDescriptor.h:
• ftl/FTLJITFinalizer.cpp:

(JSC::FTL::JITFinalizer::codeSize):

• ftl/FTLJSCall.cpp:

(JSC::FTL::JSCall::JSCall):

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

(JSC::FTL::LowerDFGToLLVM::compileGetById):
(JSC::FTL::LowerDFGToLLVM::getById):

• ftl/FTLOSREntry.cpp:

(JSC::FTL::prepareOSREntry):

• ftl/FTLStackMaps.cpp:

(JSC::FTL::StackMaps::getRecordMap):

• ftl/FTLStackMaps.h:
• tests/stress/get-by-id-untyped.js: Added.

(foo):

File:

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

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

@@ -1605 +1605 @@
     void compileGetById()
     {
-        // UntypedUse is a bit harder to reason about and I'm not sure how best to do it, yet.
-        // Basically we need to emit a cell branch that takes you to the slow path, but the slow
-        // path is generated by the IC generator so we can't jump to it from here. And the IC
-        // generator currently doesn't know how to emit such a branch. So, for now, we just
-        // restrict this to CellUse.
-        ASSERT(m_node->child1().useKind() == CellUse);
-
-        LValue base = lowCell(m_node->child1());
-        StringImpl* uid = m_graph.identifiers()[m_node->identifierNumber()];
-
-        // Arguments: id, bytes, target, numArgs, args...
-        unsigned stackmapID = m_stackmapIDs++;
-        
-        if (Options::verboseCompilation())
-            dataLog("    Emitting GetById patchpoint with stackmap #", stackmapID, "\n");
-        
-        LValue call = m_out.call(
-            m_out.patchpointInt64Intrinsic(),
-            m_out.constInt64(stackmapID), m_out.constInt32(sizeOfGetById()),
-            constNull(m_out.ref8), m_out.constInt32(1), base);
-        setInstructionCallingConvention(call, LLVMAnyRegCallConv);
-        setJSValue(call);
-        
-        m_ftlState.getByIds.append(GetByIdDescriptor(stackmapID, m_node->codeOrigin, uid));
+        // Pretty much the only reason why we don't also support GetByIdFlush is because:
+        // https://bugs.webkit.org/show_bug.cgi?id=125711
+        
+        switch (m_node->child1().useKind()) {
+        case CellUse: {
+            setJSValue(getById(lowCell(m_node->child1())));
+            return;
+        }
+            
+        case UntypedUse: {
+            // This is pretty weird, since we duplicate the slow path both here and in the
+            // code generated by the IC. We should investigate making this less bad.
+            // https://bugs.webkit.org/show_bug.cgi?id=127830
+            LValue value = lowJSValue(m_node->child1());
+            
+            LBasicBlock cellCase = FTL_NEW_BLOCK(m_out, ("GetById untyped cell case"));
+            LBasicBlock notCellCase = FTL_NEW_BLOCK(m_out, ("GetById untyped not cell case"));
+            LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("GetById untyped continuation"));
+            
+            m_out.branch(isCell(value), cellCase, notCellCase);
+            
+            LBasicBlock lastNext = m_out.appendTo(cellCase, notCellCase);
+            ValueFromBlock cellResult = m_out.anchor(getById(value));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(notCellCase, continuation);
+            ValueFromBlock notCellResult = m_out.anchor(vmCall(
+                m_out.operation(operationGetById),
+                m_callFrame, getUndef(m_out.intPtr), value,
+                m_out.constIntPtr(m_graph.identifiers()[m_node->identifierNumber()])));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(continuation, lastNext);
+            setJSValue(m_out.phi(m_out.int64, cellResult, notCellResult));
+            return;
+        }
+            
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+            return;
+        }
     }
     
@@ -3396 +3413 @@
     }
     
+    LValue getById(LValue base)
+    {
+        StringImpl* uid = m_graph.identifiers()[m_node->identifierNumber()];
+
+        // Arguments: id, bytes, target, numArgs, args...
+        unsigned stackmapID = m_stackmapIDs++;
+        
+        if (Options::verboseCompilation())
+            dataLog("    Emitting GetById patchpoint with stackmap #", stackmapID, "\n");
+        
+        LValue call = m_out.call(
+            m_out.patchpointInt64Intrinsic(),
+            m_out.constInt64(stackmapID), m_out.constInt32(sizeOfGetById()),
+            constNull(m_out.ref8), m_out.constInt32(1), base);
+        setInstructionCallingConvention(call, LLVMAnyRegCallConv);
+        
+        m_ftlState.getByIds.append(GetByIdDescriptor(stackmapID, m_node->codeOrigin, uid));
+        
+        return call;
+    }
+    
     TypedPointer baseIndex(IndexedAbstractHeap& heap, LValue storage, LValue index, Edge edge)
     {