⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 245047 in webkit


Ignore:
Timestamp:
May 7, 2019, 6:23:22 PM (7 years ago)
Author:
ysuzuki@apple.com
Message:

JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
​https://bugs.webkit.org/show_bug.cgi?id=197479

Reviewed by Saam Barati.

JSTests:

  • stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.

(shouldBe):

Source/JavaScriptCore:

Our peephole optimization in BytecodeGenerator is (1) rewinding the previous instruction and (2) emit optimized instruction instead.
If we have jump target between the previous instruction and the subsequent instruction, this peephole optimization breaks the jump target.
To prevent it, we had a mechanism disabling peephole optimization, setting m_lastOpcodeID = op_end and checking m_lastOpcodeID when performing
peephole optimization. However, BytecodeGenerator::emitEqualityOpImpl checks m_lastInstruction->is<OpTypeof> instead of m_lastOpcodeID == op_typeof,
and miss op_end case.

This patch makes the following changes.

  1. Add canDoPeepholeOptimization method to clarify the intent of m_lastInstruction = op_end.
  2. Check canDoPeepholeOptimization status before performing peephole optimization in emitJumpIfTrue, emitJumpIfFalse, and emitEqualityOpImpl.
  3. Add ASSERT(canDoPeepholeOptimization()) in fuseCompareAndJump and fuseTestAndJmp to ensure that peephole optimization is allowed.
  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::fuseCompareAndJump):
(JSC::BytecodeGenerator::fuseTestAndJmp):
(JSC::BytecodeGenerator::emitJumpIfTrue):
(JSC::BytecodeGenerator::emitJumpIfFalse):
(JSC::BytecodeGenerator::emitEqualityOpImpl):

  • bytecompiler/BytecodeGenerator.h:

(JSC::BytecodeGenerator::canDoPeepholeOptimization const):

