RESOLVED FIXED 192723
[BigInt] We should enable CSE into arithmetic operations that speculate BigIntUse
https://bugs.webkit.org/show_bug.cgi?id=192723
Summary [BigInt] We should enable CSE into arithmetic operations that speculate BigIn...
Caio Lima
Reported 2018-12-14 16:16:03 PST
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.
Attachments
Proposed Patch (13.07 KB, patch)
2018-12-17 05:51 PST, Caio Lima
no flags
Patch (13.12 KB, patch)
2018-12-20 02:37 PST, Caio Lima
no flags
Caio Lima
Comment 1 2018-12-17 05:51:11 PST
Created attachment 357436 [details] Proposed Patch
Saam Barati
Comment 2 2018-12-18 23:33:14 PST
Comment on attachment 357436 [details] Proposed Patch r=me
Caio Lima
Comment 3 2018-12-19 03:47:00 PST
Comment on attachment 357436 [details] Proposed Patch Thank you very much for the review.
WebKit Commit Bot
Comment 4 2018-12-19 04:12:45 PST
Comment on attachment 357436 [details] Proposed Patch Clearing flags on attachment: 357436 Committed r239377: <https://trac.webkit.org/changeset/239377>
WebKit Commit Bot
Comment 5 2018-12-19 04:12:47 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 6 2018-12-19 04:13:38 PST
Ryan Haddad
Comment 7 2018-12-19 17:39:14 PST
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
Caio Lima
Comment 8 2018-12-19 17:55:31 PST
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
Caio Lima
Comment 9 2018-12-19 18:19:36 PST
Keith Miller
Comment 10 2018-12-19 23:32:46 PST
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.
WebKit Commit Bot
Comment 11 2018-12-19 23:33:42 PST
Re-opened since this is blocked by bug 192921
Caio Lima
Comment 12 2018-12-20 02:28:44 PST
*** Bug 192908 has been marked as a duplicate of this bug. ***
Caio Lima
Comment 13 2018-12-20 02:32:38 PST
(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.
Caio Lima
Comment 14 2018-12-20 02:37:51 PST
Caio Lima
Comment 15 2018-12-20 02:40:01 PST
(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
Yusuke Suzuki
Comment 16 2018-12-20 02:53:18 PST
Comment on attachment 357805 [details] Patch r=me
Caio Lima
Comment 17 2018-12-20 03:33:20 PST
Comment on attachment 357805 [details] Patch Thank you for the review!
WebKit Commit Bot
Comment 18 2018-12-20 03:59:12 PST
Comment on attachment 357805 [details] Patch Clearing flags on attachment: 357805 Committed r239438: <https://trac.webkit.org/changeset/239438>
WebKit Commit Bot
Comment 19 2018-12-20 03:59:14 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.