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

Changeset 245352 in webkit


Ignore:
Timestamp:
May 15, 2019, 2:44:54 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r245047. rdar://problem/50753944

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

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

Location:
branches/safari-607-branch
Files:
1 added
4 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-607-branch/JSTests/ChangeLog

    r245351 r245352  
     12019-05-14  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245047. rdar://problem/50753944
     4
     5    JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     6    https://bugs.webkit.org/show_bug.cgi?id=197479
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.
     13    (shouldBe):
     14   
     15    Source/JavaScriptCore:
     16   
     17    Our peephole optimization in BytecodeGenerator is (1) rewinding the previous instruction and (2) emit optimized instruction instead.
     18    If we have jump target between the previous instruction and the subsequent instruction, this peephole optimization breaks the jump target.
     19    To prevent it, we had a mechanism disabling peephole optimization, setting m_lastOpcodeID = op_end and checking m_lastOpcodeID when performing
     20    peephole optimization. However, BytecodeGenerator::emitEqualityOpImpl checks `m_lastInstruction->is<OpTypeof>` instead of `m_lastOpcodeID == op_typeof`,
     21    and miss `op_end` case.
     22   
     23    This patch makes the following changes.
     24   
     25    1. Add canDoPeepholeOptimization method to clarify the intent of `m_lastInstruction = op_end`.
     26    2. Check canDoPeepholeOptimization status before performing peephole optimization in emitJumpIfTrue, emitJumpIfFalse, and emitEqualityOpImpl.
     27    3. Add `ASSERT(canDoPeepholeOptimization())` in fuseCompareAndJump and fuseTestAndJmp to ensure that peephole optimization is allowed.
     28   
     29    * bytecompiler/BytecodeGenerator.cpp:
     30    (JSC::BytecodeGenerator::fuseCompareAndJump):
     31    (JSC::BytecodeGenerator::fuseTestAndJmp):
     32    (JSC::BytecodeGenerator::emitJumpIfTrue):
     33    (JSC::BytecodeGenerator::emitJumpIfFalse):
     34    (JSC::BytecodeGenerator::emitEqualityOpImpl):
     35    * bytecompiler/BytecodeGenerator.h:
     36    (JSC::BytecodeGenerator::canDoPeepholeOptimization const):
     37   
     38    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245047 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     39
     40    2019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     41
     42            JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     43            https://bugs.webkit.org/show_bug.cgi?id=197479
     44
     45            Reviewed by Saam Barati.
     46
     47            * stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.
     48            (shouldBe):
     49
    1502019-05-14  Kocsen Chung  <kocsen_chung@apple.com>
    251

  • branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

    r245351 r245352  
     12019-05-14  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r245047. rdar://problem/50753944
     4
     5    JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     6    https://bugs.webkit.org/show_bug.cgi?id=197479
     7   
     8    Reviewed by Saam Barati.
     9   
     10    JSTests:
     11   
     12    * stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.
     13    (shouldBe):
     14   
     15    Source/JavaScriptCore:
     16   
     17    Our peephole optimization in BytecodeGenerator is (1) rewinding the previous instruction and (2) emit optimized instruction instead.
     18    If we have jump target between the previous instruction and the subsequent instruction, this peephole optimization breaks the jump target.
     19    To prevent it, we had a mechanism disabling peephole optimization, setting m_lastOpcodeID = op_end and checking m_lastOpcodeID when performing
     20    peephole optimization. However, BytecodeGenerator::emitEqualityOpImpl checks `m_lastInstruction->is<OpTypeof>` instead of `m_lastOpcodeID == op_typeof`,
     21    and miss `op_end` case.
     22   
     23    This patch makes the following changes.
     24   
     25    1. Add canDoPeepholeOptimization method to clarify the intent of `m_lastInstruction = op_end`.
     26    2. Check canDoPeepholeOptimization status before performing peephole optimization in emitJumpIfTrue, emitJumpIfFalse, and emitEqualityOpImpl.
     27    3. Add `ASSERT(canDoPeepholeOptimization())` in fuseCompareAndJump and fuseTestAndJmp to ensure that peephole optimization is allowed.
     28   
     29    * bytecompiler/BytecodeGenerator.cpp:
     30    (JSC::BytecodeGenerator::fuseCompareAndJump):
     31    (JSC::BytecodeGenerator::fuseTestAndJmp):
     32    (JSC::BytecodeGenerator::emitJumpIfTrue):
     33    (JSC::BytecodeGenerator::emitJumpIfFalse):
     34    (JSC::BytecodeGenerator::emitEqualityOpImpl):
     35    * bytecompiler/BytecodeGenerator.h:
     36    (JSC::BytecodeGenerator::canDoPeepholeOptimization const):
     37   
     38    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245047 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     39
     40    2019-05-07  Yusuke Suzuki  <ysuzuki@apple.com>
     41
     42            JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
     43            https://bugs.webkit.org/show_bug.cgi?id=197479
     44
     45            Reviewed by Saam Barati.
     46
     47            Our peephole optimization in BytecodeGenerator is (1) rewinding the previous instruction and (2) emit optimized instruction instead.
     48            If we have jump target between the previous instruction and the subsequent instruction, this peephole optimization breaks the jump target.
     49            To prevent it, we had a mechanism disabling peephole optimization, setting m_lastOpcodeID = op_end and checking m_lastOpcodeID when performing
     50            peephole optimization. However, BytecodeGenerator::emitEqualityOpImpl checks `m_lastInstruction->is<OpTypeof>` instead of `m_lastOpcodeID == op_typeof`,
     51            and miss `op_end` case.
     52
     53            This patch makes the following changes.
     54
     55            1. Add canDoPeepholeOptimization method to clarify the intent of `m_lastInstruction = op_end`.
     56            2. Check canDoPeepholeOptimization status before performing peephole optimization in emitJumpIfTrue, emitJumpIfFalse, and emitEqualityOpImpl.
     57            3. Add `ASSERT(canDoPeepholeOptimization())` in fuseCompareAndJump and fuseTestAndJmp to ensure that peephole optimization is allowed.
     58
     59            * bytecompiler/BytecodeGenerator.cpp:
     60            (JSC::BytecodeGenerator::fuseCompareAndJump):
     61            (JSC::BytecodeGenerator::fuseTestAndJmp):
     62            (JSC::BytecodeGenerator::emitJumpIfTrue):
     63            (JSC::BytecodeGenerator::emitJumpIfFalse):
     64            (JSC::BytecodeGenerator::emitEqualityOpImpl):
     65            * bytecompiler/BytecodeGenerator.h:
     66            (JSC::BytecodeGenerator::canDoPeepholeOptimization const):
     67
    1682019-05-14  Kocsen Chung  <kocsen_chung@apple.com>
    269

  • branches/safari-607-branch/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r240399 r245352  
    13881388bool BytecodeGenerator::fuseCompareAndJump(RegisterID* cond, Label& target, bool swapOperands)
    13891389{
     1390    ASSERT(canDoPeepholeOptimization());
    13901391    auto binop = m_lastInstruction->as<BinOp>();
    13911392    if (cond->index() == binop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
    … …  
    14041405bool BytecodeGenerator::fuseTestAndJmp(RegisterID* cond, Label& target)
    14051406{
     1407    ASSERT(canDoPeepholeOptimization());
    14061408    auto unop = m_lastInstruction->as<UnaryOp>();
    14071409    if (cond->index() == unop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
    … …  
    14161418void BytecodeGenerator::emitJumpIfTrue(RegisterID* cond, Label& target)
    14171419{
    1418 
    1419     if (m_lastOpcodeID == op_less) {
    1420         if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
    1421             return;
    1422     } else if (m_lastOpcodeID == op_lesseq) {
    1423         if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
    1424             return;
    1425     } else if (m_lastOpcodeID == op_greater) {
    1426         if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
    1427             return;
    1428     } else if (m_lastOpcodeID == op_greatereq) {
    1429         if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
    1430             return;
    1431     } else if (m_lastOpcodeID == op_eq) {
    1432         if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
    1433             return;
    1434     } else if (m_lastOpcodeID == op_stricteq) {
    1435         if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
    1436             return;
    1437     } else if (m_lastOpcodeID == op_neq) {
    1438         if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
    1439             return;
    1440     } else if (m_lastOpcodeID == op_nstricteq) {
    1441         if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
    1442             return;
    1443     } else if (m_lastOpcodeID == op_below) {
    1444         if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
    1445             return;
    1446     } else if (m_lastOpcodeID == op_beloweq) {
    1447         if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
    1448             return;
    1449     } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
    1450         if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
    1451             return;
    1452     } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
    1453         if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
    1454             return;
     1420    if (canDoPeepholeOptimization()) {
     1421        if (m_lastOpcodeID == op_less) {
     1422            if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
     1423                return;
     1424        } else if (m_lastOpcodeID == op_lesseq) {
     1425            if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
     1426                return;
     1427        } else if (m_lastOpcodeID == op_greater) {
     1428            if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
     1429                return;
     1430        } else if (m_lastOpcodeID == op_greatereq) {
     1431            if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
     1432                return;
     1433        } else if (m_lastOpcodeID == op_eq) {
     1434            if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
     1435                return;
     1436        } else if (m_lastOpcodeID == op_stricteq) {
     1437            if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
     1438                return;
     1439        } else if (m_lastOpcodeID == op_neq) {
     1440            if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
     1441                return;
     1442        } else if (m_lastOpcodeID == op_nstricteq) {
     1443            if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
     1444                return;
     1445        } else if (m_lastOpcodeID == op_below) {
     1446            if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
     1447                return;
     1448        } else if (m_lastOpcodeID == op_beloweq) {
     1449            if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
     1450                return;
     1451        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
     1452            if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
     1453                return;
     1454        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
     1455            if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
     1456                return;
     1457        }
    14551458    }
    14561459
    … …  
    14601463void BytecodeGenerator::emitJumpIfFalse(RegisterID* cond, Label& target)
    14611464{
    1462     if (m_lastOpcodeID == op_less && target.isForward()) {
    1463         if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
    1464             return;
    1465     } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
    1466         if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
    1467             return;
    1468     } else if (m_lastOpcodeID == op_greater && target.isForward()) {
    1469         if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
    1470             return;
    1471     } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
    1472         if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
    1473             return;
    1474     } else if (m_lastOpcodeID == op_eq && target.isForward()) {
    1475         if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
    1476             return;
    1477     } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
    1478         if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
    1479             return;
    1480     } else if (m_lastOpcodeID == op_neq && target.isForward()) {
    1481         if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
    1482             return;
    1483     } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
    1484         if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
    1485             return;
    1486     } else if (m_lastOpcodeID == op_below && target.isForward()) {
    1487         if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
    1488             return;
    1489     } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
    1490         if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
    1491             return;
    1492     } else if (m_lastOpcodeID == op_not) {
    1493         if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
    1494             return;
    1495     } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
    1496         if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
    1497             return;
    1498     } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
    1499         if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
    1500             return;
     1465    if (canDoPeepholeOptimization()) {
     1466        if (m_lastOpcodeID == op_less && target.isForward()) {
     1467            if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
     1468                return;
     1469        } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
     1470            if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
     1471                return;
     1472        } else if (m_lastOpcodeID == op_greater && target.isForward()) {
     1473            if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
     1474                return;
     1475        } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
     1476            if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
     1477                return;
     1478        } else if (m_lastOpcodeID == op_eq && target.isForward()) {
     1479            if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
     1480                return;
     1481        } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
     1482            if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
     1483                return;
     1484        } else if (m_lastOpcodeID == op_neq && target.isForward()) {
     1485            if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
     1486                return;
     1487        } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
     1488            if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
     1489                return;
     1490        } else if (m_lastOpcodeID == op_below && target.isForward()) {
     1491            if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
     1492                return;
     1493        } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
     1494            if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
     1495                return;
     1496        } else if (m_lastOpcodeID == op_not) {
     1497            if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
     1498                return;
     1499        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
     1500            if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
     1501                return;
     1502        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
     1503            if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
     1504                return;
     1505        }
    15011506    }
    15021507
    … …  
    17071712RegisterID* BytecodeGenerator::emitEqualityOp(RegisterID* dst, RegisterID* src1, RegisterID* src2)
    17081713{
     1714    if (!canDoPeepholeOptimization())
     1715        return false;
     1716
    17091717    if (m_lastInstruction->is<OpTypeof>()) {
    17101718        auto op = m_lastInstruction->as<OpTypeof>();

  • branches/safari-607-branch/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r239427 r245352  
    10481048        RegisterID* emitMove(RegisterID* dst, RegisterID* src);
    10491049
     1050        bool canDoPeepholeOptimization() const { return m_lastOpcodeID != op_end; }
     1051
    10501052    public:
    10511053        bool isSuperUsedInInnerArrowFunction();
Note: See TracChangeset for help on using the changeset viewer.