Changeset 249736 in webkit

Timestamp:

Sep 10, 2019 3:07:08 PM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] ResultType implementation is wrong for bit ops, and ends up making ArithDiv take the DFG Int32 fast path even if Baseline constantly produces Double result
​https://bugs.webkit.org/show_bug.cgi?id=198253

Reviewed by Mark Lam.

ResultType of bitwise operation needs to include TypeMaybeNumber. TypeInt32 is something like a flag indicating the number looks like a int32.
When it is specified, TypeMaybeNumber must exist too. This issue compiles op_div in JetStream2/async-fs slow-path. And eventually DFG first mis-compiles
it with Int32 ArithDiv while that div always produces double. And unnecessary OSR exit happens.

In this patch, we add TypeMaybeNumber to bigIntOrInt32Type correctly.

• parser/ResultType.h:

(JSC::ResultType::bigIntOrInt32Type):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• parser/ResultType.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-09-10  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ResultType implementation is wrong for bit ops, and ends up making ArithDiv take the DFG Int32 fast path even if Baseline constantly produces Double result
+        https://bugs.webkit.org/show_bug.cgi?id=198253
+
+        Reviewed by Mark Lam.
+
+        ResultType of bitwise operation needs to include TypeMaybeNumber. TypeInt32 is something like a flag indicating the number looks like a int32.
+        When it is specified, TypeMaybeNumber must exist too. This issue compiles op_div in JetStream2/async-fs slow-path. And eventually DFG first mis-compiles
+        it with Int32 ArithDiv while that div always produces double. And unnecessary OSR exit happens.
+
+        In this patch, we add TypeMaybeNumber to bigIntOrInt32Type correctly.
+
+        * parser/ResultType.h:
+        (JSC::ResultType::bigIntOrInt32Type):
+
 2019-09-10  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/parser/ResultType.h

@@ -28 +28 @@
 namespace JSC {
 
+    // FIXME: Consider whether this is actually necessary. Is LLInt and Baseline's profiling information enough?
+    // https://bugs.webkit.org/show_bug.cgi?id=201659
     struct ResultType {
     private:
@@ -149 +151 @@
         static constexpr ResultType bigIntOrInt32Type()
         {
-            return ResultType(TypeMaybeBigInt | TypeInt32);
+            return ResultType(TypeMaybeBigInt | TypeInt32 | TypeMaybeNumber);
         }