Title: [245410] branches/safari-607.2.1.2-branch

Diff

Modified: branches/safari-607.2.1.2-branch/JSTests/ChangeLog (245409 => 245410)


--- branches/safari-607.2.1.2-branch/JSTests/ChangeLog	2019-05-16 22:47:52 UTC (rev 245409)
+++ branches/safari-607.2.1.2-branch/JSTests/ChangeLog	2019-05-16 22:47:56 UTC (rev 245410)
@@ -1,54 +1,5 @@
 2019-05-15  Alan Coon  <[email protected]>
 
-        Cherry-pick r245047. rdar://problem/50754976
-
-    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/_javascript_Core:
-    
-    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
-
-    2019-05-07  Yusuke Suzuki  <[email protected]>
-
-            JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
-            https://bugs.webkit.org/show_bug.cgi?id=197479
-
-            Reviewed by Saam Barati.
-
-            * stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js: Added.
-            (shouldBe):
-
-2019-05-15  Alan Coon  <[email protected]>
-
         Cherry-pick r244996. rdar://problem/50754980
 
     [JSC] We should check OOM for description string of Symbol

Deleted: branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js (245409 => 245410)


--- branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js	2019-05-16 22:47:52 UTC (rev 245409)
+++ branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js	2019-05-16 22:47:56 UTC (rev 245410)
@@ -1,7 +0,0 @@
-function shouldBe(actual, expected) {
-    if (actual !== expected)
-        throw new Error('bad value: ' + actual);
-}
-
-let a = (1 || typeof 1) === 'string';
-shouldBe(a, false);

Modified: branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog (245409 => 245410)


--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog	2019-05-16 22:47:52 UTC (rev 245409)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog	2019-05-16 22:47:56 UTC (rev 245410)
@@ -1,72 +1,5 @@
 2019-05-15  Alan Coon  <[email protected]>
 
-        Cherry-pick r245047. rdar://problem/50754976
-
-    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/_javascript_Core:
-    
-    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
-
-    2019-05-07  Yusuke Suzuki  <[email protected]>
-
-            JSC: A bug in BytecodeGenerator::emitEqualityOpImpl
-            https://bugs.webkit.org/show_bug.cgi?id=197479
-
-            Reviewed by Saam Barati.
-
-            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):
-
-2019-05-15  Alan Coon  <[email protected]>
-
         Cherry-pick r244996. rdar://problem/50754980
 
     [JSC] We should check OOM for description string of Symbol

Modified: branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (245409 => 245410)


