Log Message
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
Modified Paths
- branches/safari-607.2.1.2-branch/JSTests/ChangeLog
- branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog
- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp
- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h
Added Paths
Diff
Modified: branches/safari-607.2.1.2-branch/JSTests/ChangeLog (245332 => 245333)
--- branches/safari-607.2.1.2-branch/JSTests/ChangeLog 2019-05-15 18:20:49 UTC (rev 245332)
+++ branches/safari-607.2.1.2-branch/JSTests/ChangeLog 2019-05-15 18:20:52 UTC (rev 245333)
@@ -1,5 +1,54 @@
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
Added: branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js (0 => 245333)
--- branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js (rev 0)
+++ branches/safari-607.2.1.2-branch/JSTests/stress/do-not-perform-bytecode-peephole-optimization-in-jump-target.js 2019-05-15 18:20:52 UTC (rev 245333)
@@ -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: branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog (245332 => 245333)
--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog 2019-05-15 18:20:49 UTC (rev 245332)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/ChangeLog 2019-05-15 18:20:52 UTC (rev 245333)
@@ -1,5 +1,72 @@
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 (245332 => 245333)
--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2019-05-15 18:20:49 UTC (rev 245332)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2019-05-15 18:20:52 UTC (rev 245333)
@@ -1387,6 +1387,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();
@@ -1403,6 +1404,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();
@@ -1415,43 +1417,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));
@@ -1459,45 +1462,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));
@@ -1706,6 +1711,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: branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (245332 => 245333)
--- branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-05-15 18:20:49 UTC (rev 245332)
+++ branches/safari-607.2.1.2-branch/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-05-15 18:20:52 UTC (rev 245333)
@@ -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
