Bug 141019

Created attachment 245593 [details] Work in progress

Attachment 245593 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1130: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 245593 [details] Work in progress all the disappearing code makes me happy

As well as de-duplicating the arguments to op_create_this, you can probably also remove op_get_callee, too!

Geoff tells me he'd prefer keeping the two-arguments model so that the rest of the code base doens't need to know this quirk. Also, we can't quite get rid of op_get_callee because my subsequent patches to implement ES6 class needs to use op_get_callee to get various private flags stored in the constructor.

Created attachment 247180 [details] Patch

Comment on attachment 247180 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247180&action=review > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:-3884 > arguments.append(jsCallee); > if (jsArguments) > arguments.append(jsArguments); > - if (thisArg) Just above here there is the line: arguments.append(m_out.constInt32(1 + !!jsArguments + !!thisArg)); I guess this can now be 2 + !!jsArguments

Created attachment 247273 [details] Patch for landing

Comment on attachment 247273 [details] Patch for landing Clearing flags on attachment: 247273 Committed r180595: <http://trac.webkit.org/changeset/180595>

All reviewed patches have been landed. Closing bug.

Looks like this could be a 1.7% progression on Sunspider: http://arewefastyet.com/#machine=29

It looks like this caused test failures on iOS Arm 64 Test r180594 r180600 regress/script-tests/deltablue-varargs.js.ftl-eager Passed Failed regress/script-tests/deltablue-varargs.js.ftl-no-cjit-no-inline-validate Passed Failed stress/construct-varargs-inline-smaller-Foo.js.ftl-no-cjit-no-inline-validate Passed Failed stress/construct-varargs-inline.js.ftl-eager Passed Failed stress/construct-varargs-inline.js.ftl-eager-no-cjit Passed Failed stress/construct-varargs-inline.js.ftl-no-cjit-no-inline-validate Passed Failed stress/construct-varargs-inline.js.ftl-no-cjit-validate Passed Failed stress/construct-varargs-no-inline.js.ftl-eager Passed Failed stress/construct-varargs-no-inline.js.ftl-eager-no-cjit Passed Failed stress/construct-varargs-no-inline.js.ftl-no-cjit-no-inline-validate Passed Failed stress/construct-varargs-no-inline.js.ftl-no-cjit-validate Passed Failed The failures have a similar output to: [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: Failed to insert inline cache for varargs call (specifically, ConstructVarargs) because we thought the size would be 284 but it ended up being 300 prior to compaction. [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 1 0x1002af98c JSC::FTL::compile(JSC::FTL::State&, JSC::DFG::Safepoint::Result&) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 2 0x100226668 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 3 0x100225db0 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 4 0x1001d15b0 JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue, JSC::OperandValueTraits<JSC::JSValue> > const&, WTF::PassRefPtr<JSC::DeferredCompilationCallback>) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 5 0x10020a684 JSC::DFG::triggerFTLReplacementCompile(JSC::VM*, JSC::CodeBlock*, JSC::DFG::JITCode*) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 6 0x10020a37c triggerTierUpNow [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 7 0x130adcf34 [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 8 0x12fae2938 [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 9 0x12fae3a7c [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 10 0x100467848 vmEntryToJavaScript [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 11 0x10037db84 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 12 0x10036c508 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 13 0x100159fd8 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 14 0x1000c9c60 jscmain(int, char**) [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 15 0x1000c9670 main [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: 16 0x1980269e8 <redacted> [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: ./test_script_13755: line 2: 34513 Segmentation fault: 11 "$@" /System/Library/Frameworks/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --enableFunctionDotArguments\=true --validateGraph\=true --useFTLJIT\=true --enableConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 --thresholdForJITAfterWarmUp\=10 --thresholdForJITSoon\=10 --thresholdForOptimizeAfterWarmUp\=20 --thresholdForOptimizeAfterLongWarmUp\=20 --thresholdForOptimizeSoon\=20 --thresholdForFTLOptimizeAfterWarmUp\=20 --thresholdForFTLOptimizeSoon\=20 --maximumEvalCacheableSourceLength\=150000 construct-varargs-inline.js [2015-02-25 00:40:02] INFO: stress/construct-varargs-inline.js.ftl-eager-no-cjit: ERROR: Unexpected exit code: 139 [2015-02-25 00:40:02] ERROR: FAIL: stress/construct-varargs-inline.js.ftl-eager-no-cjit

The regression was fixed in https://bugs.webkit.org/show_bug.cgi?id=142030.