Location:
trunk
Files:
1 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r245040 r245047  
     12019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     4        https://bugs.webkit.org/show_bug.cgi?id=197479
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.
     9        (shouldBe):
     10
    1112019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r245040 r245047  
     12019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     4        https://bugs.webkit.org/show_bug.cgi?id=197479
     5
     6        Reviewed by Saam Barati.
     7
     8        Our peephole optimization in BytecodeGenerator is (1) rewinding the previous instruction and (2) emit optimized instruction instead.
     9        If we have jump target between the previous instruction and the subsequent instruction, this peephole optimization breaks the jump target.
     10        To prevent it, we had a mechanism disabling peephole optimization, setting m_lastOpcodeID = op_end and checking m_lastOpcodeID when performing
     11        peephole optimization. However, BytecodeGenerator::emitEqualityOpImpl checks `m_lastInstruction->is<OpTypeof>` instead of `m_lastOpcodeID == op_typeof`,
     12        and miss `op_end` case.
     13
     14        This patch makes the following changes.
     15
     16        1. Add canDoPeepholeOptimization method to clarify the intent of `m_lastInstruction = op_end`.
     17        2. Check canDoPeepholeOptimization status before performing peephole optimization in emitJumpIfTrue, emitJumpIfFalse, and emitEqualityOpImpl.
     18        3. Add `ASSERT(canDoPeepholeOptimization())` in fuseCompareAndJump and fuseTestAndJmp to ensure that peephole optimization is allowed.
     19
     20        * bytecompiler/BytecodeGenerator.cpp:
     21        (JSC::BytecodeGenerator::fuseCompareAndJump):
     22        (JSC::BytecodeGenerator::fuseTestAndJmp):
     23        (JSC::BytecodeGenerator::emitJumpIfTrue):
     24        (JSC::BytecodeGenerator::emitJumpIfFalse):
     25        (JSC::BytecodeGenerator::emitEqualityOpImpl):
     26        * bytecompiler/BytecodeGenerator.h:
     27        (JSC::BytecodeGenerator::canDoPeepholeOptimization const):
     28
    1292019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
    230

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r245040 r245047  
    14071407bool BytecodeGenerator::fuseCompareAndJump(RegisterID* cond, Label& target, bool swapOperands)
    14081408{
     1409    ASSERT(canDoPeepholeOptimization());
    14091410    auto binop = m_lastInstruction->as<BinOp>();
    14101411    if (cond->index() == binop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
    … …  
    14231424bool BytecodeGenerator::fuseTestAndJmp(RegisterID* cond, Label& target)
    14241425{
     1426    ASSERT(canDoPeepholeOptimization());
    14251427    auto unop = m_lastInstruction->as<UnaryOp>();
    14261428    if (cond->index() == unop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
    … …  
    14351437void BytecodeGenerator::emitJumpIfTrue(RegisterID* cond, Label& target)
    14361438{
    1437 
    1438     if (m_lastOpcodeID == op_less) {
    1439         if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
    1440             return;
    1441     } else if (m_lastOpcodeID == op_lesseq) {
    1442         if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
    1443             return;
    1444     } else if (m_lastOpcodeID == op_greater) {
    1445         if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
    1446             return;
    1447     } else if (m_lastOpcodeID == op_greatereq) {
    1448         if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
    1449             return;
    1450     } else if (m_lastOpcodeID == op_eq) {
    1451         if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
    1452             return;
    1453     } else if (m_lastOpcodeID == op_stricteq) {
    1454         if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
    1455             return;
    1456     } else if (m_lastOpcodeID == op_neq) {
    1457         if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
    1458             return;
    1459     } else if (m_lastOpcodeID == op_nstricteq) {
    1460         if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
    1461             return;
    1462     } else if (m_lastOpcodeID == op_below) {
    1463         if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
    1464             return;
    1465     } else if (m_lastOpcodeID == op_beloweq) {
    1466         if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
    1467             return;
    1468     } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
    1469         if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
    1470             return;
    1471     } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
    1472         if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
    1473             return;
     1439    if (canDoPeepholeOptimization()) {
     1440        if (m_lastOpcodeID == op_less) {
     1441            if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
     1442                return;
     1443        } else if (m_lastOpcodeID == op_lesseq) {
     1444            if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
     1445                return;
     1446        } else if (m_lastOpcodeID == op_greater) {
     1447            if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
     1448                return;
     1449        } else if (m_lastOpcodeID == op_greatereq) {
     1450            if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
     1451                return;
     1452        } else if (m_lastOpcodeID == op_eq) {
     1453            if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
     1454                return;
     1455        } else if (m_lastOpcodeID == op_stricteq) {
     1456            if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
     1457                return;
     1458        } else if (m_lastOpcodeID == op_neq) {
     1459            if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
     1460                return;
     1461        } else if (m_lastOpcodeID == op_nstricteq) {
     1462            if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
     1463                return;
     1464        } else if (m_lastOpcodeID == op_below) {
     1465            if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
     1466                return;
     1467        } else if (m_lastOpcodeID == op_beloweq) {
     1468            if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
     1469                return;
     1470        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
     1471            if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
     1472                return;
     1473        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
     1474            if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
     1475                return;
     1476        }
    14741477    }
    14751478
    … …  
    14791482void BytecodeGenerator::emitJumpIfFalse(RegisterID* cond, Label& target)
    14801483{
    1481     if (m_lastOpcodeID == op_less && target.isForward()) {
    1482         if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
    1483             return;
    1484     } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
    1485         if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
    1486             return;
    1487     } else if (m_lastOpcodeID == op_greater && target.isForward()) {
    1488         if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
    1489             return;
    1490     } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
    1491         if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
    1492             return;
    1493     } else if (m_lastOpcodeID == op_eq && target.isForward()) {
    1494         if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
    1495             return;
    1496     } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
    1497         if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
    1498             return;
    1499     } else if (m_lastOpcodeID == op_neq && target.isForward()) {
    1500         if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
    1501             return;
    1502     } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
    1503         if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
    1504             return;
    1505     } else if (m_lastOpcodeID == op_below && target.isForward()) {
    1506         if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
    1507             return;
    1508     } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
    1509         if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
    1510             return;
    1511     } else if (m_lastOpcodeID == op_not) {
    1512         if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
    1513             return;
    1514     } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
    1515         if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
    1516             return;
    1517     } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
    1518         if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
    1519             return;
     1484    if (canDoPeepholeOptimization()) {
     1485        if (m_lastOpcodeID == op_less && target.isForward()) {
     1486            if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
     1487                return;
     1488        } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
     1489            if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
     1490                return;
     1491        } else if (m_lastOpcodeID == op_greater && target.isForward()) {
     1492            if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
     1493                return;
     1494        } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
     1495            if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
     1496                return;
     1497        } else if (m_lastOpcodeID == op_eq && target.isForward()) {
     1498            if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
     1499                return;
     1500        } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
     1501            if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
     1502                return;
     1503        } else if (m_lastOpcodeID == op_neq && target.isForward()) {
     1504            if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
     1505                return;
     1506        } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
     1507            if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
     1508                return;
     1509        } else if (m_lastOpcodeID == op_below && target.isForward()) {
     1510            if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
     1511                return;
     1512        } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
     1513            if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
     1514                return;
     1515        } else if (m_lastOpcodeID == op_not) {
     1516            if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
     1517                return;
     1518        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
     1519            if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
     1520                return;
     1521        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
     1522            if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
     1523                return;
     1524        }
    15201525    }
    15211526
    … …  
    17251730bool BytecodeGenerator::emitEqualityOpImpl(RegisterID* dst, RegisterID* src1, RegisterID* src2)
    17261731{
     1732    if (!canDoPeepholeOptimization())
     1733        return false;
     1734
    17271735    if (m_lastInstruction->is<OpTypeof>()) {
    17281736        auto op = m_lastInstruction->as<OpTypeof>();

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r245040 r245047  
    10261026        RegisterID* emitMove(RegisterID* dst, RegisterID* src);
    10271027
     1028        bool canDoPeepholeOptimization() const { return m_lastOpcodeID != op_end; }
     1029
    10281030    public:
    10291031        bool isSuperUsedInInnerArrowFunction();
Note: See TracChangeset for help on using the changeset viewer.