Changeset 171190 in webkit
- Timestamp:
- Jul 17, 2014 11:17:43 AM (10 years ago)
- Location:
- trunk/Source/JavaScriptCore
- Files:
-
- 2 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/JavaScriptCore/ChangeLog
r171123 r171190 1 2014-07-16 Filip Pizlo <fpizlo@apple.com> 2 3 DFG Flush(SetLocal) store elimination is overzealous for captured variables in the presence of nodes that have no effects but may throw 4 https://bugs.webkit.org/show_bug.cgi?id=134988 5 <rdar://problem/17706349> 6 7 Reviewed by Oliver Hunt. 8 9 Luckily, we also don't need this optimization to be super powerful: the only place 10 where it really matters is for getting rid of the redundancy between op_enter and 11 op_init_lazy_reg, and in that case, there is a small set of possible nodes between the 12 two things. This change updates the store eliminator to know about only that small, 13 obviously safe, set of nodes over which we can store-eliminate. 14 15 This shouldn't have any performance impact in the DFG because this optimization kicks 16 in relatively rarely already. And once we tier up into the FTL, we get a much better 17 store elimination over LLVM IR, so this really shouldn't matter at all. 18 19 The tricky part of this patch is that there is a close relative of this optimization, 20 for uncaptured variables that got flushed. This happens for arguments to inlined calls. 21 I make this work by splitting it into two different store eliminators. 22 23 Note that in the process of crafting the tests, I realized that we were incorrectly 24 DCEing NewArrayWithSize. That's not cool, since that can throw an exception for 25 negative array sizes. If we ever did want to DCE this node, we'd need to lower the node 26 to a check node followed by the actual allocation. 27 28 * dfg/DFGCSEPhase.cpp: 29 (JSC::DFG::CSEPhase::uncapturedSetLocalStoreElimination): 30 (JSC::DFG::CSEPhase::capturedSetLocalStoreElimination): 31 (JSC::DFG::CSEPhase::setLocalStoreElimination): 32 (JSC::DFG::CSEPhase::performNodeCSE): 33 (JSC::DFG::CSEPhase::SetLocalStoreEliminationResult::SetLocalStoreEliminationResult): Deleted. 34 * dfg/DFGNodeType.h: 35 * tests/stress/capture-escape-and-throw.js: Added. 36 (foo.f): 37 (foo): 38 * tests/stress/new-array-with-size-throw-exception-and-tear-off-arguments.js: Added. 39 (foo): 40 (bar): 41 1 42 2014-07-15 Benjamin Poulain <benjamin@webkit.org> 2 43 -
trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
r170555 r171190 929 929 } 930 930 931 struct SetLocalStoreEliminationResult { 932 SetLocalStoreEliminationResult() 933 : mayBeAccessed(false) 934 , mayExit(false) 935 , mayClobberWorld(false) 936 { 937 } 938 939 bool mayBeAccessed; 940 bool mayExit; 941 bool mayClobberWorld; 942 }; 943 SetLocalStoreEliminationResult setLocalStoreElimination( 944 VirtualRegister local, Node* expectedNode) 945 { 946 SetLocalStoreEliminationResult result; 931 bool uncapturedSetLocalStoreElimination(VirtualRegister local, Node* expectedNode) 932 { 947 933 for (unsigned i = m_indexInBlock; i--;) { 948 934 Node* node = m_currentBlock->at(i); … … 951 937 case Flush: 952 938 if (node->local() == local) 953 re sult.mayBeAccessed = true;939 return false; 954 940 break; 955 941 956 942 case GetLocalUnlinked: 957 943 if (node->unlinkedLocal() == local) 958 re sult.mayBeAccessed = true;944 return false; 959 945 break; 960 946 … … 963 949 break; 964 950 if (node != expectedNode) 965 re sult.mayBeAccessed = true;966 return result;951 return false; 952 return true; 967 953 } 968 954 … … 970 956 case PutClosureVar: 971 957 if (static_cast<VirtualRegister>(node->varNumber()) == local) 972 re sult.mayBeAccessed = true;958 return false; 973 959 break; 974 960 … … 978 964 break; 979 965 if (m_graph.uncheckedActivationRegister() == local) 980 re sult.mayBeAccessed = true;966 return false; 981 967 break; 982 968 … … 985 971 case GetMyArgumentsLengthSafe: 986 972 if (m_graph.uncheckedArgumentsRegisterFor(node->origin.semantic) == local) 987 re sult.mayBeAccessed = true;973 return false; 988 974 break; 989 975 990 976 case GetMyArgumentByVal: 991 977 case GetMyArgumentByValSafe: 992 result.mayBeAccessed = true; 993 break; 978 return false; 994 979 995 980 case GetByVal: 996 981 // If this is accessing arguments then it's potentially accessing locals. 997 982 if (node->arrayMode().type() == Array::Arguments) 998 re sult.mayBeAccessed = true;983 return false; 999 984 break; 1000 985 … … 1006 991 // argument register. But that seems like it would buy us very little since 1007 992 // any kind of tear offs are rare to begin with. 1008 result.mayBeAccessed = true; 1009 break; 1010 1011 default: 1012 break; 1013 } 1014 result.mayExit |= node->canExit(); 1015 result.mayClobberWorld |= m_graph.clobbersWorld(node); 993 return false; 994 995 default: 996 break; 997 } 998 if (m_graph.clobbersWorld(node)) 999 return false; 1016 1000 } 1017 1001 RELEASE_ASSERT_NOT_REACHED(); 1018 // Be safe in release mode. 1019 result.mayBeAccessed = true; 1020 return result; 1002 return false; 1003 } 1004 1005 bool capturedSetLocalStoreElimination(VirtualRegister local, Node* expectedNode) 1006 { 1007 for (unsigned i = m_indexInBlock; i--;) { 1008 Node* node = m_currentBlock->at(i); 1009 switch (node->op()) { 1010 case GetLocal: 1011 case Flush: 1012 if (node->local() == local) 1013 return false; 1014 break; 1015 1016 case GetLocalUnlinked: 1017 if (node->unlinkedLocal() == local) 1018 return false; 1019 break; 1020 1021 case SetLocal: { 1022 if (node->local() != local) 1023 break; 1024 if (node != expectedNode) 1025 return false; 1026 return true; 1027 } 1028 1029 case Phantom: 1030 case Check: 1031 case HardPhantom: 1032 case MovHint: 1033 case JSConstant: 1034 case DoubleConstant: 1035 case Int52Constant: 1036 break; 1037 1038 default: 1039 return false; 1040 } 1041 } 1042 RELEASE_ASSERT_NOT_REACHED(); 1043 return false; 1044 } 1045 1046 bool setLocalStoreElimination(VariableAccessData* variableAccessData, Node* expectedNode) 1047 { 1048 if (variableAccessData->isCaptured()) 1049 return capturedSetLocalStoreElimination(variableAccessData->local(), expectedNode); 1050 return uncapturedSetLocalStoreElimination(variableAccessData->local(), expectedNode); 1021 1051 } 1022 1052 … … 1216 1246 ASSERT(m_graph.m_form != SSA); 1217 1247 VariableAccessData* variableAccessData = node->variableAccessData(); 1218 VirtualRegister local = variableAccessData->local();1219 1248 if (!node->child1()) { 1220 1249 // FIXME: It's silly that we punt on flush-eliminating here. We don't really … … 1240 1269 break; 1241 1270 } 1242 SetLocalStoreEliminationResult result = 1243 setLocalStoreElimination(local, replacement); 1244 if (result.mayBeAccessed || result.mayClobberWorld) 1271 if (!setLocalStoreElimination(variableAccessData, replacement)) 1245 1272 break; 1246 1273 ASSERT(replacement->op() == SetLocal); 1247 // FIXME: Investigate using mayExit as a further optimization.1248 1274 node->convertToPhantom(); 1249 1275 Node* dataNode = replacement->child1().node(); -
trunk/Source/JavaScriptCore/dfg/DFGNodeType.h
r171096 r171190 234 234 macro(NewObject, NodeResultJS) \ 235 235 macro(NewArray, NodeResultJS | NodeHasVarArgs) \ 236 macro(NewArrayWithSize, NodeResultJS ) \236 macro(NewArrayWithSize, NodeResultJS | NodeMustGenerate) \ 237 237 macro(NewArrayBuffer, NodeResultJS) \ 238 238 macro(NewTypedArray, NodeResultJS | NodeClobbersWorld | NodeMustGenerate) \
Note: See TracChangeset
for help on using the changeset viewer.