Bug 221927 – BytecodeGenerator::fuseCompareAndJump() fails for some language constructs

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alexey Shvayka]

BytecodeGenerator::fuseCompareAndJump() fails for some language constructs

[Alexey Shvayka]

Created attachment 420376 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 420376 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420376&action=review

r=me, awesome!

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:5176
> +                emitJumpIfTrue(emitEqualityOp<OpBeloweq>(newTemporary(), context.completionTypeRegister(), emitLoad(nullptr, CompletionType::Throw)), isThrowOrNormalLabel.get());

I think emitEqualityOp is not correct for OpBeloweq (emitBinaryOp?).
Could you also insert static_assert for them in emitEqualityOp? (static_assert for EqOp is StrictEq etc.).

[Radar WebKit Bug Importer]

<rdar://problem/74614681>

[Alexey Shvayka]

Created attachment 421814 [details]
Patch for landing

[Alexey Shvayka]

(In reply to Yusuke Suzuki from comment #2)
> r=me, awesome!

I always appreciate your thorough reviews. Thanks Yusuke!

> I think emitEqualityOp is not correct for OpBeloweq (emitBinaryOp?).
> Could you also insert static_assert for them in emitEqualityOp?
> (static_assert for EqOp is StrictEq etc.).

Great catch!

I've added a static_assert only for op_eq / op_stricteq because op_neq / op_nstricteq are handled in BinaryOpNode::emitBytecode() and never get passed to emitEqualityOp().

[EWS]

Committed r273649: <https://commits.webkit.org/r273649>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421814 [details].

[Yusuke Suzuki]

This improved JetStream2/Air by 5%. Awesome!