Attachment 339654 Details for Bug 185355 – the patch

[patch] the patch

blah.patch (text/plain), 2.39 KB, created by Filip Pizlo on 2018-05-05 12:17:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-05 12:17:53 PDT

Size: 2.39 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231396)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-05-05  Filip Pizlo  <fpizlo@apple.com>
>+
>+        DFG AI doesn't need to merge valuesAtTail - it can just assign them
>+        https://bugs.webkit.org/show_bug.cgi?id=185355
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This is a further attempt to improve compile times. Assigning AbstractValue ought to always
>+        be faster than merging. There's no need to merge valuesAtTail. In most cases, assigning and
>+        merging will get the same answer because the value computed this time will be either the same
>+        as or more general than the value computed last time. If the value does change for some
>+        reason, then valuesAtHead are already merged, which ensures monotonicity. Also, if the value
>+        changes, then we have no reason to believe that this new value is less right than the last
>+        one we computed. Finally, the one client of valuesAtTail (AtTailAbstractState) doesn't care
>+        if it's getting the merged valuesAtTail or just some correct answer to for valuesAtTail.
>+
>+        * dfg/DFGInPlaceAbstractState.cpp:
>+        (JSC::DFG::InPlaceAbstractState::endBasicBlock):
>+
> 2018-05-04  Keith Miller  <keith_miller@apple.com>
> 
>         isCacheableArrayLength should return true for undecided arrays
>Index: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(revision 231338)
>+++ Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(working copy)
>@@ -213,13 +213,10 @@ bool InPlaceAbstractState::endBasicBlock
> 
>     case SSA: {
>         for (size_t i = 0; i < block->valuesAtTail.size(); ++i)
>-            block->valuesAtTail[i].merge(m_variables[i]);
>+            block->valuesAtTail[i] = m_variables[i];
> 
>-        for (NodeAbstractValuePair& valueAtTail : block->ssa->valuesAtTail) {
>-            AbstractValue& valueAtNode = forNode(valueAtTail.node);
>-            valueAtTail.value.merge(valueAtNode);
>-            valueAtNode = valueAtTail.value;
>-        }
>+        for (NodeAbstractValuePair& valueAtTail : block->ssa->valuesAtTail)
>+            valueAtTail.value = forNode(valueAtTail.node);
>         break;
>     }
>

Flags:

mark.lam: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185355: 339654 | 339658