Attachment 358230 Details for Bug 193094 – Patch

[patch] Patch

193094.patch (text/plain), 2.27 KB, created by Michael Saboff on 2019-01-02 17:30:41 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Saboff

Created: 2019-01-02 17:30:41 PST

Size: 2.27 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 239582)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2019-01-02  Michael Saboff  <msaboff@apple.com>
>+
>+        DFG IntegerRangeOptimization phase exceeding loop limit shouldn't ASSERT
>+        https://bugs.webkit.org/show_bug.cgi?id=193094
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Removed this debug only ASSERT as one can construct test cases that will exceed the giveUpThreshold
>+        amount.   This can be done with a large switch statement with at least one const or variable case
>+        clause.  Such code bytecompiles to compare / jtrue sequences.  Increasing the giveUpThreshold count
>+        doesn't help with the eventual code generated as B3 has optimizations to coalesce compare / branch
>+        code sequences even when we don't run the IntegerRangeOptimization phase.
>+
>+        * dfg/DFGIntegerRangeOptimizationPhase.cpp:
>+
> 2018-12-31  Keith Miller  <keith_miller@apple.com>
> 
>         SourceProviders should use an actual URL instead of a string
>Index: Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp	(revision 239571)
>+++ Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp	(working copy)
>@@ -1092,13 +1092,12 @@ public:
>             ++m_iterations;
>             if (m_iterations >= giveUpThreshold) {
>                 // This case is not necessarily wrong but it can be a sign that this phase
>-                // does not converge.
>+                // does not converge. The value giveUpThreshold was chosen emperically based on
>+                // current tests and real world JS.
>                 // If you hit this assertion for a legitimate case, update the giveUpThreshold
>                 // to the smallest values that converges.
>-                ASSERT_NOT_REACHED();
> 
>-                // In release, do not risk holding the thread for too long since this phase
>-                // is really slow.
>+                // Do not risk holding the thread for too long since this phase is really slow.
>                 return false;
>             }
>

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193094: 358230