Title: [205364] trunk/Source/_javascript_Core
Revision
205364
Author
[email protected]
Date
2016-09-02 12:49:09 -0700 (Fri, 02 Sep 2016)

Log Message

Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly
https://bugs.webkit.org/show_bug.cgi?id=160802

Patch by Caio Lima <[email protected]> on 2016-09-02
Reviewed by Saam Barati.

This patch is fixing a broken mechanism of MathIC that avoids allocate
a register to LHS or RHS if one of these operands are proven as valid
constant for JIT*Generator. In previous implementation, even if the
JIT*Generator was not using an operand register because it was proven as a
constant, compileMathIC and emitICFast were allocating a register for
it. This was broken because mathIC->isLeftOperandValidConstant and
mathIC->isLeftOperandValidConstant were being called before its Generator be
properly initialized. We changed this mechanism to enable Generators write
their validConstant rules using static methods isLeftOperandValidConstant(SnippetOperand)
and isRightOperandValidConstant(SnippetOperand).

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileMathIC):
* jit/JITAddGenerator.h:
(JSC::JITAddGenerator::JITAddGenerator):
(JSC::JITAddGenerator::isLeftOperandValidConstant):
(JSC::JITAddGenerator::isRightOperandValidConstant):
* jit/JITArithmetic.cpp:
(JSC::JIT::emitMathICFast):
* jit/JITMathIC.h:
* jit/JITMulGenerator.h:
(JSC::JITMulGenerator::JITMulGenerator):
(JSC::JITMulGenerator::isLeftOperandValidConstant):
(JSC::JITMulGenerator::isRightOperandValidConstant):
* jit/JITSubGenerator.h:
(JSC::JITSubGenerator::isLeftOperandValidConstant):
(JSC::JITSubGenerator::isRightOperandValidConstant):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (205363 => 205364)


--- trunk/Source/_javascript_Core/ChangeLog	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-09-02 19:49:09 UTC (rev 205364)
@@ -1,3 +1,38 @@
+2016-09-02  Caio Lima  <[email protected]>
+
+        Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly
+        https://bugs.webkit.org/show_bug.cgi?id=160802
+
+        Reviewed by Saam Barati.
+
+        This patch is fixing a broken mechanism of MathIC that avoids allocate
+        a register to LHS or RHS if one of these operands are proven as valid
+        constant for JIT*Generator. In previous implementation, even if the
+        JIT*Generator was not using an operand register because it was proven as a
+        constant, compileMathIC and emitICFast were allocating a register for
+        it. This was broken because mathIC->isLeftOperandValidConstant and
+        mathIC->isLeftOperandValidConstant were being called before its Generator be
+        properly initialized. We changed this mechanism to enable Generators write
+        their validConstant rules using static methods isLeftOperandValidConstant(SnippetOperand)
+        and isRightOperandValidConstant(SnippetOperand).
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileMathIC):
+        * jit/JITAddGenerator.h:
+        (JSC::JITAddGenerator::JITAddGenerator):
+        (JSC::JITAddGenerator::isLeftOperandValidConstant):
+        (JSC::JITAddGenerator::isRightOperandValidConstant):
+        * jit/JITArithmetic.cpp:
+        (JSC::JIT::emitMathICFast):
+        * jit/JITMathIC.h:
+        * jit/JITMulGenerator.h:
+        (JSC::JITMulGenerator::JITMulGenerator):
+        (JSC::JITMulGenerator::isLeftOperandValidConstant):
+        (JSC::JITMulGenerator::isRightOperandValidConstant):
+        * jit/JITSubGenerator.h:
+        (JSC::JITSubGenerator::isLeftOperandValidConstant):
+        (JSC::JITSubGenerator::isRightOperandValidConstant):
+
 2016-09-02  JF Bastien  <[email protected]>
 
         GetByValWithThis: fix opInfo in DFG creation

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (205363 => 205364)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2016-09-02 19:49:09 UTC (rev 205364)
@@ -3471,12 +3471,13 @@
         rightOperand.setConstInt32(rightChild->asInt32());
 
     ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
+    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
 
