Attachment 341136 Details for Bug 185923 – Patch

[patch] Patch

bug-185923-20180523152825.patch (text/plain), 2.97 KB, created by Keith Miller on 2018-05-23 15:28:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-05-23 15:28:26 PDT

Size: 2.97 KB

patch

obsolete

>Subversion Revision: 232113
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 5d6abeb4cf87e6cbebbd787ef91ac95ec5ccb92a..1689132ebf185a0987a7c703fddf72421ef2d589 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-05-23  Keith Miller  <keith_miller@apple.com>
>+
>+        InPlaceAbstractState should filter variables at the tail from a GetLocal by their flush format
>+        https://bugs.webkit.org/show_bug.cgi?id=185923
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Previously, we could confuse AI by overly broadening a type. This happens when a block in a
>+        loop has a local mutated following a GetLocal but never SetLocaled to the stack. For example,
>+
>+        Block 1:
>+        @1: GetLocal(loc42, FlushedInt32);
>+        @2: PutStructure(Check: Cell: @1);
>+        @3: Jump(Block 1);
>+
>+        Would cause us to claim that loc42 could be either an int32 or a some cell. However,
>+        the type of an local cannot change without writing to it.
>+
>+        * dfg/DFGInPlaceAbstractState.cpp:
>+        (JSC::DFG::InPlaceAbstractState::endBasicBlock):
>+
> 2018-05-23  Rick Waldron  <waldron.rick@gmail.com>
> 
>         Conversion misspelled "Convertion" in error message string
>diff --git a/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp b/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>index 8d4a656552549875210577a51fbff28136085b03..d2fbdecad8a553be204d3b72d48910c2dfd13066 100644
>--- a/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>@@ -236,21 +236,29 @@ bool InPlaceAbstractState::endBasicBlock()
>             case Phi:
>             case SetArgument:
>             case PhantomLocal:
>-            case Flush:
>+            case Flush: {
>                 // The block transfers the value from head to tail.
>                 destination = variableAt(index);
>                 break;
>+            }
>                 
>-            case GetLocal:
>+            case GetLocal: {
>                 // The block refines the value with additional speculations.
>                 destination = forNode(node);
>+
>+                // We need to make sure that we don't broaden the type beyond what the flush
>+                // format says it will be. The value may have changed type state but since we
>+                // never wrote it back to the stack we would not see any such changes.
>+                FlushFormat flushFormat = node->variableAccessData()->flushFormat();
>+                destination.filter(typeFilterFor(flushFormat));
>                 break;
>-                
>-            case SetLocal:
>+            }
>+            case SetLocal: {
>                 // The block sets the variable, and potentially refines it, both
>                 // before and after setting it.
>                 destination = forNode(node->child1());
>                 break;
>+            }
>                 
>             default:
>                 RELEASE_ASSERT_NOT_REACHED();

Actions: View | Formatted Diff | Diff

Attachments on bug 185923: 341136 | 341140