[webkit-changes] [248220] releases/WebKitGTK/webkit-2.24

Sat, 03 Aug 2019 20:23:48 -0700

Title: [248220] releases/WebKitGTK/webkit-2.24
Revision
248220
Author
[email protected]
Date
2019-08-03 20:22:41 -0700 (Sat, 03 Aug 2019)

Log Message

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

Patch by Yusuke Suzuki <[email protected]> on 2019-05-07
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):

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog (248219 => 248220)


--- releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog	2019-08-04 03:22:38 UTC (rev 248219)
+++ releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog	2019-08-04 03:22:41 UTC (rev 248220)
@@ -1,3 +1,13 @@
+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-07  Tadeu Zagallo  <[email protected]>
 
         tryCachePutByID should not crash if target offset changes

Added: releases/WebKitGTK/webkit-2.24/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js (0 => 248220)


--- releases/WebKitGTK/webkit-2.24/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.24/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js	2019-08-04 03:22:41 UTC (rev 248220)
@@ -0,0 +1,7 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+let a = (1 || typeof 1) === 'string';
+shouldBe(a, false);

Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog (248219 => 248220)


--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog	2019-08-04 03:22:38 UTC (rev 248219)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog	2019-08-04 03:22:41 UTC (rev 248220)
@@ -1,3 +1,31 @@
+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-07  Tadeu Zagallo  <[email protected]>
 
         tryCachePutByID should not crash if target offset changes

Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (248219 => 248220)


--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2019-08-04 03:22:38 UTC (rev 248219)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2019-08-04 03:22:41 UTC (rev 248220)
@@ -1388,6 +1388,7 @@
 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,6 +1405,7 @@
 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();
@@ -1416,43 +1418,44 @@
 
 void BytecodeGenerator::emitJumpIfTrue(RegisterID* cond, Label& target)
 {
-
-    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 (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;
+        }
     }
 
     OpJtrue::emit(this, cond, target.bind(this));
@@ -1460,45 +1463,47 @@
 
 void BytecodeGenerator::emitJumpIfFalse(RegisterID* cond, Label& target)
 {
-    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 (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;
+        }
     }
 
     OpJfalse::emit(this, cond, target.bind(this));
@@ -1707,6 +1712,9 @@
 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: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (248219 => 248220)


--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2019-08-04 03:22:38 UTC (rev 248219)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2019-08-04 03:22:41 UTC (rev 248220)
@@ -1047,6 +1047,8 @@
 
         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