Merge the instruction pair (less, jfalse)

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Cameron Zwarich (cpst)]

The pair (less, jfalse) is 1.61% of SunSpider instruction pairs. Merging them into a new opcode jnless is generally a win, but is a large regression on the regexp-dna and nbody tests.

[Cameron Zwarich (cpst)]

Created attachment 21999 [details]
Patch

Here's a patch that implements jnless. It has no dump code for the new opcode and the opcode documentation is wrong.

[Cameron Zwarich (cpst)]

Created attachment 22000 [details]
SunSpider results

Here are the SunSpider results. I'll do a before-and-after with Shark on the regexp-dna test.

[Cameron Zwarich (cpst)]

The regression seems to be entirely caused by the CodeGenerator logic, even though the regexp-dna test doesn't hit it. I can substitute an empty opcode at the end of the opcodes in Machine.cpp and I still get the regression. If I remove the extra logic in CodeGenerator::emitJumpIfFalse, it's fine.

[Cameron Zwarich (cpst)]

Created attachment 22002 [details]
Proposed patch

It turns out that the regression was entirely due to the changes in CodeGenerator. Adding ALWAYS_INLINE to CodeGenerator::rewindBinaryOp() fixed the regression. It is now a 2.4% progression on SunSpider.

[Cameron Zwarich (cpst)]

Created attachment 22003 [details]
New SunSpider results

Here are new SunSpider results, for a reference.

[Oliver Hunt]

Comment on attachment 22002 [details]
Proposed patch

r=me

[Cameron Zwarich (cpst)]

Landed in r34883.