Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (189135 => 189136)
--- trunk/Source/_javascript_Core/ChangeLog 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-08-29 01:57:05 UTC (rev 189136)
@@ -1,3 +1,32 @@
+2015-08-28 Benjamin Poulain <[email protected]>
+
+ [JSC][x86] Improve the compare functions when comparing with zero
+ https://bugs.webkit.org/show_bug.cgi?id=148536
+
+ Reviewed by Geoffrey Garen.
+
+ This patch has two parts:
+ 1) The macro assembler gets an additional cmp->test optimization
+ for LessThan and GreaterThanOrEqual.
+ Instead of comparing the value with an immediate, test the value
+ with itself and use the flag.
+ 2) Extend the DFG JIT optimization of compare.
+ In particular, use the same optimization in compileInt32Compare()
+ as we have in compilePeepHoleBooleanBranch().
+ The generator compileInt32Compare() is unfortunately very
+ common due to MoveHints placed between the Compare node and the Branch
+ node.
+
+ * assembler/MacroAssembler.h:
+ (JSC::MacroAssembler::compare32):
+ * assembler/MacroAssemblerX86Common.h:
+ (JSC::MacroAssemblerX86Common::branch32):
+ (JSC::MacroAssemblerX86Common::compare32):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compilePeepHoleBooleanBranch):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compileInt32Compare):
+
2015-08-28 Mark Lam <[email protected]>
Add MacroAssemblerPrinter support for printing memory.
Modified: trunk/Source/_javascript_Core/assembler/MacroAssembler.h (189135 => 189136)
--- trunk/Source/_javascript_Core/assembler/MacroAssembler.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/assembler/MacroAssembler.h 2015-08-29 01:57:05 UTC (rev 189136)
@@ -112,6 +112,7 @@
using MacroAssemblerBase::pop;
using MacroAssemblerBase::jump;
using MacroAssemblerBase::branch32;
+ using MacroAssemblerBase::compare32;
using MacroAssemblerBase::move;
using MacroAssemblerBase::add32;
using MacroAssemblerBase::and32;
@@ -329,6 +330,11 @@
return branch32(commute(cond), right, left);
}
+ void compare32(RelationalCondition cond, Imm32 left, RegisterID right, RegisterID dest)
+ {
+ compare32(commute(cond), right, left, dest);
+ }
+
void branchTestPtr(ResultCondition cond, RegisterID reg, Label target)
{
branchTestPtr(cond, reg).linkTo(target, this);
@@ -1509,6 +1515,24 @@
return branch32(cond, left, right.asTrustedImm32());
}
+ void compare32(RelationalCondition cond, RegisterID left, Imm32 right, RegisterID dest)
+ {
+ if (shouldBlind(right)) {
+ if (haveScratchRegisterForBlinding()) {
+ loadXorBlindedConstant(xorBlindConstant(right), scratchRegisterForBlinding());
+ compare32(cond, left, scratchRegisterForBlinding(), dest);
+ }
+ // If we don't have a scratch register available for use, we'll just
+ // place a random number of nops.
+ uint32_t nopCount = random() & 3;
+ while (nopCount--)
+ nop();
+ compare32(cond, left, right.asTrustedImm32(), dest);
+ }
+
+ compare32(cond, left, right.asTrustedImm32(), dest);
+ }
+
Jump branchAdd32(ResultCondition cond, RegisterID src, Imm32 imm, RegisterID dest)
{
if (src == dest)
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (189135 => 189136)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2015-08-29 01:57:05 UTC (rev 189136)
@@ -1305,7 +1305,7 @@
private:
// Should we be using TEQ for equal/not-equal?
- void compare32(RegisterID left, TrustedImm32 right)
+ void compare32AndSetFlags(RegisterID left, TrustedImm32 right)
{
int32_t imm = right.m_value;
ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm);
@@ -1363,7 +1363,7 @@
Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
{
- compare32(left, right);
+ compare32AndSetFlags(left, right);
return Jump(makeBranch(cond));
}
@@ -1421,7 +1421,7 @@
Jump branch8(RelationalCondition cond, RegisterID left, TrustedImm32 right)
{
- compare32(left, right);
+ compare32AndSetFlags(left, right);
return Jump(makeBranch(cond));
}
@@ -1721,7 +1721,7 @@
void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
{
- compare32(left, right);
+ compare32AndSetFlags(left, right);
m_assembler.it(armV7Condition(cond), false);
m_assembler.mov(dest, ARMThumbImmediate::makeUInt16(1));
m_assembler.mov(dest, ARMThumbImmediate::makeUInt16(0));
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h (189135 => 189136)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h 2015-08-29 01:57:05 UTC (rev 189136)
@@ -1124,10 +1124,28 @@
Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
{
- if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
+ if (!right.m_value && (cond == Equal || cond == NotEqual || cond == LessThan || cond == GreaterThanOrEqual)) {
+ ResultCondition resultCondition;
+ switch (cond) {
+ case Equal:
+ resultCondition = Zero;
+ break;
+ case NotEqual:
+ resultCondition = NonZero;
+ break;
+ case LessThan:
+ resultCondition = Signed;
+ break;
+ case GreaterThanOrEqual:
+ resultCondition = PositiveOrZero;
+ break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ }
m_assembler.testl_rr(left, left);
- else
- m_assembler.cmpl_ir(right.m_value, left);
+ return Jump(m_assembler.jCC(x86Condition(resultCondition)));
+ }
+ m_assembler.cmpl_ir(right.m_value, left);
return Jump(m_assembler.jCC(x86Condition(cond)));
}
@@ -1437,10 +1455,29 @@
void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
{
- if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
+ if (!right.m_value && (cond == Equal || cond == NotEqual || cond == LessThan || cond == GreaterThanOrEqual)) {
+ ResultCondition resultCondition;
+ switch (cond) {
+ case Equal:
+ resultCondition = Zero;
+ break;
+ case NotEqual:
+ resultCondition = NonZero;
+ break;
+ case LessThan:
+ resultCondition = Signed;
+ break;
+ case GreaterThanOrEqual:
+ resultCondition = PositiveOrZero;
+ break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ }
m_assembler.testl_rr(left, left);
- else
- m_assembler.cmpl_ir(right.m_value, left);
+ set32(x86Condition(resultCondition), dest);
+ return;
+ }
+ m_assembler.cmpl_ir(right.m_value, left);
set32(x86Condition(cond), dest);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (189135 => 189136)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-08-29 01:57:05 UTC (rev 189136)
@@ -1278,14 +1278,14 @@
notTaken = tmp;
}
- if (node->child1()->isBooleanConstant()) {
- bool imm = node->child1()->asBoolean();
+ if (node->child1()->isInt32Constant()) {
+ int32_t imm = node->child1()->asInt32();
SpeculateBooleanOperand op2(this, node->child2());
- branch32(condition, JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), op2.gpr(), taken);
- } else if (node->child2()->isBooleanConstant()) {
+ branch32(condition, JITCompiler::Imm32(imm), op2.gpr(), taken);
+ } else if (node->child2()->isInt32Constant()) {
SpeculateBooleanOperand op1(this, node->child1());
- bool imm = node->child2()->asBoolean();
- branch32(condition, op1.gpr(), JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), taken);
+ int32_t imm = node->child2()->asInt32();
+ branch32(condition, op1.gpr(), JITCompiler::Imm32(imm), taken);
} else {
SpeculateBooleanOperand op1(this, node->child1());
SpeculateBooleanOperand op2(this, node->child2());
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (189135 => 189136)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2015-08-29 01:57:05 UTC (rev 189136)
@@ -1470,15 +1470,34 @@
void SpeculativeJIT::compileInt32Compare(Node* node, MacroAssembler::RelationalCondition condition)
{
- SpeculateInt32Operand op1(this, node->child1());
- SpeculateInt32Operand op2(this, node->child2());
- GPRTemporary result(this, Reuse, op1, op2);
-
- m_jit.compare32(condition, op1.gpr(), op2.gpr(), result.gpr());
-
- // If we add a DataFormatBool, we should use it here.
- m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
- jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
+ if (node->child1()->isInt32Constant()) {
+ SpeculateInt32Operand op2(this, node->child2());
+ GPRTemporary result(this, Reuse, op2);
+ int32_t imm = node->child1()->asInt32();
+ m_jit.compare32(condition, JITCompiler::Imm32(imm), op2.gpr(), result.gpr());
+
+ // If we add a DataFormatBool, we should use it here.
+ m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+ jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
+ } else if (node->child2()->isInt32Constant()) {
+ SpeculateInt32Operand op1(this, node->child1());
+ GPRTemporary result(this, Reuse, op1);
+ int32_t imm = node->child2()->asInt32();
+ m_jit.compare32(condition, op1.gpr(), JITCompiler::Imm32(imm), result.gpr());
+
+ // If we add a DataFormatBool, we should use it here.
+ m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+ jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
+ } else {
+ SpeculateInt32Operand op1(this, node->child1());
+ SpeculateInt32Operand op2(this, node->child2());
+ GPRTemporary result(this, Reuse, op1, op2);
+ m_jit.compare32(condition, op1.gpr(), op2.gpr(), result.gpr());
+
+ // If we add a DataFormatBool, we should use it here.
+ m_jit.or32(TrustedImm32(ValueFalse), result.gpr());
+ jsValueResult(result.gpr(), m_currentNode, DataFormatJSBoolean);
+ }
}
void SpeculativeJIT::compileInt52Compare(Node* node, MacroAssembler::RelationalCondition condition)
