Changeset 203034 in webkit

Timestamp:

Jul 9, 2016, 2:07:45 PM (9 years ago)

Author:

fpizlo@apple.com

Message:

REGRESSION(201900): validation failure for GetByOffset/PutByOffset in VALIDATE((node), node->child1().node() == node->child2().node()

node->child1()->result() == NodeResultStorage)

​https://bugs.webkit.org/show_bug.cgi?id=159603

Reviewed by Keith Miller.

This removes an incorrect validation rule and replaces it with a FIXME about how to make this
aspect of IR easier to validate soundly.

It's not valid to assert that two children of a node are the same. It should always be valid
to take:

Foo(@x, @x)

and turn it into:

a: ValueRep(@x)
b: ValueRep(@x)
Foo(@a, @b)

or even something like:

y: Identity(@y)
Foo(@x, @y)

That's because it should be possible to rewire any data flow edge something that produces an
equivalent value.

The validation rule that this patch removes meant that such rewirings were invalid on
GetByOffset/PutByOffset. FixupPhase did such a rewiring sometimes.

• dfg/DFGValidate.cpp:
• tests/stress/get-by-offset-double.js: Added.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGValidate.cpp (modified) (1 diff)
• tests/stress/get-by-offset-double.js (added)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-07-09  Filip Pizlo  <fpizlo@apple.com>
+
+        REGRESSION(201900): validation failure for GetByOffset/PutByOffset in VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage)
+        https://bugs.webkit.org/show_bug.cgi?id=159603
+
+        Reviewed by Keith Miller.
+        
+        This removes an incorrect validation rule and replaces it with a FIXME about how to make this
+        aspect of IR easier to validate soundly.
+        
+        It's not valid to assert that two children of a node are the same. It should always be valid
+        to take:
+        
+        Foo(@x, @x)
+        
+        and turn it into:
+        
+        a: ValueRep(@x)
+        b: ValueRep(@x)
+        Foo(@a, @b)
+        
+        or even something like:
+        
+        y: Identity(@y)
+        Foo(@x, @y)
+        
+        That's because it should be possible to rewire any data flow edge something that produces an
+        equivalent value.
+        
+        The validation rule that this patch removes meant that such rewirings were invalid on
+        GetByOffset/PutByOffset. FixupPhase did such a rewiring sometimes.
+
+        * dfg/DFGValidate.cpp:
+        * tests/stress/get-by-offset-double.js: Added.
+
 2016-07-09  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp

@@ -300 +300 @@
                 case GetByOffset:
                 case PutByOffset:
-                    VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage);
+                    // FIXME: We should be able to validate that GetByOffset and PutByOffset are
+                    // using the same object for storage and base. I think this means finally
+                    // splitting these nodes into two node types, one for inline and one for
+                    // out-of-line. The out-of-line one will require that the first node is storage,
+                    // while the inline one will not take a storage child at all.
+                    // https://bugs.webkit.org/show_bug.cgi?id=159602
                     break;
                 default: