Changeset 161072 in webkit
- Timestamp:
- Dec 25, 2013 3:44:57 PM (10 years ago)
- Location:
- trunk
- Files:
-
- 3 added
- 11 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r161067 r161072 1 2013-12-25 Filip Pizlo <fpizlo@apple.com> 2 3 DFG PhantomArguments shouldn't rely on a dead Phi graph 4 https://bugs.webkit.org/show_bug.cgi?id=126218 5 6 Reviewed by Oliver Hunt. 7 8 Added a test for an obvious case that I don't think we had coverage for in 9 microbenchmarks. Of course, this case was already covered by more complex tests. 10 11 * js/regress/inline-arguments-aliased-access-expected.txt: Added. 12 * js/regress/inline-arguments-aliased-access.html: Added. 13 * js/regress/script-tests/inline-arguments-aliased-access.js: Added. 14 (foo): 15 (bar): 16 1 17 2013-12-25 Dirk Schulze <krit@webkit.org> 2 18 -
trunk/Source/JavaScriptCore/ChangeLog
r161033 r161072 1 2013-12-25 Filip Pizlo <fpizlo@apple.com> 2 3 DFG PhantomArguments shouldn't rely on a dead Phi graph 4 https://bugs.webkit.org/show_bug.cgi?id=126218 5 6 Reviewed by Oliver Hunt. 7 8 This change dramatically rationalizes our handling of PhantomArguments (i.e. 9 speculative elision of arguments object allocation). 10 11 It's now the case that if we decide that we can elide arguments allocation, we just 12 turn the arguments-creating node into a PhantomArguments and mark all locals that 13 it's stored to as being arguments aliases. Being an arguments alias and being a 14 PhantomArguments means basically the same thing: in DFG execution you have the empty 15 value, on OSR exit an arguments object is allocated in your place, and all operations 16 that use the value now just refer directly to the actual arguments in the call frame 17 header (or the arguments we know that we passed to the call, in case of inlining). 18 19 This means that we no longer have arguments simplification creating a dead Phi graph 20 that then has to be interpreted by the OSR exit logic. That sort of never made any 21 sense. 22 23 This means that PhantomArguments now has a clear story in SSA: basically SSA just 24 gets rid of the "locals" but everything else is the same. 25 26 Finally, this means that we can more easily get rid of forward exiting. As I was 27 working on the code to get rid of forward exiting, I realized that I'd have to 28 carefully preserve the special meanings of MovHint and SetLocal in the case of 29 PhantomArguments. It was really bizarre: even the semantics of MovHint were tied to 30 our specific treatment of PhantomArguments. After this change this is no longer the 31 case. 32 33 One of the really cool things about this change is that arguments reification now 34 just becomes a special kind of FlushFormat. This further unifies things: it means 35 that a MovHint(PhantomArguments) and a SetLocal(PhantomArguments) both have the same 36 meaning, since both of them dictate that the way we recover the local on exit is by 37 reifying arguments. Previously, the SetLocal(PhantomArguments) case needed some 38 special handling to accomplish this. 39 40 A downside of this approach is that we will now emit code to store the empty value 41 into aliased arguments variables, and we will even emit code to load that empty value 42 as well. As far as I can tell this doesn't cost anything, since PhantomArguments are 43 most profitable in cases where it allows us to simplify control flow and kill the 44 arguments locals entirely. Of course, this isn't an issue in SSA form since SSA form 45 also eliminates the locals. 46 47 * dfg/DFGArgumentsSimplificationPhase.cpp: 48 (JSC::DFG::ArgumentsSimplificationPhase::run): 49 (JSC::DFG::ArgumentsSimplificationPhase::detypeArgumentsReferencingPhantomChild): 50 * dfg/DFGFlushFormat.cpp: 51 (WTF::printInternal): 52 * dfg/DFGFlushFormat.h: 53 (JSC::DFG::resultFor): 54 (JSC::DFG::useKindFor): 55 (JSC::DFG::dataFormatFor): 56 * dfg/DFGSpeculativeJIT.cpp: 57 (JSC::DFG::SpeculativeJIT::compileCurrentBlock): 58 * dfg/DFGSpeculativeJIT32_64.cpp: 59 (JSC::DFG::SpeculativeJIT::compile): 60 * dfg/DFGSpeculativeJIT64.cpp: 61 (JSC::DFG::SpeculativeJIT::compile): 62 * dfg/DFGValueSource.h: 63 (JSC::DFG::ValueSource::ValueSource): 64 (JSC::DFG::ValueSource::forFlushFormat): 65 * dfg/DFGVariableAccessData.h: 66 (JSC::DFG::VariableAccessData::flushFormat): 67 * ftl/FTLLowerDFGToLLVM.cpp: 68 (JSC::FTL::LowerDFGToLLVM::buildExitArguments): 69 1 70 2013-12-23 Oliver Hunt <oliver@apple.com> 2 71 -
trunk/Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp
r159886 r161072 396 396 break; 397 397 398 ASSERT(!variableAccessData->isCaptured());399 400 // If this is a store into a VariableAccessData* that is marked as401 // arguments aliasing for an InlineCallFrame* that does not create402 // arguments, then flag the VariableAccessData as being an403 // arguments-aliased. This'll let the OSR exit machinery do the right404 // things. Note also that the SetLocal should become dead as soon as405 // we replace all uses of this variable with GetMyArgumentsLength and406 // GetMyArgumentByVal.407 ASSERT(m_argumentsAliasing.find(variableAccessData)->value.isValid());408 398 if (variableAccessData->mergeIsArgumentsAlias(true)) { 409 399 changed = true; … … 421 411 } 422 412 423 case PhantomLocal: {424 VariableAccessData* variableAccessData = node->variableAccessData();425 426 if (variableAccessData->isCaptured()427 || !m_argumentsAliasing.find(variableAccessData)->value.isValid()428 || m_createsArguments.contains(node->codeOrigin.inlineCallFrame))429 break;430 431 // Turn PhantomLocals into just GetLocals. This will preserve the threading432 // of the local through to this point, but will allow it to die, causing433 // only OSR to know about it.434 435 node->setOpAndDefaultFlags(GetLocal);436 break;437 }438 439 413 case Flush: { 440 414 VariableAccessData* variableAccessData = node->variableAccessData(); … … 460 434 // precisely what we don't want. 461 435 for (unsigned i = 0; i < AdjacencyList::Size; ++i) 462 removeArgumentsReferencingPhantomChild(node, i);436 detypeArgumentsReferencingPhantomChild(node, i); 463 437 break; 464 438 } … … 471 445 break; 472 446 node->convertToPhantom(); 473 node->children.setChild1(Edge());474 447 break; 475 448 } … … 489 462 break; 490 463 491 node->children.child1() = node->children.child2(); 492 node->children.child2() = Edge(); 464 insertionSet.insertNode( 465 indexInBlock, SpecNone, Phantom, node->codeOrigin, node->child1()); 466 467 node->child1() = node->child2(); 468 node->child2() = Edge(); 493 469 node->setOpAndDefaultFlags(GetMyArgumentByVal); 494 470 changed = true; … … 504 480 break; 505 481 506 node->children.child1() = Edge(); 482 insertionSet.insertNode( 483 indexInBlock, SpecNone, Phantom, node->codeOrigin, node->child1()); 484 485 node->child1() = Edge(); 507 486 node->setOpAndDefaultFlags(GetMyArgumentsLength); 508 487 changed = true; … … 581 560 codeOrigin); 582 561 insertionSet.insertNode( 583 indexInBlock, SpecNone, Phantom, codeOrigin, 584 children); 562 indexInBlock, SpecNone, Phantom, codeOrigin, children); 585 563 586 564 changed = true; … … 592 570 continue; 593 571 594 node->setOpAndDefaultFlags(Phantom); 595 node->children.reset(); 572 node->convertToPhantom(); 596 573 break; 597 574 } … … 628 605 } 629 606 607 for (BlockIndex blockIndex = 0; blockIndex < m_graph.numBlocks(); ++blockIndex) { 608 BasicBlock* block = m_graph.block(blockIndex); 609 if (!block) 610 continue; 611 for (unsigned indexInBlock = 0; indexInBlock < block->size(); ++indexInBlock) { 612 Node* node = block->at(indexInBlock); 613 if (node->op() != Phantom) 614 continue; 615 for (unsigned i = 0; i < AdjacencyList::Size; ++i) 616 detypeArgumentsReferencingPhantomChild(node, i); 617 } 618 } 619 630 620 if (changed) { 631 621 m_graph.dethread(); … … 765 755 } 766 756 767 void removeArgumentsReferencingPhantomChild(Node* node, unsigned edgeIndex)757 void detypeArgumentsReferencingPhantomChild(Node* node, unsigned edgeIndex) 768 758 { 769 759 Edge edge = node->children.child(edgeIndex); … … 772 762 773 763 switch (edge->op()) { 774 case Phi: // Arises if we had CSE on a GetLocal of the arguments register. 775 case GetLocal: // Arises if we had CSE on an arguments access to a variable aliased to the arguments. 776 case SetLocal: { // Arises if we had CSE on a GetLocal of the arguments register. 764 case GetLocal: { 777 765 VariableAccessData* variableAccessData = edge->variableAccessData(); 778 bool isDeadArgumentsRegister = 779 variableAccessData->local() == 780 m_graph.uncheckedArgumentsRegisterFor(edge->codeOrigin) 781 && !m_createsArguments.contains(edge->codeOrigin.inlineCallFrame); 782 bool isAliasedArgumentsRegister = 783 !variableAccessData->isCaptured() 784 && m_argumentsAliasing.find(variableAccessData)->value.isValid() 785 && !m_createsArguments.contains(edge->codeOrigin.inlineCallFrame); 786 if (!isDeadArgumentsRegister && !isAliasedArgumentsRegister) 766 if (!variableAccessData->isArgumentsAlias()) 787 767 break; 788 node->children. removeEdge(edgeIndex);768 node->children.child(edgeIndex).setUseKind(UntypedUse); 789 769 break; 790 770 } 791 771 792 case CreateArguments: { // Arises if we CSE two GetLocals to the arguments register and then CSE the second use of the GetLocal to the first. 793 if (m_createsArguments.contains(edge->codeOrigin.inlineCallFrame)) 794 break; 795 node->children.removeEdge(edgeIndex); 772 case PhantomArguments: { 773 node->children.child(edgeIndex).setUseKind(UntypedUse); 796 774 break; 797 775 } -
trunk/Source/JavaScriptCore/dfg/DFGFlushFormat.cpp
r159394 r161072 57 57 out.print("FlushedJSValue"); 58 58 return; 59 case FlushedArguments: 60 out.print("FlushedArguments"); 61 return; 59 62 case ConflictingFlush: 60 63 out.print("ConflictingFlush"); -
trunk/Source/JavaScriptCore/dfg/DFGFlushFormat.h
r159394 r161072 47 47 FlushedBoolean, 48 48 FlushedJSValue, 49 FlushedArguments, 49 50 ConflictingFlush 50 51 }; … … 57 58 case FlushedCell: 58 59 case ConflictingFlush: 60 case FlushedArguments: 59 61 return NodeResultJS; 60 62 case FlushedInt32: … … 77 79 case FlushedJSValue: 78 80 case ConflictingFlush: 81 case FlushedArguments: 79 82 return UntypedUse; 80 83 case FlushedCell: … … 111 114 case FlushedBoolean: 112 115 return DataFormatBoolean; 116 case FlushedArguments: 117 return DataFormatArguments; 113 118 } 114 119 RELEASE_ASSERT_NOT_REACHED(); -
trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
r160919 r161072 1437 1437 VariableAccessData* variable = node->variableAccessData(); 1438 1438 DataFormat format; 1439 if (variable->isArgumentsAlias()) 1440 format = DataFormatArguments; 1441 else if (!node->refCount()) 1439 if (!node->refCount()) 1442 1440 continue; // No need to record dead SetLocal's. 1443 1441 else -
trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
r160796 r161072 1890 1890 } 1891 1891 1892 case FlushedJSValue: { 1892 case FlushedJSValue: 1893 case FlushedArguments: { 1893 1894 GPRTemporary result(this); 1894 1895 GPRTemporary tag(this); … … 1976 1977 } 1977 1978 1978 case FlushedJSValue: { 1979 case FlushedJSValue: 1980 case FlushedArguments: { 1979 1981 if (generationInfoFromVirtualRegister(node->child1()->virtualRegister()).registerFormat() == DataFormatDouble) { 1980 1982 SpeculateDoubleOperand value(this, node->child1(), ManualOperandSpeculation); … … 1990 1992 noResult(node); 1991 1993 recordSetLocal(DataFormatJS); 1992 1993 // If we're storing an arguments object that has been optimized away,1994 // our variable event stream for OSR exit now reflects the optimized1995 // value (JSValue()). On the slow path, we want an arguments object1996 // instead. We add an additional move hint to show OSR exit that it1997 // needs to reconstruct the arguments object.1998 if (node->child1()->op() == PhantomArguments)1999 compileMovHint(node);2000 1994 break; 2001 1995 } -
trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
r160796 r161072 2300 2300 } 2301 2301 2302 case FlushedJSValue: { 2302 case FlushedJSValue: 2303 case FlushedArguments: { 2303 2304 JSValueOperand value(this, node->child1()); 2304 2305 m_jit.store64(value.gpr(), JITCompiler::addressFor(node->machineLocal())); … … 2306 2307 2307 2308 recordSetLocal(DataFormatJS); 2308 2309 // If we're storing an arguments object that has been optimized away,2310 // our variable event stream for OSR exit now reflects the optimized2311 // value (JSValue()). On the slow path, we want an arguments object2312 // instead. We add an additional move hint to show OSR exit that it2313 // needs to reconstruct the arguments object.2314 if (node->child1()->op() == PhantomArguments)2315 compileMovHint(node);2316 2309 break; 2317 2310 } -
trunk/Source/JavaScriptCore/dfg/DFGValueSource.h
r159394 r161072 123 123 : m_kind(valueSourceKind) 124 124 { 125 ASSERT(kind() == ArgumentsSource || kind() == SourceIsDead );125 ASSERT(kind() == ArgumentsSource || kind() == SourceIsDead || kind() == ArgumentsSource); 126 126 } 127 127 … … 160 160 case FlushedBoolean: 161 161 return ValueSource(BooleanInJSStack, where); 162 case FlushedArguments: 163 return ValueSource(ArgumentsSource); 162 164 } 163 165 RELEASE_ASSERT_NOT_REACHED(); -
trunk/Source/JavaScriptCore/dfg/DFGVariableAccessData.h
r160796 r161072 329 329 ASSERT(find() == this); 330 330 331 if (isArgumentsAlias()) 332 return FlushedArguments; 333 331 334 if (!shouldUnboxIfPossible()) 332 335 return FlushedJSValue; -
trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
r160587 r161072 4210 4210 exit.m_values[i] = ExitValue::inJSStackAsDouble(flush.virtualRegister()); 4211 4211 break; 4212 4213 case FlushedArguments: 4214 // FIXME: implement PhantomArguments. 4215 // https://bugs.webkit.org/show_bug.cgi?id=113986 4216 RELEASE_ASSERT_NOT_REACHED(); 4217 break; 4212 4218 } 4213 4219 }
Note: See TracChangeset
for help on using the changeset viewer.