ValueRep(DoubleRep(@v)) can not simply convert to @v
[WebKit-https.git] / Source / JavaScriptCore / ChangeLog
index 2649cfe..5f9f8de 100644 (file)
@@ -1,3 +1,67 @@
+2017-06-22  Saam Barati  <sbarati@apple.com>
+
+        ValueRep(DoubleRep(@v)) can not simply convert to @v
+        https://bugs.webkit.org/show_bug.cgi?id=173687
+        <rdar://problem/32855563>
+
+        Reviewed by Mark Lam.
+
+        Consider this IR:
+         block#x
+          p: Phi() // int32 and double flows into this phi from various control flow
+          d: DoubleRep(@p)
+          some uses of @d here
+          v: ValueRep(DoubleRepUse:@d)
+          a: NewArrayWithSize(Int32:@v)
+          some more nodes here ...
+        
+        Because the flow of ValueRep(DoubleRep(@p)) will not produce an Int32,
+        AI proves that the Int32 check will fail. Constant folding phase removes
+        all nodes after @a and inserts an Unreachable after the NewArrayWithSize node.
+        
+        The IR then looks like this:
+        block#x
+          p: Phi() // int32 and double flows into this phi from various control flow
+          d: DoubleRep(@p)
+          some uses of @d here
+          v: ValueRep(DoubleRepUse:@d)
+          a: NewArrayWithSize(Int32:@v)
+          Unreachable
+        
+        However, there was a strength reduction rule that tries eliminate redundant
+        conversions. It used to convert the program to:
+        block#x
+          p: Phi() // int32 and double flows into this phi from various control flow
+          d: DoubleRep(@p)
+          some uses of @d here
+          a: NewArrayWithSize(Int32:@p)
+          Unreachable
+        
+        However, at runtime, @p will actually be an Int32, so @a will not OSR exit,
+        and we'll crash. This patch removes this strength reduction rule since it
+        does not maintain what would have happened if we executed the program before
+        the rule.
+        
+        This rule is also wrong for other types of programs (I'm not sure we'd
+        actually emit this code, but if such IR were generated, we would previously
+        optimize it incorrectly):
+        @a: Constant(JSTrue)
+        @b: DoubleRep(@a)
+        @c: ValueRep(@b)
+        @d: use(@c)
+        
+        However, the strength reduction rule would've transformed this into:
+        @a: Constant(JSTrue)
+        @d: use(@a)
+        
+        And this would be wrong because node @c before the transformation would
+        have produced the JSValue jsNumber(1.0).
+        
+        This patch was neutral in the benchmark run I did.
+
+        * dfg/DFGStrengthReductionPhase.cpp:
+        (JSC::DFG::StrengthReductionPhase::handleNode):
+
 2017-06-22  JF Bastien  <jfbastien@apple.com>
 
         ARM64: doubled executable memory limit from 32MiB to 64MiB