Attachment 373057 Details for Bug 199291 – proposed patch.

[patch] proposed patch.

bug-199291.patch (text/plain), 2.08 KB, created by Mark Lam on 2019-06-27 15:19:04 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-06-27 15:19:04 PDT

Size: 2.08 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 246906)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2019-06-27  Mark Lam  <mark.lam@apple.com>
>+
>+        FTL keepAlive()'s patchpoint should also declare that it reads HeapRange::top().
>+        https://bugs.webkit.org/show_bug.cgi?id=199291
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The sole purpose of keepAlive() is to communicate to B3 that an LValue
>+        needs to be kept alive pass the last opportunity for a GC.  The only way
>+        we can get a GC is via a function call.  Hence, what keepAlive() really
>+        needs to communicate is that the LValue needs to be kept alive pass the
>+        last function call.  Function calls read and write HeapRange::top().
>+        Currently, B3 does not shuffle writes.  Hence, simply inserting the
>+        keepAlive() after the calls that can GC is sufficient.
>+
>+        But to be strictly correct, keepAlive() should also declare that it reads
>+        HeapRange::top().  This will guarantee that the keepAlive patchpoint won't
>+        ever be moved before the function call should B3 gain the ability to shuffle
>+        writes in the future.
>+
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::keepAlive):
>+
> 2019-06-27  Beth Dakin  <bdakin@apple.com>
> 
>         Upstream use of MACCATALYST
>Index: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(revision 246906)
>+++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(working copy)
>@@ -17284,6 +17284,7 @@ private:
>         PatchpointValue* patchpoint = m_out.patchpoint(Void);
>         patchpoint->effects = Effects::none();
>         patchpoint->effects.writesLocalState = true;
>+        patchpoint->effects.reads = HeapRange::top();
>         patchpoint->append(value, ValueRep::ColdAny);
>         patchpoint->setGenerator([=] (CCallHelpers&, const StackmapGenerationParams&) { });
>     }

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 199291: 373057