-
oliver@apple.com authored
https://bugs.webkit.org/show_bug.cgi?id=117375 Reviewed by Filip Pizlo. Source/JavaScriptCore: This patch has two goals: (1) Simplicity. * Net removes 15 opcodes. * Net removes 2,000 lines of code. * Removes setPair() from the DFG: All DFG nodes have 1 result register now. (2) Performance. * 2%-3% speedup on SunSpider (20% in LLInt and Baseline JIT) * 2% speedup on v8-spider * 10% speedup on js-regress-hashmap* * Amusing 2X speedup on js-regress-poly-stricteq The bytecode now separates the scope chain resolution opcode from the scope access opcode. OLD: get_scoped_var r0, 1, 0 inc r0 put_scoped_var 1, 0, r0 NEW: resolve_scope r0, x(@id0) get_from_scope r1, r0, x(@id0) inc r1 put_to_scope r0, x(@id0), r1 Also, we link non-local variable resolution opcodes at CodeBlock link time instead of time of first opcode execution. This means that we can represent all possible non-local variable resolutions using just three opcodes, and any optimizations in these opcodes naturally apply across-the-board. * API/JSCTestRunnerUtils.cpp: (JSC::numberOfDFGCompiles): * GNUmakefile.list.am: * JavaScriptCore.gypi: * JavaScriptCore.order: * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj: * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj: * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters: * JavaScriptCore.xcodeproj/project.pbxproj: * Target.pri: Build! * bytecode/CodeBlock.cpp: (JSC::CodeBlock::dumpBytecode): Updated for removed things. (JSC::CodeBlock::CodeBlock): Always provide the full scope chain when creating a CodeBlock, so we can perform non-local variable resolution. Added code to perform linking for these opcodes. This is where we figure out which non-local variable resolutions are optimizable, and how. (JSC::CodeBlock::finalizeUnconditionally): (JSC::CodeBlock::noticeIncomingCall): (JSC::CodeBlock::optimizeAfterWarmUp): (JSC::CodeBlock::optimizeAfterLongWarmUp): (JSC::CodeBlock::optimizeSoon): Updated for removed things. * bytecode/CodeBlock.h: (JSC::CodeBlock::needsActivation): (JSC::GlobalCodeBlock::GlobalCodeBlock): (JSC::ProgramCodeBlock::ProgramCodeBlock): (JSC::EvalCodeBlock::EvalCodeBlock): (JSC::FunctionCodeBlock::FunctionCodeBlock): * bytecode/EvalCodeCache.h: (JSC::EvalCodeCache::getSlow): Updated for interface changes. * bytecode/GetByIdStatus.cpp: (JSC::GetByIdStatus::computeFor): Treat global object access as optimizable even though the global object has a custom property access callback. This is what we've always done since, otherwise, we can't optimize globals. (In future, we probably want to figure out a more targeted policy than "any property access callback means no optimization".) * bytecode/GlobalResolveInfo.h: Removed. * bytecode/Instruction.h: * bytecode/Opcode.h: (JSC::padOpcodeName): * bytecode/PutByIdStatus.cpp: (JSC::PutByIdStatus::computeFor): Like GetByIdStatus. * bytecode/ResolveGlobalStatus.cpp: Removed. * bytecode/ResolveGlobalStatus.h: Removed. * bytecode/ResolveOperation.h: Removed. * bytecode/UnlinkedCodeBlock.cpp: (JSC::generateFunctionCodeBlock): (JSC::UnlinkedFunctionExecutable::codeBlockFor): (JSC::UnlinkedCodeBlock::UnlinkedCodeBlock): * bytecode/UnlinkedCodeBlock.h: Don't provide a scope chain to unlinked code blocks. Giving a scope to an unscoped compilation unit invites programming errors. * bytecode/Watchpoint.h: (JSC::WatchpointSet::addressOfIsInvalidated): * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::BytecodeGenerator): (JSC::BytecodeGenerator::resolveCallee): (JSC::BytecodeGenerator::local): (JSC::BytecodeGenerator::constLocal): (JSC::BytecodeGenerator::resolveType): (JSC::BytecodeGenerator::emitResolveScope): (JSC::BytecodeGenerator::emitGetFromScope): (JSC::BytecodeGenerator::emitPutToScope): (JSC::BytecodeGenerator::emitInstanceOf): (JSC::BytecodeGenerator::emitPushWithScope): (JSC::BytecodeGenerator::emitPopScope): (JSC::BytecodeGenerator::pushFinallyContext): (JSC::BytecodeGenerator::emitComplexPopScopes): (JSC::BytecodeGenerator::popTryAndEmitCatch): (JSC::BytecodeGenerator::emitPushNameScope): (JSC::BytecodeGenerator::isArgumentNumber): * bytecompiler/BytecodeGenerator.h: (JSC::Local::Local): (JSC::Local::operator bool): (JSC::Local::get): (JSC::Local::isReadOnly): (JSC::BytecodeGenerator::scopeDepth): (JSC::BytecodeGenerator::shouldOptimizeLocals): (JSC::BytecodeGenerator::canOptimizeNonLocals): Refactored the bytecode generator to resolve all variables within local scope, as if there were no non-local scope. This helps provide a separation of concerns: unlinked bytecode is always scope-free, and the linking stage links in the provided scope. * bytecompiler/NodesCodegen.cpp: (JSC::ResolveNode::isPure): (JSC::ResolveNode::emitBytecode): (JSC::EvalFunctionCallNode::emitBytecode): (JSC::FunctionCallResolveNode::emitBytecode): (JSC::PostfixNode::emitResolve): (JSC::DeleteResolveNode::emitBytecode): (JSC::TypeOfResolveNode::emitBytecode): (JSC::PrefixNode::emitResolve): (JSC::ReadModifyResolveNode::emitBytecode): (JSC::AssignResolveNode::emitBytecode): (JSC::ConstDeclNode::emitCodeSingle): (JSC::ForInNode::emitBytecode): A bunch of this codegen is no longer necessary, since it's redundant with the linking stage. * dfg/DFGAbstractState.cpp: (JSC::DFG::AbstractState::executeEffects): * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::ByteCodeParser): (JSC::DFG::ByteCodeParser::cellConstantWithStructureCheck): (JSC::DFG::ByteCodeParser::handlePutByOffset): (JSC::DFG::ByteCodeParser::handleGetById): (JSC::DFG::ByteCodeParser::parseBlock): Updated for interface changes. Notably, we can reuse existing DFG nodes -- but the mapping between bytecode and DFG nodes has changed, and some nodes and corner cases have been removed. * dfg/DFGCSEPhase.cpp: (JSC::DFG::CSEPhase::scopedVarLoadElimination): (JSC::DFG::CSEPhase::varInjectionWatchpointElimination): (JSC::DFG::CSEPhase::globalVarStoreElimination): (JSC::DFG::CSEPhase::scopedVarStoreElimination): (JSC::DFG::CSEPhase::getLocalLoadElimination): (JSC::DFG::CSEPhase::setLocalStoreElimination): (JSC::DFG::CSEPhase::performNodeCSE): Added CSE for var injection watchpoints. Even though watchpoints are "free", they're quite common inside code that's subject to var injection, so I figured we'd save a little memory. * dfg/DFGCapabilities.cpp: (JSC::DFG::capabilityLevel): * dfg/DFGCapabilities.h: Removed detection for old forms. * dfg/DFGDriver.h: (JSC::DFG::tryCompile): (JSC::DFG::tryCompileFunction): * dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): * dfg/DFGGraph.h: * dfg/DFGJITCode.cpp: * dfg/DFGNode.h: (JSC::DFG::Node::convertToStructureTransitionWatchpoint): (JSC::DFG::Node::hasVarNumber): (JSC::DFG::Node::hasIdentifierNumberForCheck): (JSC::DFG::Node::hasRegisterPointer): (JSC::DFG::Node::hasHeapPrediction): * dfg/DFGNodeType.h: * dfg/DFGOperations.cpp: * dfg/DFGOperations.h: * dfg/DFGPredictionPropagationPhase.cpp: (JSC::DFG::PredictionPropagationPhase::propagate): * dfg/DFGRepatch.h: (JSC::DFG::dfgResetGetByID): (JSC::DFG::dfgResetPutByID): * dfg/DFGSpeculativeJIT.h: (JSC::DFG::SpeculativeJIT::callOperation): Removed some unneeded things, and updated for renames. * dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compile): * dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compile): The two primary changes here are: (1) Use a watchpoint for var injection instead of looping over the scope chain and checking. This is more efficient and much easier to model in code generation. (2) I've eliminated the notion of an optimized global assignment that needs to check for whether it should fire a watchpiont. Instead, we fire pre-emptively at the point of optimization. This removes a bunch of edge cases, and it seems like a more honest representation of the fact that our new optimization contradicts our old one. * dfg/DFGTypeCheckHoistingPhase.cpp: (JSC::DFG::TypeCheckHoistingPhase::identifyRedundantStructureChecks): (JSC::DFG::TypeCheckHoistingPhase::identifyRedundantArrayChecks): * heap/DFGCodeBlocks.cpp: (JSC::DFGCodeBlocks::jettison): * interpreter/CallFrame.h: (JSC::ExecState::trueCallFrame): Removed stuff that's unused now, and fixed the build. * interpreter/Interpreter.cpp: (JSC::eval): (JSC::getBytecodeOffsetForCallFrame): (JSC::getCallerInfo): (JSC::Interpreter::throwException): Updated exception scope tracking to match the rest of our linking strategy: The unlinked bytecode compiles exception scope as if non-local scope did not exist, and we add in non-local scope at link time. This means that we can restore the right scope depth based on a simple number, without checking the contents of the scope chain. (JSC::Interpreter::execute): Make sure to establish the full scope chain before linking eval code. We now require the full scope chain at link time, in order to link non-local variable resolution opcodes. * jit/JIT.cpp: (JSC::JIT::JIT): (JSC::JIT::privateCompileMainPass): (JSC::JIT::privateCompileSlowCases): * jit/JIT.h: * jit/JITArithmetic.cpp: (JSC::JIT::emit_op_add): * jit/JITCode.cpp: * jit/JITOpcodes.cpp: (JSC::JIT::emitSlow_op_bitxor): (JSC::JIT::emitSlow_op_bitor): * jit/JITOpcodes32_64.cpp: (JSC::JIT::emitSlow_op_to_primitive): (JSC::JIT::emit_op_strcat): (JSC::JIT::emitSlow_op_create_this): (JSC::JIT::emitSlow_op_to_this): * jit/JITPropertyAccess.cpp: (JSC::JIT::emitVarInjectionCheck): (JSC::JIT::emitResolveClosure): (JSC::JIT::emit_op_resolve_scope): (JSC::JIT::emitSlow_op_resolve_scope): (JSC::JIT::emitLoadWithStructureCheck): (JSC::JIT::emitGetGlobalProperty): (JSC::JIT::emitGetGlobalVar): (JSC::JIT::emitGetClosureVar): (JSC::JIT::emit_op_get_from_scope): (JSC::JIT::emitSlow_op_get_from_scope): (JSC::JIT::emitPutGlobalProperty): (JSC::JIT::emitPutGlobalVar): (JSC::JIT::emitPutClosureVar): (JSC::JIT::emit_op_put_to_scope): (JSC::JIT::emitSlow_op_put_to_scope): (JSC::JIT::emit_op_init_global_const): * jit/JITPropertyAccess32_64.cpp: (JSC::JIT::emitVarInjectionCheck): (JSC::JIT::emitResolveClosure): (JSC::JIT::emit_op_resolve_scope): (JSC::JIT::emitSlow_op_resolve_scope): (JSC::JIT::emitLoadWithStructureCheck): (JSC::JIT::emitGetGlobalProperty): (JSC::JIT::emitGetGlobalVar): (JSC::JIT::emitGetClosureVar): (JSC::JIT::emit_op_get_from_scope): (JSC::JIT::emitSlow_op_get_from_scope): (JSC::JIT::emitPutGlobalProperty): (JSC::JIT::emitPutGlobalVar): (JSC::JIT::emitPutClosureVar): (JSC::JIT::emit_op_put_to_scope): (JSC::JIT::emitSlow_op_put_to_scope): (JSC::JIT::emit_op_init_global_const): * jit/JITStubs.cpp: (JSC::DEFINE_STUB_FUNCTION): * jit/JITStubs.h: Re-wrote baseline JIT codegen for our new variable resolution model. * llint/LLIntData.cpp: (JSC::LLInt::Data::performAssertions): * llint/LLIntSlowPaths.cpp: * llint/LLIntSlowPaths.h: * llint/LowLevelInterpreter.asm: * llint/LowLevelInterpreter.cpp: (JSC::CLoop::execute): * llint/LowLevelInterpreter32_64.asm: * llint/LowLevelInterpreter64.asm: Ditto for LLInt. * offlineasm/x86.rb: Fixed a pre-existing encoding bug for a syntactic form that we never used before. * runtime/ArrayPrototype.cpp: (JSC::arrayProtoFuncToString): (JSC::arrayProtoFuncToLocaleString): (JSC::arrayProtoFuncJoin): (JSC::arrayProtoFuncConcat): (JSC::arrayProtoFuncPop): (JSC::arrayProtoFuncPush): (JSC::arrayProtoFuncReverse): (JSC::arrayProtoFuncShift): (JSC::arrayProtoFuncSlice): (JSC::arrayProtoFuncSort): (JSC::arrayProtoFuncSplice): (JSC::arrayProtoFuncUnShift): (JSC::arrayProtoFuncFilter): (JSC::arrayProtoFuncMap): (JSC::arrayProtoFuncEvery): (JSC::arrayProtoFuncForEach): (JSC::arrayProtoFuncSome): (JSC::arrayProtoFuncReduce): (JSC::arrayProtoFuncReduceRight): (JSC::arrayProtoFuncIndexOf): (JSC::arrayProtoFuncLastIndexOf): Fixed some pre-existing bugs in 'this' value conversion, which I made much more common by removing special cases in bytecode generation. These functions need to invoke toThis() because they observe the 'this' value. Also, toLocaleString() is specified to accept non-array 'this' values. (Most other host functions don't need this fix because they perform strict 'this' checking, which never coerces unexpected types.) * runtime/CodeCache.cpp: (JSC::CodeCache::getCodeBlock): (JSC::CodeCache::getProgramCodeBlock): (JSC::CodeCache::getEvalCodeBlock): * runtime/CodeCache.h: Don't supply a scope to the unlinked code cache. Unlinked code is supposed to be scope-free, so let's have the compiler help verify that. * runtime/CommonSlowPaths.cpp: (JSC::SLOW_PATH_DECL): * runtime/CommonSlowPaths.h: * runtime/Executable.cpp: (JSC::EvalExecutable::create): (JSC::EvalExecutable::compileInternal): (JSC::ProgramExecutable::compileInternal): (JSC::FunctionExecutable::produceCodeBlockFor): (JSC::FunctionExecutable::compileForCallInternal): (JSC::FunctionExecutable::compileForConstructInternal): * runtime/Executable.h: (JSC::EvalExecutable::numVariables): (JSC::EvalExecutable::numberOfFunctionDecls): * runtime/ExecutionHarness.h: (JSC::prepareForExecutionImpl): (JSC::prepareFunctionForExecutionImpl): (JSC::installOptimizedCode): Fiddled with executable initialization so that we can always generate a full scope chain before we go to link a code block. We need this because code block linking now depends on the scope chain to link non-local variable resolution opcodes. * runtime/JSActivation.h: * runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::JSGlobalObject): (JSC::JSGlobalObject::createEvalCodeBlock): * runtime/JSGlobalObject.h: (JSC::JSGlobalObject::varInjectionWatchpoint): * runtime/JSGlobalObjectFunctions.cpp: (JSC::globalFuncEval): * runtime/JSNameScope.h: * runtime/JSScope.cpp: (JSC::abstractAccess): (JSC::JSScope::objectAtScope): (JSC::JSScope::depth): (JSC::JSScope::resolve): (JSC::JSScope::abstractResolve): Updated to match changes explained above. * runtime/JSScope.h: (JSC::makeType): (JSC::needsVarInjectionChecks): (JSC::ResolveOp::ResolveOp): (JSC::ResolveModeAndType::ResolveModeAndType): (JSC::ResolveModeAndType::mode): (JSC::ResolveModeAndType::type): (JSC::ResolveModeAndType::operand): Removed the old variable resolution state machine, since it's unused now. Added logic for performing abstract variable resolution at link time. This is used by codeblock linking. * runtime/ObjectPrototype.cpp: (JSC::objectProtoFuncValueOf): (JSC::objectProtoFuncHasOwnProperty): (JSC::objectProtoFuncIsPrototypeOf): (JSC::objectProtoFuncDefineGetter): (JSC::objectProtoFuncDefineSetter): (JSC::objectProtoFuncLookupGetter): (JSC::objectProtoFuncLookupSetter): (JSC::objectProtoFuncPropertyIsEnumerable): (JSC::objectProtoFuncToLocaleString): (JSC::objectProtoFuncToString): Fixed some pre-existing bugs in 'this' value conversion, which I made much more common by removing special cases in bytecode generation. These functions need to invoke toThis() because they observe the 'this' value. * runtime/StringPrototype.cpp: (JSC::checkObjectCoercible): (JSC::stringProtoFuncReplace): (JSC::stringProtoFuncCharAt): (JSC::stringProtoFuncCharCodeAt): (JSC::stringProtoFuncConcat): (JSC::stringProtoFuncIndexOf): (JSC::stringProtoFuncLastIndexOf): (JSC::stringProtoFuncMatch): (JSC::stringProtoFuncSearch): (JSC::stringProtoFuncSlice): (JSC::stringProtoFuncSplit): (JSC::stringProtoFuncSubstr): (JSC::stringProtoFuncSubstring): (JSC::stringProtoFuncToLowerCase): (JSC::stringProtoFuncToUpperCase): (JSC::stringProtoFuncLocaleCompare): (JSC::stringProtoFuncBig): (JSC::stringProtoFuncSmall): (JSC::stringProtoFuncBlink): (JSC::stringProtoFuncBold): (JSC::stringProtoFuncFixed): (JSC::stringProtoFuncItalics): (JSC::stringProtoFuncStrike): (JSC::stringProtoFuncSub): (JSC::stringProtoFuncSup): (JSC::stringProtoFuncFontcolor): (JSC::stringProtoFuncFontsize): (JSC::stringProtoFuncAnchor): (JSC::stringProtoFuncLink): (JSC::trimString): Fixed some pre-existing bugs in 'this' value conversion, which I made much more common by removing special cases in bytecode generation. These functions need to invoke toThis() because they observe the 'this' value. * runtime/StructureRareData.cpp: * runtime/VM.cpp: (JSC::VM::~VM): * runtime/WriteBarrier.h: (JSC::WriteBarrierBase::slot): Modified to reduce casting in client code. LayoutTests: This patch removed special-case 'this' resolution from bytecode, making some pre-existing edge cases in 'this' value treatment much more common. I updated the test results below, and added some tests, to match bug fixes for these cases. * fast/js/script-tests/array-functions-non-arrays.js: * fast/js/array-functions-non-arrays-expected.txt: As specified, it's not an error to pass a non-array to toLocaleString. Our new result matches Firefox and Chrome. * fast/js/array-prototype-properties-expected.txt: Updated for slightly clearer error message. * fast/js/basic-strict-mode-expected.txt: Updated for slightly more standard error message. * fast/js/object-prototype-toString-expected.txt: Added. * fast/js/object-prototype-toString.html: Added. This test demonstrates why we now fail a Sputnik test below, while Firefox and Chrome pass it. (The test doesn't test what it thinks it tests, and this test verifies that we get right what it does think it tests.) * fast/js/string-prototype-function-this-expected.txt: Added. * fast/js/string-prototype-function-this.html: Added. This test shows that we CheckObjectCoercible in string prototype functions. (We used to get this wrong, but Sputnik tests made it seem like we got it right because they didn't test the dynamic scope case.) * sputnik/Conformance/11_Expressions/11.1_Primary_Expressions/11.1.1_The_this_Keyword/S11.1.1_A2-expected.txt: * sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.3_Array_prototype_toLocaleString/S15.4.4.3_A2_T1-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.11_String.prototype.replace/S15.5.4.11_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.12_String.prototype.search/S15.5.4.12_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.13_String.prototype.slice/S15.5.4.13_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.15_String.prototype.substring/S15.5.4.15_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.6_String.prototype.concat/S15.5.4.6_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.7_String.prototype.indexOf/S15.5.4.7_A1_T3-expected.txt: * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.8_String.prototype.lastIndexOf/S15.5.4.8_A1_T3-expected.txt: Updated to show failing results. Firefox and Chrome also fail these tests, and the ES5 spec seems to mandate failure. Because these tests resolve a String.prototype function at global scope, the 'this' value for the call is an environment record. Logically, an environment record converts to 'undefined' at the call site, and should then fail the CheckObjectCoercible test. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153221 268f45cc-cd09-0410-ab3c-d52691b4dbfc
58c86752