Title: [226806] trunk
Revision
226806
Author
sbar...@apple.com
Date
2018-01-11 14:18:17 -0800 (Thu, 11 Jan 2018)

Log Message

JITMathIC code in the FTL is wrong when code gets duplicated
https://bugs.webkit.org/show_bug.cgi?id=181525
<rdar://problem/36351993>

Reviewed by Michael Saboff and Keith Miller.

JSTests:

* stress/allow-math-ic-b3-code-duplication.js: Added.

Source/_javascript_Core:

B3/Air may duplicate code for various reasons. Patchpoint generators inside
FTLLower must be aware that they can be called multiple times because of this.
The patchpoint for math ICs was not aware of this, and shared state amongst
all invocations of the patchpoint's generator. This patch fixes this bug so
that each invocation of the patchpoint's generator gets a unique math IC.

* bytecode/CodeBlock.h:
(JSC::CodeBlock::addMathIC):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileValueAdd):
(JSC::FTL::DFG::LowerDFGToB3::compileUnaryMathIC):
(JSC::FTL::DFG::LowerDFGToB3::compileBinaryMathIC):
(JSC::FTL::DFG::LowerDFGToB3::compileArithAddOrSub):
(JSC::FTL::DFG::LowerDFGToB3::compileArithMul):
(JSC::FTL::DFG::LowerDFGToB3::compileArithNegate):
(JSC::FTL::DFG::LowerDFGToB3::compileMathIC): Deleted.
* jit/JITMathIC.h:
(JSC::isProfileEmpty):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (226805 => 226806)


--- trunk/JSTests/ChangeLog	2018-01-11 22:14:38 UTC (rev 226805)
+++ trunk/JSTests/ChangeLog	2018-01-11 22:18:17 UTC (rev 226806)
@@ -1,5 +1,15 @@
 2018-01-11  Saam Barati  <sbar...@apple.com>
 
+        JITMathIC code in the FTL is wrong when code gets duplicated
+        https://bugs.webkit.org/show_bug.cgi?id=181525
+        <rdar://problem/36351993>
+
+        Reviewed by Michael Saboff and Keith Miller.
+
+        * stress/allow-math-ic-b3-code-duplication.js: Added.
+
+2018-01-11  Saam Barati  <sbar...@apple.com>
+
         Our for-in caching is wrong when we add indexed properties on things in the prototype chain
         https://bugs.webkit.org/show_bug.cgi?id=181508
 

Added: trunk/JSTests/stress/allow-math-ic-b3-code-duplication.js (0 => 226806)


--- trunk/JSTests/stress/allow-math-ic-b3-code-duplication.js	                        (rev 0)
+++ trunk/JSTests/stress/allow-math-ic-b3-code-duplication.js	2018-01-11 22:18:17 UTC (rev 226806)
@@ -0,0 +1,35 @@
+function test1() {
+    var o1;
+    for (let i = 0; i < 1000000; ++i) {
+        var o2 = { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { } } } } } } } } } } } } };
+    }
+    return -o2;
+}
+test1();
+
+function test2() {
+    var o1;
+    for (let i = 0; i < 1000000; ++i) {
+        var o2 = { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { } } } } } } } } } } } } };
+    }
+    return o1 - o2;
+}
+test2();
+
+function test3() {
+    var o1;
+    for (let i = 0; i < 1000000; ++i) {
+        var o2 = { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { } } } } } } } } } } } } };
+    }
+    return o1 + o2;
+}
+test3();
+
+function test4() {
+    var o1;
+    for (let i = 0; i < 1000000; ++i) {
+        var o2 = { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { f: { } } } } } } } } } } } } };
+    }
+    return o1 * o2;
+}
+test4();

Modified: trunk/Source/_javascript_Core/ChangeLog (226805 => 226806)


