JITMathIC code in the FTL is wrong when code gets duplicated
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jan 2018 22:18:17 +0000 (22:18 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jan 2018 22:18:17 +0000 (22:18 +0000)
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/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226806 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/allow-math-ic-b3-code-duplication.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JITMathIC.h

index 83949236f489de2b28df4f312e844f4a2ec9f03a..b9b6c9a1b71d467941228250f098b13e7187a112 100644 (file)
@@ -1,3 +1,13 @@
+2018-01-11  Saam Barati  <sbarati@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  <sbarati@apple.com>
 
         Our for-in caching is wrong when we add indexed properties on things in the prototype chain
diff --git a/JSTests/stress/allow-math-ic-b3-code-duplication.js b/JSTests/stress/allow-math-ic-b3-code-duplication.js
new file mode 100644 (file)
index 0000000..2059b18
--- /dev/null
@@ -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();
index 4840c69fe7e7f6321a0479589b8561b7b93d258a..5def7341ce7145c6ea5c221c100e397bdf21e9d6 100644 (file)
@@ -1,3 +1,30 @@
+2018-01-11  Saam Barati  <sbarati@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  <msaboff@apple.com>
 
         Ensure there are no unsafe uses of MacroAssemblerARM64::dataTempRegister
index fb7e68ba97d57c7759fd76dbe93f6c981496b262..c7acb016513d9eae087b0af3476776ee8235eb69 100644 (file)
@@ -249,11 +249,24 @@ public:
     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(); }
 
index 9951cf78ef67f7a70be89714a6d48f38f130b2c5..55bafd51a576e836387ca3edab895b47b55d0cb0 100644 (file)
@@ -1776,14 +1776,13 @@ private:
     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 @@ private:
 #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 @@ private:
     }
 
     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 @@ private:
             [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
                 AllowMacroScratchRegisterUsage allowScratch(jit);
 
+
                 Box<CCallHelpers::JumpList> exceptions =
                     exceptionHandle->scheduleExitCreation(params)->jumps(jit);
 
@@ -1900,6 +1901,7 @@ private:
 #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 @@ private:
             }
 
             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 @@ private:
 
         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 @@ private:
         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;
         }
     }
index b885a66994b15f58346a3e0afbebbef625081508..b70d1314dc06b4fcc2cca7d06c6fc30aa262462f 100644 (file)
@@ -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 @@ struct MathICGenerationState {
 
 template <typename GeneratorType, bool(*isProfileEmpty)(ArithProfile&)>
 class JITMathIC {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     JITMathIC(ArithProfile* arithProfile)
         : m_arithProfile(arithProfile)