Skip to content
Permalink
Browse files
Reland "Add Above/Below comparisons for UInt32 patterns"
https://bugs.webkit.org/show_bug.cgi?id=177281

Reviewed by Saam Barati.

JSTests:

* stress/uint32-comparison-jump.js: Added.
(shouldBe):
(above):
(aboveOrEqual):
(below):
(belowOrEqual):
(notAbove):
(notAboveOrEqual):
(notBelow):
(notBelowOrEqual):
* stress/uint32-comparison.js: Added.
(shouldBe):
(above):
(aboveOrEqual):
(below):
(belowOrEqual):
(aboveTest):
(aboveOrEqualTest):
(belowTest):
(belowOrEqualTest):

Source/JavaScriptCore:

We reland this patch without DFGStrengthReduction change to see what causes
regression in the iOS bot.

Sometimes, we would like to have UInt32 operations in JS. While VM does
not support UInt32 nicely, VM supports efficient Int32 operations. As long
as signedness does not matter, we can just perform Int32 operations instead
and recognize its bit pattern as UInt32.

But of course, some operations respect signedness. The most frequently
used one is comparison. Octane/zlib performs UInt32 comparison by performing
`val >>> 0`. It emits op_urshift and op_unsigned. op_urshift produces
UInt32 in Int32 form. And op_unsigned will generate Double value if
the generated Int32 is < 0 (which should be UInt32).

There is a chance for optimization. The given code pattern is the following.

    op_unsigned(op_urshift(@1)) lessThan:< op_unsigned(op_urshift(@2))

This can be converted to the following.

    op_urshift(@1) below:< op_urshift(@2)

The above conversion is nice since

1. We can avoid op_unsigned. This could be unsignedness check in DFG. Since
this check depends on the value of Int32, dropping this check is not as easy as
removing Int32 edge filters.

2. We can perform unsigned comparison in Int32 form. We do not need to convert
them to DoubleRep.

Since the above comparison exists in Octane/zlib's *super* hot path, dropping
op_unsigned offers huge win.

At first, my patch attempts to convert the above thing in DFG pipeline.
However it poses several problems.

1. MovHint is not well removed. It makes UInt32ToNumber (which is for op_unsigned) live.
2. UInt32ToNumber could cause an OSR exit. So if we have the following nodes,

    2: UInt32ToNumber(@0)
    3: MovHint(@2, xxx)
    4: UInt32ToNumber(@1)
    5: MovHint(@1, xxx)

we could drop @5's MovHint. But @3 is difficult since @4 can exit.

So, instead, we start introducing a simple optimization in the bytecode compiler.
It performs pattern matching for op_urshift and comparison to drop op_unsigned.
We adds op_below and op_above families to bytecodes. They only accept Int32 and
perform unsigned comparison.

This offers 4% performance improvement in Octane/zlib.

                            baseline                  patched

zlib           x2     431.07483+-16.28434       414.33407+-9.38375         might be 1.0404x faster

* bytecode/BytecodeDumper.cpp:
(JSC::BytecodeDumper<Block>::printCompareJump):
(JSC::BytecodeDumper<Block>::dumpBytecode):
* bytecode/BytecodeDumper.h:
* bytecode/BytecodeList.json:
* bytecode/BytecodeUseDef.h:
(JSC::computeUsesForBytecodeOffset):
(JSC::computeDefsForBytecodeOffset):
* bytecode/Opcode.h:
(JSC::isBranch):
* bytecode/PreciseJumpTargetsInlines.h:
(JSC::extractStoredJumpTargetsForBytecodeOffset):
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitJumpIfTrue):
(JSC::BytecodeGenerator::emitJumpIfFalse):
* bytecompiler/NodesCodegen.cpp:
(JSC::BinaryOpNode::emitBytecode):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGIntegerRangeOptimizationPhase.cpp:
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCompareUnsigned):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGValidate.cpp:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareBelow):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareBelowEq):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
* jit/JIT.h:
* jit/JITArithmetic.cpp:
(JSC::JIT::emit_op_below):
(JSC::JIT::emit_op_beloweq):
(JSC::JIT::emit_op_jbelow):
(JSC::JIT::emit_op_jbeloweq):
(JSC::JIT::emit_compareUnsignedAndJump):
(JSC::JIT::emit_compareUnsigned):
* jit/JITArithmetic32_64.cpp:
(JSC::JIT::emit_compareUnsignedAndJump):
(JSC::JIT::emit_compareUnsigned):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* parser/Nodes.h:
(JSC::ExpressionNode::isBinaryOpNode const):