--- trunk/Source/_javascript_Core/ChangeLog	2018-01-11 22:14:38 UTC (rev 226805)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-01-11 22:18:17 UTC (rev 226806)
@@ -1,3 +1,30 @@
+2018-01-11  Saam Barati  <sbar...@apple.com>
+
+        JITMathIC code in the FTL is wrong when code gets duplicated
+        https://bugs.webkit.org/show_bug.cgi?id=181525
+        <rdar://problem/36351993>
+
+        Reviewed by Michael Saboff and Keith Miller.
+
+        B3/Air may duplicate code for various reasons. Patchpoint generators inside
+        FTLLower must be aware that they can be called multiple times because of this.
+        The patchpoint for math ICs was not aware of this, and shared state amongst
+        all invocations of the patchpoint's generator. This patch fixes this bug so
+        that each invocation of the patchpoint's generator gets a unique math IC.
+
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::addMathIC):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileValueAdd):
+        (JSC::FTL::DFG::LowerDFGToB3::compileUnaryMathIC):
+        (JSC::FTL::DFG::LowerDFGToB3::compileBinaryMathIC):
+        (JSC::FTL::DFG::LowerDFGToB3::compileArithAddOrSub):
+        (JSC::FTL::DFG::LowerDFGToB3::compileArithMul):
+        (JSC::FTL::DFG::LowerDFGToB3::compileArithNegate):
+        (JSC::FTL::DFG::LowerDFGToB3::compileMathIC): Deleted.
+        * jit/JITMathIC.h:
+        (JSC::isProfileEmpty):
+
 2018-01-11  Michael Saboff  <msab...@apple.com>
 
         Ensure there are no unsafe uses of MacroAssemblerARM64::dataTempRegister

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (226805 => 226806)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2018-01-11 22:14:38 UTC (rev 226805)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2018-01-11 22:18:17 UTC (rev 226806)
@@ -249,11 +249,24 @@
     void getByValInfoMap(ByValInfoMap& result);
     
 #if ENABLE(JIT)
-    StructureStubInfo* addStubInfo(AccessType);
     JITAddIC* addJITAddIC(ArithProfile*);
     JITMulIC* addJITMulIC(ArithProfile*);
     JITNegIC* addJITNegIC(ArithProfile*);
     JITSubIC* addJITSubIC(ArithProfile*);
+
+    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITAddGenerator>::value>::type>
+    JITAddIC* addMathIC(ArithProfile* profile) { return addJITAddIC(profile); }
+
+    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITMulGenerator>::value>::type>
+    JITMulIC* addMathIC(ArithProfile* profile) { return addJITMulIC(profile); }
+
+    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITNegGenerator>::value>::type>
+    JITNegIC* addMathIC(ArithProfile* profile) { return addJITNegIC(profile); }
+
+    template <typename Generator, typename = typename std::enable_if<std::is_same<Generator, JITSubGenerator>::value>::type>
+    JITSubIC* addMathIC(ArithProfile* profile) { return addJITSubIC(profile); }
+
+    StructureStubInfo* addStubInfo(AccessType);
     auto stubInfoBegin() { return m_stubInfos.begin(); }
     auto stubInfoEnd() { return m_stubInfos.end(); }
 

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (226805 => 226806)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-01-11 22:14:38 UTC (rev 226805)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2018-01-11 22:18:17 UTC (rev 226806)
@@ -1776,14 +1776,13 @@
     void compileValueAdd()
     {
         ArithProfile* arithProfile = m_ftlState.graph.baselineCodeBlockFor(m_node->origin.semantic)->arithProfileForBytecodeOffset(m_node->origin.semantic.bytecodeIndex);
-        JITAddIC* addIC = codeBlock()->addJITAddIC(arithProfile);
         auto repatchingFunction = operationValueAddOptimize;
         auto nonRepatchingFunction = operationValueAdd;
-        compileMathIC(addIC, repatchingFunction, nonRepatchingFunction);
+        compileBinaryMathIC<JITAddGenerator>(arithProfile, repatchingFunction, nonRepatchingFunction);
     }
 
     template <typename Generator>
-    void compileMathIC(JITUnaryMathIC<Generator>* mathIC, FunctionPtr repatchingFunction, FunctionPtr nonRepatchingFunction)
+    void compileUnaryMathIC(ArithProfile* arithProfile, FunctionPtr repatchingFunction, FunctionPtr nonRepatchingFunction)
     {
         Node* node = m_node;
 
@@ -1809,6 +1808,7 @@
 #endif
 
                 Box<MathICGenerationState> mathICGenerationState = Box<MathICGenerationState>::create();
+                JITUnaryMathIC<Generator>* mathIC = jit.codeBlock()->addMathIC<Generator>(arithProfile);
                 mathIC->m_generator = Generator(JSValueRegs(params[0].gpr()), JSValueRegs(params[1].gpr()), params.gpScratch(0));
 
                 bool shouldEmitProfiling = false;
@@ -1867,7 +1867,7 @@
     }
 
     template <typename Generator>
