<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[189136] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/189136">189136</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2015-08-28 18:57:05 -0700 (Fri, 28 Aug 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC][x86] Improve the compare functions when comparing with zero
https://bugs.webkit.org/show_bug.cgi?id=148536
Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-08-28
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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreassemblerMacroAssemblerh">trunk/Source/JavaScriptCore/assembler/MacroAssembler.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreassemblerMacroAssemblerARMv7h">trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreassemblerMacroAssemblerX86Commonh">trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2015-08-28 Benjamin Poulain <bpoulain@apple.com>
+
+ [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):
+
</ins><span class="cx"> 2015-08-28 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> Add MacroAssemblerPrinter support for printing memory.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreassemblerMacroAssemblerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/assembler/MacroAssembler.h (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/assembler/MacroAssembler.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/assembler/MacroAssembler.h 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -112,6 +112,7 @@
</span><span class="cx"> using MacroAssemblerBase::pop;
</span><span class="cx"> using MacroAssemblerBase::jump;
</span><span class="cx"> using MacroAssemblerBase::branch32;
</span><ins>+ using MacroAssemblerBase::compare32;
</ins><span class="cx"> using MacroAssemblerBase::move;
</span><span class="cx"> using MacroAssemblerBase::add32;
</span><span class="cx"> using MacroAssemblerBase::and32;
</span><span class="lines">@@ -329,6 +330,11 @@
</span><span class="cx"> return branch32(commute(cond), right, left);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ void compare32(RelationalCondition cond, Imm32 left, RegisterID right, RegisterID dest)
+ {
+ compare32(commute(cond), right, left, dest);
+ }
+
</ins><span class="cx"> void branchTestPtr(ResultCondition cond, RegisterID reg, Label target)
</span><span class="cx"> {
</span><span class="cx"> branchTestPtr(cond, reg).linkTo(target, this);
</span><span class="lines">@@ -1509,6 +1515,24 @@
</span><span class="cx"> return branch32(cond, left, right.asTrustedImm32());
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ 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);
+ }
+
</ins><span class="cx"> Jump branchAdd32(ResultCondition cond, RegisterID src, Imm32 imm, RegisterID dest)
</span><span class="cx"> {
</span><span class="cx"> if (src == dest)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreassemblerMacroAssemblerARMv7h"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -1305,7 +1305,7 @@
</span><span class="cx"> private:
</span><span class="cx">
</span><span class="cx"> // Should we be using TEQ for equal/not-equal?
</span><del>- void compare32(RegisterID left, TrustedImm32 right)
</del><ins>+ void compare32AndSetFlags(RegisterID left, TrustedImm32 right)
</ins><span class="cx"> {
</span><span class="cx"> int32_t imm = right.m_value;
</span><span class="cx"> ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm);
</span><span class="lines">@@ -1363,7 +1363,7 @@
</span><span class="cx">
</span><span class="cx"> Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
</span><span class="cx"> {
</span><del>- compare32(left, right);
</del><ins>+ compare32AndSetFlags(left, right);
</ins><span class="cx"> return Jump(makeBranch(cond));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1421,7 +1421,7 @@
</span><span class="cx">
</span><span class="cx"> Jump branch8(RelationalCondition cond, RegisterID left, TrustedImm32 right)
</span><span class="cx"> {
</span><del>- compare32(left, right);
</del><ins>+ compare32AndSetFlags(left, right);
</ins><span class="cx"> return Jump(makeBranch(cond));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1721,7 +1721,7 @@
</span><span class="cx">
</span><span class="cx"> void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
</span><span class="cx"> {
</span><del>- compare32(left, right);
</del><ins>+ compare32AndSetFlags(left, right);
</ins><span class="cx"> m_assembler.it(armV7Condition(cond), false);
</span><span class="cx"> m_assembler.mov(dest, ARMThumbImmediate::makeUInt16(1));
</span><span class="cx"> m_assembler.mov(dest, ARMThumbImmediate::makeUInt16(0));
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreassemblerMacroAssemblerX86Commonh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -1124,10 +1124,28 @@
</span><span class="cx">
</span><span class="cx"> Jump branch32(RelationalCondition cond, RegisterID left, TrustedImm32 right)
</span><span class="cx"> {
</span><del>- if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
</del><ins>+ 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();
+ }
</ins><span class="cx"> m_assembler.testl_rr(left, left);
</span><del>- else
- m_assembler.cmpl_ir(right.m_value, left);
</del><ins>+ return Jump(m_assembler.jCC(x86Condition(resultCondition)));
+ }
+ m_assembler.cmpl_ir(right.m_value, left);
</ins><span class="cx"> return Jump(m_assembler.jCC(x86Condition(cond)));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1437,10 +1455,29 @@
</span><span class="cx">
</span><span class="cx"> void compare32(RelationalCondition cond, RegisterID left, TrustedImm32 right, RegisterID dest)
</span><span class="cx"> {
</span><del>- if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
</del><ins>+ 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();
+ }
</ins><span class="cx"> m_assembler.testl_rr(left, left);
</span><del>- else
- m_assembler.cmpl_ir(right.m_value, left);
</del><ins>+ set32(x86Condition(resultCondition), dest);
+ return;
+ }
+ m_assembler.cmpl_ir(right.m_value, left);
</ins><span class="cx"> set32(x86Condition(cond), dest);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -1278,14 +1278,14 @@
</span><span class="cx"> notTaken = tmp;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (node->child1()->isBooleanConstant()) {
- bool imm = node->child1()->asBoolean();
</del><ins>+ if (node->child1()->isInt32Constant()) {
+ int32_t imm = node->child1()->asInt32();
</ins><span class="cx"> SpeculateBooleanOperand op2(this, node->child2());
</span><del>- branch32(condition, JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), op2.gpr(), taken);
- } else if (node->child2()->isBooleanConstant()) {
</del><ins>+ branch32(condition, JITCompiler::Imm32(imm), op2.gpr(), taken);
+ } else if (node->child2()->isInt32Constant()) {
</ins><span class="cx"> SpeculateBooleanOperand op1(this, node->child1());
</span><del>- bool imm = node->child2()->asBoolean();
- branch32(condition, op1.gpr(), JITCompiler::Imm32(static_cast<int32_t>(JSValue::encode(jsBoolean(imm)))), taken);
</del><ins>+ int32_t imm = node->child2()->asInt32();
+ branch32(condition, op1.gpr(), JITCompiler::Imm32(imm), taken);
</ins><span class="cx"> } else {
</span><span class="cx"> SpeculateBooleanOperand op1(this, node->child1());
</span><span class="cx"> SpeculateBooleanOperand op2(this, node->child2());
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJIT64cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp (189135 => 189136)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp 2015-08-29 00:14:54 UTC (rev 189135)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp 2015-08-29 01:57:05 UTC (rev 189136)
</span><span class="lines">@@ -1470,15 +1470,34 @@
</span><span class="cx">
</span><span class="cx"> void SpeculativeJIT::compileInt32Compare(Node* node, MacroAssembler::RelationalCondition condition)
</span><span class="cx"> {
</span><del>- 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);
</del><ins>+ 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);
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void SpeculativeJIT::compileInt52Compare(Node* node, MacroAssembler::RelationalCondition condition)
</span></span></pre>
</div>
</div>
</body>
</html>