Canonical link: https://commits.webkit.org/194525@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223318 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Constellation committed Oct 14, 2017
1 parent cfcb3a0 commit ea8fdc2259c609881dae1ad870264d339ef21175
Showing 37 changed files with 875 additions and 142 deletions.
@@ -1,3 +1,31 @@
2017-10-14 Yusuke Suzuki <utatane.tea@gmail.com>

Reland "Add Above/Below comparisons for UInt32 patterns"
https://bugs.webkit.org/show_bug.cgi?id=177281

Reviewed by Saam Barati.

* stress/uint32-comparison-jump.js: Added.
(shouldBe):
(above):
(aboveOrEqual):
(below):
(belowOrEqual):
(notAbove):
(notAboveOrEqual):
(notBelow):
(notBelowOrEqual):
* stress/uint32-comparison.js: Added.
(shouldBe):
(above):
(aboveOrEqual):
(below):
(belowOrEqual):
(aboveTest):
(aboveOrEqualTest):
(belowTest):
(belowOrEqualTest):

2017-10-12 Yusuke Suzuki <utatane.tea@gmail.com>

WebAssembly: Wasm functions should have either JSFunctionType or TypeOfShouldCallGetCallData
@@ -0,0 +1,141 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function above(a, trap) {
let result = 0;
for (let i = 0; (a >>> 0) > (i >>> 0); ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(above);

function aboveOrEqual(a, trap) {
let result = 0;
for (let i = 0; (a >>> 0) >= (i >>> 0); ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(aboveOrEqual);

function below(a, trap) {
let result = 0;
for (let i = 0; (i >>> 0) < (a >>> 0); ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(below);

function belowOrEqual(a, trap) {
let result = 0;
for (let i = 0; (i >>> 0) <= (a >>> 0); ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(belowOrEqual);

function notAbove(a, trap) {
let result = 0;
for (let i = 0; (a >>> 0) > (i >>> 0) && a; ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(notAbove);

function notAboveOrEqual(a, trap) {
let result = 0;
for (let i = 0; (a >>> 0) >= (i >>> 0) && a; ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(notAboveOrEqual);

function notBelow(a, trap) {
let result = 0;
for (let i = 0; (i >>> 0) < (a >>> 0) && a; ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(notBelow);

function notBelowOrEqual(a, trap) {
let result = 0;
for (let i = 0; (i >>> 0) <= (a >>> 0) && a; ++i) {
result += i;
if (i === trap)
break;
}
return result;
}
noInline(notBelowOrEqual);

for (var i = 0; i < 1e2; ++i) {
shouldBe(above(0, -1), 0);
shouldBe(above(20000, -1), 199990000);
shouldBe(above(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(aboveOrEqual(0, -1), 0);
shouldBe(aboveOrEqual(20000, -1), 200010000);
shouldBe(aboveOrEqual(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(below(0, -1), 0);
shouldBe(below(20000, -1), 199990000);
shouldBe(below(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(belowOrEqual(0, -1), 0);
shouldBe(belowOrEqual(20000, -1), 200010000);
shouldBe(belowOrEqual(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(notAbove(0, -1), 0);
shouldBe(notAbove(20000, -1), 199990000);
shouldBe(notAbove(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(notAboveOrEqual(0, -1), 0);
shouldBe(notAboveOrEqual(20000, -1), 200010000);
shouldBe(notAboveOrEqual(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(notBelow(0, -1), 0);
shouldBe(notBelow(20000, -1), 199990000);
shouldBe(notBelow(-1, 10000), 50005000);
}

for (var i = 0; i < 1e2; ++i) {
shouldBe(notBelowOrEqual(0, -1), 0);
shouldBe(notBelowOrEqual(20000, -1), 200010000);
shouldBe(notBelowOrEqual(-1, 10000), 50005000);
}

@@ -0,0 +1,88 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

function above(a, b) {
return (a >>> 0) > (b >>> 0);
}
noInline(above);

function aboveOrEqual(a, b) {
return (a >>> 0) >= (b >>> 0);
}
noInline(aboveOrEqual);

function below(a, b) {
return (a >>> 0) < (b >>> 0);
}
noInline(below);

function belowOrEqual(a, b) {
return (a >>> 0) <= (b >>> 0);
}
noInline(belowOrEqual);

(function aboveTest() {
for (let i = 0; i < 1e5; ++i) {
shouldBe(above(0, 20), false);
shouldBe(above(0, 0), false);
shouldBe(above(0, -0), false);
shouldBe(above(-1, 0), true);
shouldBe(above(-1, -1), false);
shouldBe(above(-1, 1), true);
shouldBe(above(1, -1), false);
shouldBe(above(1, 0xffffffff), false);
shouldBe(above(0xffffffff, 0xffffffff), false);
shouldBe(above(-1, 0xffffffff), false);
shouldBe(above(-1, 0xfffffffff), false);
}
}());

(function aboveOrEqualTest() {
for (let i = 0; i < 1e5; ++i) {
shouldBe(aboveOrEqual(0, 20), false);
shouldBe(aboveOrEqual(0, 0), true);
shouldBe(aboveOrEqual(0, -0), true);
shouldBe(aboveOrEqual(-1, 0), true);
shouldBe(aboveOrEqual(-1, -1), true);
shouldBe(aboveOrEqual(-1, 1), true);
shouldBe(aboveOrEqual(1, -1), false);
shouldBe(aboveOrEqual(1, 0xffffffff), false);
shouldBe(aboveOrEqual(0xffffffff, 0xffffffff), true);
shouldBe(aboveOrEqual(-1, 0xffffffff), true);
shouldBe(aboveOrEqual(-1, 0xfffffffff), true);
}
}());

(function belowTest() {
for (let i = 0; i < 1e5; ++i) {
shouldBe(below(0, 20), true);
shouldBe(below(0, 0), false);
shouldBe(below(0, -0), false);
shouldBe(below(-1, 0), false);
shouldBe(below(-1, -1), false);
shouldBe(below(-1, 1), false);
shouldBe(below(1, -1), true);
shouldBe(below(1, 0xffffffff), true);
shouldBe(below(0xffffffff, 0xffffffff), false);
shouldBe(below(-1, 0xffffffff), false);
shouldBe(below(-1, 0xfffffffff), false);
}
}());

(function belowOrEqualTest() {
for (let i = 0; i < 1e5; ++i) {
shouldBe(belowOrEqual(0, 20), true);
shouldBe(belowOrEqual(0, 0), true);
shouldBe(belowOrEqual(0, -0), true);
shouldBe(belowOrEqual(-1, 0), false);
shouldBe(belowOrEqual(-1, -1), true);
shouldBe(belowOrEqual(-1, 1), false);
shouldBe(belowOrEqual(1, -1), true);
shouldBe(belowOrEqual(1, 0xffffffff), true);
shouldBe(belowOrEqual(0xffffffff, 0xffffffff), true);
shouldBe(belowOrEqual(-1, 0xffffffff), true);
shouldBe(belowOrEqual(-1, 0xfffffffff), true);
}
}());
@@ -1,3 +1,135 @@
2017-10-14 Yusuke Suzuki <utatane.tea@gmail.com>

Reland "Add Above/Below comparisons for UInt32 patterns"
https://bugs.webkit.org/show_bug.cgi?id=177281

Reviewed by Saam Barati.

We reland this patch without DFGStrengthReduction change to see what causes
regression in the iOS bot.

Sometimes, we would like to have UInt32 operations in JS. While VM does
not support UInt32 nicely, VM supports efficient Int32 operations. As long
as signedness does not matter, we can just perform Int32 operations instead
and recognize its bit pattern as UInt32.

But of course, some operations respect signedness. The most frequently
used one is comparison. Octane/zlib performs UInt32 comparison by performing
`val >>> 0`. It emits op_urshift and op_unsigned. op_urshift produces
UInt32 in Int32 form. And op_unsigned will generate Double value if
the generated Int32 is < 0 (which should be UInt32).

There is a chance for optimization. The given code pattern is the following.

op_unsigned(op_urshift(@1)) lessThan:< op_unsigned(op_urshift(@2))

This can be converted to the following.

op_urshift(@1) below:< op_urshift(@2)

The above conversion is nice since

1. We can avoid op_unsigned. This could be unsignedness check in DFG. Since
this check depends on the value of Int32, dropping this check is not as easy as
removing Int32 edge filters.

2. We can perform unsigned comparison in Int32 form. We do not need to convert
them to DoubleRep.

Since the above comparison exists in Octane/zlib's *super* hot path, dropping
op_unsigned offers huge win.

At first, my patch attempts to convert the above thing in DFG pipeline.
However it poses several problems.

1. MovHint is not well removed. It makes UInt32ToNumber (which is for op_unsigned) live.
2. UInt32ToNumber could cause an OSR exit. So if we have the following nodes,

2: UInt32ToNumber(@0)
3: MovHint(@2, xxx)
4: UInt32ToNumber(@1)
5: MovHint(@1, xxx)

we could drop @5's MovHint. But @3 is difficult since @4 can exit.

So, instead, we start introducing a simple optimization in the bytecode compiler.
It performs pattern matching for op_urshift and comparison to drop op_unsigned.
We adds op_below and op_above families to bytecodes. They only accept Int32 and
perform unsigned comparison.

This offers 4% performance improvement in Octane/zlib.

baseline patched

zlib x2 431.07483+-16.28434 414.33407+-9.38375 might be 1.0404x faster

* bytecode/BytecodeDumper.cpp:
(JSC::BytecodeDumper<Block>::printCompareJump):
(JSC::BytecodeDumper<Block>::dumpBytecode):
* bytecode/BytecodeDumper.h:
* bytecode/BytecodeList.json:
* bytecode/BytecodeUseDef.h:
(JSC::computeUsesForBytecodeOffset):
(JSC::computeDefsForBytecodeOffset):
* bytecode/Opcode.h:
(JSC::isBranch):
* bytecode/PreciseJumpTargetsInlines.h:
(JSC::extractStoredJumpTargetsForBytecodeOffset):
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitJumpIfTrue):
(JSC::BytecodeGenerator::emitJumpIfFalse):
* bytecompiler/NodesCodegen.cpp:
(JSC::BinaryOpNode::emitBytecode):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGIntegerRangeOptimizationPhase.cpp:
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCompareUnsigned):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGValidate.cpp:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareBelow):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareBelowEq):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
* jit/JIT.h:
* jit/JITArithmetic.cpp:
(JSC::JIT::emit_op_below):
(JSC::JIT::emit_op_beloweq):
(JSC::JIT::emit_op_jbelow):
(JSC::JIT::emit_op_jbeloweq):
(JSC::JIT::emit_compareUnsignedAndJump):
(JSC::JIT::emit_compareUnsigned):
* jit/JITArithmetic32_64.cpp:
(JSC::JIT::emit_compareUnsignedAndJump):
(JSC::JIT::emit_compareUnsigned):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* parser/Nodes.h:
(JSC::ExpressionNode::isBinaryOpNode const):

2017-10-12 Yusuke Suzuki <utatane.tea@gmail.com>

WebAssembly: Wasm functions should have either JSFunctionType or TypeOfShouldCallGetCallData

0 comments on commit ea8fdc2

Please sign in to comment.