-    void compileMathIC(JITBinaryMathIC<Generator>* mathIC, FunctionPtr repatchingFunction, FunctionPtr nonRepatchingFunction)
+    void compileBinaryMathIC(ArithProfile* arithProfile, FunctionPtr repatchingFunction, FunctionPtr nonRepatchingFunction)
     {
         Node* node = m_node;
         
@@ -1892,6 +1892,7 @@
             [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
                 AllowMacroScratchRegisterUsage allowScratch(jit);
 
+
                 Box<CCallHelpers::JumpList> exceptions =
                     exceptionHandle->scheduleExitCreation(params)->jumps(jit);
 
@@ -1900,6 +1901,7 @@
 #endif
 
                 Box<MathICGenerationState> mathICGenerationState = Box<MathICGenerationState>::create();
+                JITBinaryMathIC<Generator>* mathIC = jit.codeBlock()->addMathIC<Generator>(arithProfile);
                 mathIC->m_generator = Generator(leftOperand, rightOperand, JSValueRegs(params[0].gpr()),
                     JSValueRegs(params[1].gpr()), JSValueRegs(params[2].gpr()), params.fpScratch(0),
                     params.fpScratch(1), params.gpScratch(0), InvalidFPRReg);
@@ -2031,10 +2033,9 @@
             }
 
             ArithProfile* arithProfile = m_ftlState.graph.baselineCodeBlockFor(m_node->origin.semantic)->arithProfileForBytecodeOffset(m_node->origin.semantic.bytecodeIndex);
-            JITSubIC* subIC = codeBlock()->addJITSubIC(arithProfile);
             auto repatchingFunction = operationValueSubOptimize;
             auto nonRepatchingFunction = operationValueSub;
-            compileMathIC(subIC, repatchingFunction, nonRepatchingFunction);
+            compileBinaryMathIC<JITSubGenerator>(arithProfile, repatchingFunction, nonRepatchingFunction);
             break;
         }
 
@@ -2126,10 +2127,9 @@
 
         case UntypedUse: {
             ArithProfile* arithProfile = m_ftlState.graph.baselineCodeBlockFor(m_node->origin.semantic)->arithProfileForBytecodeOffset(m_node->origin.semantic.bytecodeIndex);
-            JITMulIC* mulIC = codeBlock()->addJITMulIC(arithProfile);
             auto repatchingFunction = operationValueMulOptimize;
             auto nonRepatchingFunction = operationValueMul;
-            compileMathIC(mulIC, repatchingFunction, nonRepatchingFunction);
+            compileBinaryMathIC<JITMulGenerator>(arithProfile, repatchingFunction, nonRepatchingFunction);
             break;
         }
 
@@ -2708,10 +2708,9 @@
         default:
             DFG_ASSERT(m_graph, m_node, m_node->child1().useKind() == UntypedUse);
             ArithProfile* arithProfile = m_ftlState.graph.baselineCodeBlockFor(m_node->origin.semantic)->arithProfileForBytecodeOffset(m_node->origin.semantic.bytecodeIndex);
-            JITNegIC* negIC = codeBlock()->addJITNegIC(arithProfile);
             auto repatchingFunction = operationArithNegateOptimize;
             auto nonRepatchingFunction = operationArithNegate;
-            compileMathIC(negIC, repatchingFunction, nonRepatchingFunction);
+            compileUnaryMathIC<JITNegGenerator>(arithProfile, repatchingFunction, nonRepatchingFunction);
             break;
         }
     }

Modified: trunk/Source/_javascript_Core/jit/JITMathIC.h (226805 => 226806)


--- trunk/Source/_javascript_Core/jit/JITMathIC.h	2018-01-11 22:14:38 UTC (rev 226805)
+++ trunk/Source/_javascript_Core/jit/JITMathIC.h	2018-01-11 22:18:17 UTC (rev 226806)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -54,6 +54,7 @@
 
 template <typename GeneratorType, bool(*isProfileEmpty)(ArithProfile&)>
 class JITMathIC {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     JITMathIC(ArithProfile* arithProfile)
         : m_arithProfile(arithProfile)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to