WebKit Bugzilla
Attachment 361095 Details for
Bug 194231
: when lowering AssertNotEmpty, create the value before creating the patchpoint
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194231-20190204135331.patch (text/plain), 3.08 KB, created by
Robin Morisset
on 2019-02-04 13:53:31 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Robin Morisset
Created:
2019-02-04 13:53:31 PST
Size:
3.08 KB
patch
obsolete
>Index: JSTests/ChangeLog >=================================================================== >--- JSTests/ChangeLog (revision 240940) >+++ JSTests/ChangeLog (working copy) >@@ -1,3 +1,17 @@ >+2019-02-04 Robin Morisset <rmorisset@apple.com> >+ >+ when lowering AssertNotEmpty, create the value before creating the patchpoint >+ https://bugs.webkit.org/show_bug.cgi?id=194231 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ This test is painfully fragile: it tries to test that AssertNotEmpty on a constant produces valid B3 IR. >+ The problem is that AssertNotEmpty is only created by DFGConstantFolding when it can simplify a CheckStructure, and constant folding is a bit capricious (https://bugs.webkit.org/show_bug.cgi?id=133947) >+ So even tiny changes to this test can change the path code taken. >+ >+ * stress/assert-not-empty.js: Added. >+ (foo): >+ > 2019-02-01 Mark Lam <mark.lam@apple.com> > > Remove invalid assertion in DFG's compileDoubleRep(). >Index: JSTests/stress/assert-not-empty.js >=================================================================== >--- JSTests/stress/assert-not-empty.js (nonexistent) >+++ JSTests/stress/assert-not-empty.js (working copy) >@@ -0,0 +1,12 @@ >+//@ runDefault("--useObjectAllocationSinking=0") >+ >+function foo(o) { >+ if (!o) { >+ +eval; >+ } >+ o.x; >+}; >+let i=0; >+for (;i<100000;++i) { >+ foo(Object); >+} >Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 240935) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -1,3 +1,16 @@ >+2019-02-04 Robin Morisset <rmorisset@apple.com> >+ >+ when lowering AssertNotEmpty, create the value before creating the patchpoint >+ https://bugs.webkit.org/show_bug.cgi?id=194231 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ This is a very simple change: we should never generate B3 IR where an instruction depends on a value that comes later in the instruction stream. >+ AssertNotEmpty was generating some such IR, it probably slipped through until now because it is a rather rare and tricky instruction to generate. >+ >+ * ftl/FTLLowerDFGToB3.cpp: >+ (JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty): >+ > 2019-02-04 Mark Lam <mark.lam@apple.com> > > DFG's doesGC() is incorrect about the SameValue node's behavior. >Index: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >=================================================================== >--- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (revision 240935) >+++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (working copy) >@@ -3111,8 +3111,9 @@ private: > if (!validationEnabled()) > return; > >+ LValue val = lowJSValue(m_node->child1()); > PatchpointValue* patchpoint = m_out.patchpoint(Void); >- patchpoint->appendSomeRegister(lowJSValue(m_node->child1())); >+ patchpoint->appendSomeRegister(val); > patchpoint->setGenerator( > [=] (CCallHelpers& jit, const StackmapGenerationParams& params) { > AllowMacroScratchRegisterUsage allowScratch(jit);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194231
:
361094
| 361095