-    if (!mathIC->isLeftOperandValidConstant()) {
+    if (!Generator::isLeftOperandValidConstant(leftOperand)) {
         left = JSValueOperand(this, leftChild);
         leftRegs = left->jsValueRegs();
     }
-    if (!mathIC->isRightOperandValidConstant()) {
+    if (!Generator::isRightOperandValidConstant(rightOperand)) {
         right = JSValueOperand(this, rightChild);
         rightRegs = right->jsValueRegs();
     }
@@ -3511,10 +3512,10 @@
 
             auto innerLeftRegs = leftRegs;
             auto innerRightRegs = rightRegs;
-            if (mathIC->isLeftOperandValidConstant()) {
+            if (Generator::isLeftOperandValidConstant(leftOperand)) {
                 innerLeftRegs = resultRegs;
                 m_jit.moveValue(leftChild->asJSValue(), innerLeftRegs);
-            } else if (mathIC->isRightOperandValidConstant()) {
+            } else if (Generator::isRightOperandValidConstant(rightOperand)) {
                 innerRightRegs = resultRegs;
                 m_jit.moveValue(rightChild->asJSValue(), innerRightRegs);
             }
@@ -3542,10 +3543,10 @@
 
         });
     } else {
-        if (mathIC->isLeftOperandValidConstant()) {
+        if (Generator::isLeftOperandValidConstant(leftOperand)) {
             left = JSValueOperand(this, leftChild);
             leftRegs = left->jsValueRegs();
-        } else if (mathIC->isRightOperandValidConstant()) {
+        } else if (Generator::isRightOperandValidConstant(rightOperand)) {
             right = JSValueOperand(this, rightChild);
             rightRegs = right->jsValueRegs();
         }

Modified: trunk/Source/_javascript_Core/jit/JITAddGenerator.h (205363 => 205364)


--- trunk/Source/_javascript_Core/jit/JITAddGenerator.h	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/jit/JITAddGenerator.h	2016-09-02 19:49:09 UTC (rev 205364)
@@ -39,8 +39,7 @@
 
 class JITAddGenerator {
 public:
-    JITAddGenerator()
-    { }
+    JITAddGenerator() { }
 
     JITAddGenerator(SnippetOperand leftOperand, SnippetOperand rightOperand,
         JSValueRegs result, JSValueRegs left, JSValueRegs right,
@@ -63,8 +62,9 @@
     JITMathICInlineResult generateInline(CCallHelpers&, MathICGenerationState&);
     bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowPathJumpList, bool shouldEmitProfiling);
 
-    bool isLeftOperandValidConstant() const { return m_leftOperand.isConstInt32(); }
-    bool isRightOperandValidConstant() const { return m_rightOperand.isConstInt32(); }
+    static bool isLeftOperandValidConstant(SnippetOperand leftOperand) { return leftOperand.isPositiveConstInt32(); }
+    static bool isRightOperandValidConstant(SnippetOperand rightOperand) { return rightOperand.isPositiveConstInt32(); }
+    
     ArithProfile* arithProfile() const { return m_arithProfile; }
 
 private:

Modified: trunk/Source/_javascript_Core/jit/JITArithmetic.cpp (205363 => 205364)


--- trunk/Source/_javascript_Core/jit/JITArithmetic.cpp	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/jit/JITArithmetic.cpp	2016-09-02 19:49:09 UTC (rev 205364)
@@ -734,9 +734,13 @@
 
     RELEASE_ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
 
-    if (!mathIC->isLeftOperandValidConstant())
+    mathIC->m_generator = Generator(leftOperand, rightOperand, resultRegs, leftRegs, rightRegs, fpRegT0, fpRegT1, scratchGPR, scratchFPR, arithProfile);
+    
+    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
+    
+    if (!Generator::isLeftOperandValidConstant(leftOperand))
         emitGetVirtualRegister(op1, leftRegs);
-    if (!mathIC->isRightOperandValidConstant())
+    if (!Generator::isRightOperandValidConstant(rightOperand))
         emitGetVirtualRegister(op2, rightRegs);
 
 #if ENABLE(MATH_IC_STATS)
@@ -745,8 +749,6 @@
 
     MathICGenerationState& mathICGenerationState = m_instructionToMathICGenerationState.add(currentInstruction, MathICGenerationState()).iterator->value;
 
-    mathIC->m_generator = Generator(leftOperand, rightOperand, resultRegs, leftRegs, rightRegs, fpRegT0, fpRegT1, scratchGPR, scratchFPR, arithProfile);
-
     bool generatedInlineCode = mathIC->generateInline(*this, mathICGenerationState);
     if (!generatedInlineCode) {
         if (leftOperand.isConst())
@@ -801,9 +803,11 @@
     else if (isOperandConstantInt(op2))
         rightOperand.setConstInt32(getOperandConstantInt(op2));
 
-    if (mathIC->isLeftOperandValidConstant())
+    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
+
+    if (Generator::isLeftOperandValidConstant(leftOperand))
         emitGetVirtualRegister(op1, leftRegs);
-    if (mathIC->isRightOperandValidConstant())
+    else if (Generator::isRightOperandValidConstant(rightOperand))
         emitGetVirtualRegister(op2, rightRegs);
 
 #if ENABLE(MATH_IC_STATS)

Modified: trunk/Source/_javascript_Core/jit/JITMathIC.h (205363 => 205364)


--- trunk/Source/_javascript_Core/jit/JITMathIC.h	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/jit/JITMathIC.h	2016-09-02 19:49:09 UTC (rev 205364)
@@ -58,10 +58,7 @@
     CodeLocationLabel doneLocation() { return m_inlineStart.labelAtOffset(m_inlineSize); }
     CodeLocationLabel slowPathStartLocation() { return m_inlineStart.labelAtOffset(m_deltaFromStartToSlowPathStart); }
     CodeLocationCall slowPathCallLocation() { return m_inlineStart.callAtOffset(m_deltaFromStartToSlowPathCallLocation); }
-
-    bool isLeftOperandValidConstant() const { return m_generator.isLeftOperandValidConstant(); }
-    bool isRightOperandValidConstant() const { return m_generator.isRightOperandValidConstant(); }
-
+    
     bool generateInline(CCallHelpers& jit, MathICGenerationState& state, bool shouldEmitProfiling = true)
     {
 #if CPU(ARM_TRADITIONAL)

Modified: trunk/Source/_javascript_Core/jit/JITMulGenerator.h (205363 => 205364)


--- trunk/Source/_javascript_Core/jit/JITMulGenerator.h	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/jit/JITMulGenerator.h	2016-09-02 19:49:09 UTC (rev 205364)
@@ -38,7 +38,8 @@
 
 class JITMulGenerator {
 public:
-    JITMulGenerator() { }
+    JITMulGenerator()
+    { }
 
     JITMulGenerator(SnippetOperand leftOperand, SnippetOperand rightOperand,
         JSValueRegs result, JSValueRegs left, JSValueRegs right,
@@ -61,9 +62,9 @@
     JITMathICInlineResult generateInline(CCallHelpers&, MathICGenerationState&);
     bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowJumpList, bool shouldEmitProfiling);
 
-    bool isLeftOperandValidConstant() const { return m_leftOperand.isPositiveConstInt32(); }
-    bool isRightOperandValidConstant() const { return m_rightOperand.isPositiveConstInt32(); }
-
+    static bool isLeftOperandValidConstant(SnippetOperand leftOperand) { return leftOperand.isPositiveConstInt32(); }
+    static bool isRightOperandValidConstant(SnippetOperand rightOperand) { return rightOperand.isPositiveConstInt32(); }
+    
     ArithProfile* arithProfile() const { return m_arithProfile; }
 
 private:

Modified: trunk/Source/_javascript_Core/jit/JITSubGenerator.h (205363 => 205364)


--- trunk/Source/_javascript_Core/jit/JITSubGenerator.h	2016-09-02 19:42:23 UTC (rev 205363)
+++ trunk/Source/_javascript_Core/jit/JITSubGenerator.h	2016-09-02 19:49:09 UTC (rev 205364)
@@ -59,8 +59,9 @@
     JITMathICInlineResult generateInline(CCallHelpers&, MathICGenerationState&);
     bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowPathJumpList, bool shouldEmitProfiling);
 
-    bool isLeftOperandValidConstant() const { return false; }
-    bool isRightOperandValidConstant() const { return false; }
+    static bool isLeftOperandValidConstant(SnippetOperand) { return false; }
+    static bool isRightOperandValidConstant(SnippetOperand) { return false; }
+
     ArithProfile* arithProfile() const { return m_arithProfile; }
 
 private:
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to