--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2019-05-16 22:47:52 UTC (rev 245409)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2019-05-16 22:47:56 UTC (rev 245410)
@@ -1387,7 +1387,6 @@
 template<typename BinOp, typename JmpOp>
 bool BytecodeGenerator::fuseCompareAndJump(RegisterID* cond, Label& target, bool swapOperands)
 {
-    ASSERT(canDoPeepholeOptimization());
     auto binop = m_lastInstruction->as<BinOp>();
     if (cond->index() == binop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
         rewind();
@@ -1404,7 +1403,6 @@
 template<typename UnaryOp, typename JmpOp>
 bool BytecodeGenerator::fuseTestAndJmp(RegisterID* cond, Label& target)
 {
-    ASSERT(canDoPeepholeOptimization());
     auto unop = m_lastInstruction->as<UnaryOp>();
     if (cond->index() == unop.m_dst.offset() && cond->isTemporary() && !cond->refCount()) {
         rewind();
@@ -1417,44 +1415,43 @@
 
 void BytecodeGenerator::emitJumpIfTrue(RegisterID* cond, Label& target)
 {
-    if (canDoPeepholeOptimization()) {
-        if (m_lastOpcodeID == op_less) {
-            if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_lesseq) {
-            if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_greater) {
-            if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_greatereq) {
-            if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_eq) {
-            if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_stricteq) {
-            if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_neq) {
-            if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_nstricteq) {
-            if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_below) {
-            if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_beloweq) {
-            if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
-            if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
-            if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
-                return;
-        }
+
+    if (m_lastOpcodeID == op_less) {
+        if (fuseCompareAndJump<OpLess, OpJless>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_lesseq) {
+        if (fuseCompareAndJump<OpLesseq, OpJlesseq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_greater) {
+        if (fuseCompareAndJump<OpGreater, OpJgreater>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_greatereq) {
+        if (fuseCompareAndJump<OpGreatereq, OpJgreatereq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_eq) {
+        if (fuseCompareAndJump<OpEq, OpJeq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_stricteq) {
+        if (fuseCompareAndJump<OpStricteq, OpJstricteq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_neq) {
+        if (fuseCompareAndJump<OpNeq, OpJneq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_nstricteq) {
+        if (fuseCompareAndJump<OpNstricteq, OpJnstricteq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_below) {
+        if (fuseCompareAndJump<OpBelow, OpJbelow>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_beloweq) {
+        if (fuseCompareAndJump<OpBeloweq, OpJbeloweq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
+        if (fuseTestAndJmp<OpEqNull, OpJeqNull>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
+        if (fuseTestAndJmp<OpNeqNull, OpJneqNull>(cond, target))
+            return;
     }
 
     OpJtrue::emit(this, cond, target.bind(this));
@@ -1462,47 +1459,45 @@
 
 void BytecodeGenerator::emitJumpIfFalse(RegisterID* cond, Label& target)
 {
-    if (canDoPeepholeOptimization()) {
-        if (m_lastOpcodeID == op_less && target.isForward()) {
-            if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
-            if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_greater && target.isForward()) {
-            if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
-            if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_eq && target.isForward()) {
-            if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
-            if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_neq && target.isForward()) {
-            if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
-            if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_below && target.isForward()) {
-            if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
-                return;
-        } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
-            if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
-                return;
-        } else if (m_lastOpcodeID == op_not) {
-            if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
-            if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
-                return;
-        } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
-            if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
-                return;
-        }
+    if (m_lastOpcodeID == op_less && target.isForward()) {
+        if (fuseCompareAndJump<OpLess, OpJnless>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_lesseq && target.isForward()) {
+        if (fuseCompareAndJump<OpLesseq, OpJnlesseq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_greater && target.isForward()) {
+        if (fuseCompareAndJump<OpGreater, OpJngreater>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_greatereq && target.isForward()) {
+        if (fuseCompareAndJump<OpGreatereq, OpJngreatereq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_eq && target.isForward()) {
+        if (fuseCompareAndJump<OpEq, OpJneq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_stricteq && target.isForward()) {
+        if (fuseCompareAndJump<OpStricteq, OpJnstricteq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_neq && target.isForward()) {
+        if (fuseCompareAndJump<OpNeq, OpJeq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_nstricteq && target.isForward()) {
+        if (fuseCompareAndJump<OpNstricteq, OpJstricteq>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_below && target.isForward()) {
+        if (fuseCompareAndJump<OpBelow, OpJbeloweq>(cond, target, true))
+            return;
+    } else if (m_lastOpcodeID == op_beloweq && target.isForward()) {
+        if (fuseCompareAndJump<OpBeloweq, OpJbelow>(cond, target, true))
+            return;
+    } else if (m_lastOpcodeID == op_not) {
+        if (fuseTestAndJmp<OpNot, OpJtrue>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_eq_null && target.isForward()) {
+        if (fuseTestAndJmp<OpEqNull, OpJneqNull>(cond, target))
+            return;
+    } else if (m_lastOpcodeID == op_neq_null && target.isForward()) {
+        if (fuseTestAndJmp<OpNeqNull, OpJeqNull>(cond, target))
+            return;
     }
 
     OpJfalse::emit(this, cond, target.bind(this));
@@ -1711,9 +1706,6 @@
 template<typename EqOp>
 RegisterID* BytecodeGenerator::emitEqualityOp(RegisterID* dst, RegisterID* src1, RegisterID* src2)
 {
-    if (!canDoPeepholeOptimization())
-        return false;
-
     if (m_lastInstruction->is<OpTypeof>()) {
         auto op = m_lastInstruction->as<OpTypeof>();
         if (src1->index() == op.m_dst.offset()

Modified: branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (245409 => 245410)


--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2019-05-16 22:47:52 UTC (rev 245409)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2019-05-16 22:47:56 UTC (rev 245410)
@@ -1047,8 +1047,6 @@
 
         RegisterID* emitMove(RegisterID* dst, RegisterID* src);
 
-        bool canDoPeepholeOptimization() const { return m_lastOpcodeID != op_end; }
-
     public:
         bool isSuperUsedInInnerArrowFunction();
         bool isSuperCallUsedInInnerArrowFunction();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to