Changeset 205364 in webkit


Ignore:
Timestamp:
Sep 2, 2016 12:49:09 PM (8 years ago)
Author:
commit-queue@webkit.org
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 <Caio Lima> 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):

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r205361 r205364  
     12016-09-02  Caio Lima  <ticaiolima@gmail.com>
     2
     3        Register usage optimization in mathIC when LHS and RHS are constants isn't configured correctly
     4        https://bugs.webkit.org/show_bug.cgi?id=160802
     5
     6        Reviewed by Saam Barati.
     7
     8        This patch is fixing a broken mechanism of MathIC that avoids allocate
     9        a register to LHS or RHS if one of these operands are proven as valid
     10        constant for JIT*Generator. In previous implementation, even if the
     11        JIT*Generator was not using an operand register because it was proven as a
     12        constant, compileMathIC and emitICFast were allocating a register for
     13        it. This was broken because mathIC->isLeftOperandValidConstant and
     14        mathIC->isLeftOperandValidConstant were being called before its Generator be
     15        properly initialized. We changed this mechanism to enable Generators write
     16        their validConstant rules using static methods isLeftOperandValidConstant(SnippetOperand)
     17        and isRightOperandValidConstant(SnippetOperand).
     18
     19        * dfg/DFGSpeculativeJIT.cpp:
     20        (JSC::DFG::SpeculativeJIT::compileMathIC):
     21        * jit/JITAddGenerator.h:
     22        (JSC::JITAddGenerator::JITAddGenerator):
     23        (JSC::JITAddGenerator::isLeftOperandValidConstant):
     24        (JSC::JITAddGenerator::isRightOperandValidConstant):
     25        * jit/JITArithmetic.cpp:
     26        (JSC::JIT::emitMathICFast):
     27        * jit/JITMathIC.h:
     28        * jit/JITMulGenerator.h:
     29        (JSC::JITMulGenerator::JITMulGenerator):
     30        (JSC::JITMulGenerator::isLeftOperandValidConstant):
     31        (JSC::JITMulGenerator::isRightOperandValidConstant):
     32        * jit/JITSubGenerator.h:
     33        (JSC::JITSubGenerator::isLeftOperandValidConstant):
     34        (JSC::JITSubGenerator::isRightOperandValidConstant):
     35
    1362016-09-02  JF Bastien  <jfbastien@apple.com>
    237

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r205112 r205364  
    34723472
    34733473    ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
    3474 
    3475     if (!mathIC->isLeftOperandValidConstant()) {
     3474    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
     3475
     3476    if (!Generator::isLeftOperandValidConstant(leftOperand)) {
    34763477        left = JSValueOperand(this, leftChild);
    34773478        leftRegs = left->jsValueRegs();
    34783479    }
    3479     if (!mathIC->isRightOperandValidConstant()) {
     3480    if (!Generator::isRightOperandValidConstant(rightOperand)) {
    34803481        right = JSValueOperand(this, rightChild);
    34813482        rightRegs = right->jsValueRegs();
    … …  
    35123513            auto innerLeftRegs = leftRegs;
    35133514            auto innerRightRegs = rightRegs;
    3514             if (mathIC->isLeftOperandValidConstant()) {
     3515            if (Generator::isLeftOperandValidConstant(leftOperand)) {
    35153516                innerLeftRegs = resultRegs;
    35163517                m_jit.moveValue(leftChild->asJSValue(), innerLeftRegs);
    3517             } else if (mathIC->isRightOperandValidConstant()) {
     3518            } else if (Generator::isRightOperandValidConstant(rightOperand)) {
    35183519                innerRightRegs = resultRegs;
    35193520                m_jit.moveValue(rightChild->asJSValue(), innerRightRegs);
    … …  
    35433544        });
    35443545    } else {
    3545         if (mathIC->isLeftOperandValidConstant()) {
     3546        if (Generator::isLeftOperandValidConstant(leftOperand)) {
    35463547            left = JSValueOperand(this, leftChild);
    35473548            leftRegs = left->jsValueRegs();
    3548         } else if (mathIC->isRightOperandValidConstant()) {
     3549        } else if (Generator::isRightOperandValidConstant(rightOperand)) {
    35493550            right = JSValueOperand(this, rightChild);
    35503551            rightRegs = right->jsValueRegs();

  • trunk/Source/JavaScriptCore/jit/JITAddGenerator.h

    r203786 r205364  
    4040class JITAddGenerator {
    4141public:
    42     JITAddGenerator()
    43     { }
     42    JITAddGenerator() { }
    4443
    4544    JITAddGenerator(SnippetOperand leftOperand, SnippetOperand rightOperand,
    … …  
    6463    bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowPathJumpList, bool shouldEmitProfiling);
    6564
    66     bool isLeftOperandValidConstant() const { return m_leftOperand.isConstInt32(); }
    67     bool isRightOperandValidConstant() const { return m_rightOperand.isConstInt32(); }
     65    static bool isLeftOperandValidConstant(SnippetOperand leftOperand) { return leftOperand.isPositiveConstInt32(); }
     66    static bool isRightOperandValidConstant(SnippetOperand rightOperand) { return rightOperand.isPositiveConstInt32(); }
     67   
    6868    ArithProfile* arithProfile() const { return m_arithProfile; }
    6969

  • trunk/Source/JavaScriptCore/jit/JITArithmetic.cpp

    r203979 r205364  
    735735    RELEASE_ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
    736736
    737     if (!mathIC->isLeftOperandValidConstant())
     737    mathIC->m_generator = Generator(leftOperand, rightOperand, resultRegs, leftRegs, rightRegs, fpRegT0, fpRegT1, scratchGPR, scratchFPR, arithProfile);
     738   
     739    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
     740   
     741    if (!Generator::isLeftOperandValidConstant(leftOperand))
    738742        emitGetVirtualRegister(op1, leftRegs);
    739     if (!mathIC->isRightOperandValidConstant())
     743    if (!Generator::isRightOperandValidConstant(rightOperand))
    740744        emitGetVirtualRegister(op2, rightRegs);
    741745
    … …  
    745749
    746750    MathICGenerationState& mathICGenerationState = m_instructionToMathICGenerationState.add(currentInstruction, MathICGenerationState()).iterator->value;
    747 
    748     mathIC->m_generator = Generator(leftOperand, rightOperand, resultRegs, leftRegs, rightRegs, fpRegT0, fpRegT1, scratchGPR, scratchFPR, arithProfile);
    749751
    750752    bool generatedInlineCode = mathIC->generateInline(*this, mathICGenerationState);
    … …  
    802804        rightOperand.setConstInt32(getOperandConstantInt(op2));
    803805
    804     if (mathIC->isLeftOperandValidConstant())
     806    ASSERT(!(Generator::isLeftOperandValidConstant(leftOperand) && Generator::isRightOperandValidConstant(rightOperand)));
     807
     808    if (Generator::isLeftOperandValidConstant(leftOperand))
    805809        emitGetVirtualRegister(op1, leftRegs);
    806     if (mathIC->isRightOperandValidConstant())
     810    else if (Generator::isRightOperandValidConstant(rightOperand))
    807811        emitGetVirtualRegister(op2, rightRegs);
    808812

  • trunk/Source/JavaScriptCore/jit/JITMathIC.h

    r205283 r205364  
    5959    CodeLocationLabel slowPathStartLocation() { return m_inlineStart.labelAtOffset(m_deltaFromStartToSlowPathStart); }
    6060    CodeLocationCall slowPathCallLocation() { return m_inlineStart.callAtOffset(m_deltaFromStartToSlowPathCallLocation); }
    61 
    62     bool isLeftOperandValidConstant() const { return m_generator.isLeftOperandValidConstant(); }
    63     bool isRightOperandValidConstant() const { return m_generator.isRightOperandValidConstant(); }
    64 
     61   
    6562    bool generateInline(CCallHelpers& jit, MathICGenerationState& state, bool shouldEmitProfiling = true)
    6663    {

  • trunk/Source/JavaScriptCore/jit/JITMulGenerator.h

    r203786 r205364  
    3939class JITMulGenerator {
    4040public:
    41     JITMulGenerator() { }
     41    JITMulGenerator()
     42    { }
    4243
    4344    JITMulGenerator(SnippetOperand leftOperand, SnippetOperand rightOperand,
    … …  
    6263    bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowJumpList, bool shouldEmitProfiling);
    6364
    64     bool isLeftOperandValidConstant() const { return m_leftOperand.isPositiveConstInt32(); }
    65     bool isRightOperandValidConstant() const { return m_rightOperand.isPositiveConstInt32(); }
    66 
     65    static bool isLeftOperandValidConstant(SnippetOperand leftOperand) { return leftOperand.isPositiveConstInt32(); }
     66    static bool isRightOperandValidConstant(SnippetOperand rightOperand) { return rightOperand.isPositiveConstInt32(); }
     67   
    6768    ArithProfile* arithProfile() const { return m_arithProfile; }
    6869

  • trunk/Source/JavaScriptCore/jit/JITSubGenerator.h

    r203979 r205364  
    6060    bool generateFastPath(CCallHelpers&, CCallHelpers::JumpList& endJumpList, CCallHelpers::JumpList& slowPathJumpList, bool shouldEmitProfiling);
    6161
    62     bool isLeftOperandValidConstant() const { return false; }
    63     bool isRightOperandValidConstant() const { return false; }
     62    static bool isLeftOperandValidConstant(SnippetOperand) { return false; }
     63    static bool isRightOperandValidConstant(SnippetOperand) { return false; }
     64
    6465    ArithProfile* arithProfile() const { return m_arithProfile; }
    6566
Note: See TracChangeset for help on using the changeset viewer.