Summary: | [BigInt] We should enable CSE into arithmetic operations that speculate BigIntUse | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Caio Lima <ticaiolima> | ||||||
Component: | JavaScriptCore | Assignee: | Caio Lima <ticaiolima> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, rniwa, ryanhaddad, saam, webkit-bug-importer, ysuzuki | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 192921 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Caio Lima
2018-12-14 16:16:03 PST
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. 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> All reviewed patches have been landed. Closing bug. |