Now, our rules for ValueArith nodes are conservative and we don't allow CSE. In the case of SpecBigInt, we can be less conservative and allow CSE.
Created attachment 357436 [details] Proposed Patch
Comment on attachment 357436 [details] Proposed Patch r=me
Comment on attachment 357436 [details] Proposed Patch Thank you very much for the review.
Comment on attachment 357436 [details] Proposed Patch Clearing flags on attachment: 357436 Committed r239377: <https://trac.webkit.org/changeset/239377>
All reviewed patches have been landed. Closing bug.
<rdar://problem/46838118>
It looks like this change caused 35 JSC tests to fail an assertion: ASSERTION FAILED: node() /Volumes/Data/slave/highsierra-debug/build/Source/JavaScriptCore/dfg/DFGEdge.h(82) : JSC::DFG::UseKind JSC::DFG::Edge::useKind() const 1 0x101610ec9 WTFCrash 2 0x101611ebb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x10175245d JSC::DFG::Edge::useKind() const 4 0x10176ff95 JSC::DFG::Node::binaryUseKind() 5 0x102507d39 JSC::DFG::StrengthReductionPhase::handleNode() 6 0x10250656f JSC::DFG::StrengthReductionPhase::run() 7 0x102506351 bool JSC::DFG::runAndLog<JSC::DFG::StrengthReductionPhase>(JSC::DFG::StrengthReductionPhase&) 8 0x1024fe72e bool JSC::DFG::runPhase<JSC::DFG::StrengthReductionPhase>(JSC::DFG::Graph&) 9 0x1024fe6f5 JSC::DFG::performStrengthReduction(JSC::DFG::Graph&) 10 0x10249a9d7 JSC::DFG::Plan::compileInThreadImpl() 11 0x102499112 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) 12 0x1022e21fe JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&) 13 0x1022e1c9f JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&) 14 0x1027dd5cc operationOptimize 15 0x5e89e90e72b 16 0x101b012c9 vmEntryToJavaScript 17 0x1027561de JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 18 0x10275576f JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*) 19 0x102a4f385 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) 20 0x101545cfb runWithOptions(GlobalObject*, CommandLine&, bool&) 21 0x10151d31a jscmain(int, char**)::$_6::operator()(JSC::VM&, GlobalObject*, bool&) const 22 0x101500954 int runJSC<jscmain(int, char**)::$_6>(CommandLine const&, bool, jscmain(int, char**)::$_6 const&) 23 0x1014ff0e7 jscmain(int, char**) 24 0x1014fef4e main 25 0x7fff62e41015 start 26 0xc https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/1926/steps/jscore-test/logs/stdio
I'm looking into that now. (In reply to Ryan Haddad from comment #7) > It looks like this change caused 35 JSC tests to fail an assertion: > > ASSERTION FAILED: node() > /Volumes/Data/slave/highsierra-debug/build/Source/JavaScriptCore/dfg/ > DFGEdge.h(82) : JSC::DFG::UseKind JSC::DFG::Edge::useKind() const > 1 0x101610ec9 WTFCrash > 2 0x101611ebb WTFCrashWithInfo(int, char const*, char const*, int) > 3 0x10175245d JSC::DFG::Edge::useKind() const > 4 0x10176ff95 JSC::DFG::Node::binaryUseKind() > 5 0x102507d39 JSC::DFG::StrengthReductionPhase::handleNode() > 6 0x10250656f JSC::DFG::StrengthReductionPhase::run() > 7 0x102506351 bool > JSC::DFG::runAndLog<JSC::DFG::StrengthReductionPhase>(JSC::DFG:: > StrengthReductionPhase&) > 8 0x1024fe72e bool > JSC::DFG::runPhase<JSC::DFG::StrengthReductionPhase>(JSC::DFG::Graph&) > 9 0x1024fe6f5 JSC::DFG::performStrengthReduction(JSC::DFG::Graph&) > 10 0x10249a9d7 JSC::DFG::Plan::compileInThreadImpl() > 11 0x102499112 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) > 12 0x1022e21fe JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, > JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, > JSC::Operands<JSC::JSValue> const&, > WTF::Ref<JSC::DeferredCompilationCallback, > WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&) > 13 0x1022e1c9f JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, > JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, > JSC::Operands<JSC::JSValue> const&, > WTF::Ref<JSC::DeferredCompilationCallback, > WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&) > 14 0x1027dd5cc operationOptimize > 15 0x5e89e90e72b > 16 0x101b012c9 vmEntryToJavaScript > 17 0x1027561de JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) > 18 0x10275576f JSC::Interpreter::executeProgram(JSC::SourceCode const&, > JSC::ExecState*, JSC::JSObject*) > 19 0x102a4f385 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, > JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) > 20 0x101545cfb runWithOptions(GlobalObject*, CommandLine&, bool&) > 21 0x10151d31a jscmain(int, char**)::$_6::operator()(JSC::VM&, > GlobalObject*, bool&) const > 22 0x101500954 int runJSC<jscmain(int, char**)::$_6>(CommandLine const&, > bool, jscmain(int, char**)::$_6 const&) > 23 0x1014ff0e7 jscmain(int, char**) > 24 0x1014fef4e main > 25 0x7fff62e41015 start > 26 0xc > > https://build.webkit.org/builders/ > Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/1926/steps/jscore- > test/logs/stdio
Handling the issue in https://bugs.webkit.org/show_bug.cgi?id=192908
I think we should revert this until we can reland it with a fix. It's not great to leave failing tests in the tree for an extended period of time.
Re-opened since this is blocked by bug 192921
*** Bug 192908 has been marked as a duplicate of this bug. ***
(In reply to Keith Miller from comment #10) > I think we should revert this until we can reland it with a fix. It's not > great to leave failing tests in the tree for an extended period of time. Sorry for leaving this so long and thx for the rollback.
Created attachment 357805 [details] Patch
(In reply to Caio Lima from comment #14) > Created attachment 357805 [details] > Patch The difference from previous version is that we need to avoid execution of 'm_node->binaryUseKind()' after performing String Folding into DFGStrengthReductionPhase.cpp. I added a break into line 375
Comment on attachment 357805 [details] Patch r=me
Comment on attachment 357805 [details] Patch Thank you for the review!
Comment on attachment 357805 [details] Patch Clearing flags on attachment: 357805 Committed r239438: <https://trac.webkit.org/changeset/239438>