<!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>[193793] 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/193793">193793</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-12-08 16:12:48 -0800 (Tue, 08 Dec 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>DFG and FTL should be resilient against cases where both snippet operands are constant.
https://bugs.webkit.org/show_bug.cgi?id=152017
Reviewed by Michael Saboff.
The DFG front end may not always constant fold cases where both operands are
constant. As a result, the DFG and FTL back ends needs to be resilient against
this when using snippet generators since the generators do not support the case
where both operands are constant. The strategy for handling this 2 const operands
case is to treat at least one of them as a variable if both are constant.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileValueAdd):
- Also remove the case for folding 2 constant operands. It is the front end's
job to do so, not the back end here.
(JSC::DFG::SpeculativeJIT::compileArithSub):
(JSC::DFG::SpeculativeJIT::compileArithMul):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileValueAdd):
(JSC::FTL::DFG::LowerDFGToLLVM::compileArithMul):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (193792 => 193793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-12-09 00:08:09 UTC (rev 193792)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-12-09 00:12:48 UTC (rev 193793)
</span><span class="lines">@@ -1,5 +1,29 @@
</span><span class="cx"> 2015-12-08 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><ins>+ DFG and FTL should be resilient against cases where both snippet operands are constant.
+ https://bugs.webkit.org/show_bug.cgi?id=152017
+
+ Reviewed by Michael Saboff.
+
+ The DFG front end may not always constant fold cases where both operands are
+ constant. As a result, the DFG and FTL back ends needs to be resilient against
+ this when using snippet generators since the generators do not support the case
+ where both operands are constant. The strategy for handling this 2 const operands
+ case is to treat at least one of them as a variable if both are constant.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileValueAdd):
+ - Also remove the case for folding 2 constant operands. It is the front end's
+ job to do so, not the back end here.
+
+ (JSC::DFG::SpeculativeJIT::compileArithSub):
+ (JSC::DFG::SpeculativeJIT::compileArithMul):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileValueAdd):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileArithMul):
+
+2015-12-08 Mark Lam <mark.lam@apple.com>
+
</ins><span class="cx"> Snippefy shift operators for the baseline JIT.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=151875
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (193792 => 193793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2015-12-09 00:08:09 UTC (rev 193792)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp 2015-12-09 00:12:48 UTC (rev 193793)
</span><span class="lines">@@ -2788,9 +2788,12 @@
</span><span class="cx">
</span><span class="cx"> void SpeculativeJIT::compileValueAdd(Node* node)
</span><span class="cx"> {
</span><del>- if (isKnownNotNumber(node->child1().node()) || isKnownNotNumber(node->child2().node())) {
- JSValueOperand left(this, node->child1());
- JSValueOperand right(this, node->child2());
</del><ins>+ Edge& leftChild = node->child1();
+ Edge& rightChild = node->child2();
+
+ if (isKnownNotNumber(leftChild.node()) || isKnownNotNumber(rightChild.node())) {
+ JSValueOperand left(this, leftChild);
+ JSValueOperand right(this, rightChild);
</ins><span class="cx"> JSValueRegs leftRegs = left.jsValueRegs();
</span><span class="cx"> JSValueRegs rightRegs = right.jsValueRegs();
</span><span class="cx"> #if USE(JSVALUE64)
</span><span class="lines">@@ -2809,27 +2812,6 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- bool leftIsConstInt32 = node->child1()->isInt32Constant();
- bool rightIsConstInt32 = node->child2()->isInt32Constant();
-
- // The DFG does not always fold the sum of 2 constant int operands together.
- if (leftIsConstInt32 && rightIsConstInt32) {
-#if USE(JSVALUE64)
- GPRTemporary result(this);
- JSValueRegs resultRegs = JSValueRegs(result.gpr());
-#else
- GPRTemporary resultTag(this);
- GPRTemporary resultPayload(this);
- JSValueRegs resultRegs = JSValueRegs(resultPayload.gpr(), resultTag.gpr());
-#endif
- int64_t leftConst = node->child1()->asInt32();
- int64_t rightConst = node->child2()->asInt32();
- int64_t resultConst = leftConst + rightConst;
- m_jit.moveValue(JSValue(resultConst), resultRegs);
- jsValueResult(resultRegs, node);
- return;
- }
-
</del><span class="cx"> Optional<JSValueOperand> left;
</span><span class="cx"> Optional<JSValueOperand> right;
</span><span class="cx">
</span><span class="lines">@@ -2856,22 +2838,24 @@
</span><span class="cx"> FPRReg scratchFPR = fprScratch.fpr();
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- SnippetOperand leftOperand(m_state.forNode(node->child1()).resultType());
- SnippetOperand rightOperand(m_state.forNode(node->child2()).resultType());
</del><ins>+ SnippetOperand leftOperand(m_state.forNode(leftChild).resultType());
+ SnippetOperand rightOperand(m_state.forNode(rightChild).resultType());
</ins><span class="cx">
</span><del>- if (leftIsConstInt32)
- leftOperand.setConstInt32(node->child1()->asInt32());
- if (rightIsConstInt32)
- rightOperand.setConstInt32(node->child2()->asInt32());
</del><ins>+ // The snippet generator does not support both operands being constant. If the left
+ // operand is already const, we'll ignore the right operand's constness.
+ if (leftChild->isInt32Constant())
+ leftOperand.setConstInt32(leftChild->asInt32());
+ else if (rightChild->isInt32Constant())
+ rightOperand.setConstInt32(rightChild->asInt32());
</ins><span class="cx">
</span><span class="cx"> ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
</span><span class="cx">
</span><span class="cx"> if (!leftOperand.isConst()) {
</span><del>- left = JSValueOperand(this, node->child1());
</del><ins>+ left = JSValueOperand(this, leftChild);
</ins><span class="cx"> leftRegs = left->jsValueRegs();
</span><span class="cx"> }
</span><span class="cx"> if (!rightOperand.isConst()) {
</span><del>- right = JSValueOperand(this, node->child2());
</del><ins>+ right = JSValueOperand(this, rightChild);
</ins><span class="cx"> rightRegs = right->jsValueRegs();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -2886,14 +2870,12 @@
</span><span class="cx">
</span><span class="cx"> silentSpillAllRegisters(resultRegs);
</span><span class="cx">
</span><del>- if (leftIsConstInt32) {
</del><ins>+ if (leftOperand.isConst()) {
</ins><span class="cx"> leftRegs = resultRegs;
</span><del>- int64_t leftConst = node->child1()->asInt32();
- m_jit.moveValue(JSValue(leftConst), leftRegs);
- } else if (rightIsConstInt32) {
</del><ins>+ m_jit.moveValue(leftChild->asJSValue(), leftRegs);
+ } else if (rightOperand.isConst()) {
</ins><span class="cx"> rightRegs = resultRegs;
</span><del>- int64_t rightConst = node->child2()->asInt32();
- m_jit.moveValue(JSValue(rightConst), rightRegs);
</del><ins>+ m_jit.moveValue(rightChild->asJSValue(), rightRegs);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> callOperation(operationValueAdd, resultRegs, leftRegs, rightRegs);
</span><span class="lines">@@ -3209,9 +3191,12 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case UntypedUse: {
</span><del>- JSValueOperand left(this, node->child1());
- JSValueOperand right(this, node->child2());
</del><ins>+ Edge& leftChild = node->child1();
+ Edge& rightChild = node->child2();
</ins><span class="cx">
</span><ins>+ JSValueOperand left(this, leftChild);
+ JSValueOperand right(this, rightChild);
+
</ins><span class="cx"> JSValueRegs leftRegs = left.jsValueRegs();
</span><span class="cx"> JSValueRegs rightRegs = right.jsValueRegs();
</span><span class="cx">
</span><span class="lines">@@ -3235,8 +3220,8 @@
</span><span class="cx"> FPRReg scratchFPR = fprScratch.fpr();
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- SnippetOperand leftOperand(m_state.forNode(node->child1()).resultType());
- SnippetOperand rightOperand(m_state.forNode(node->child2()).resultType());
</del><ins>+ SnippetOperand leftOperand(m_state.forNode(leftChild).resultType());
+ SnippetOperand rightOperand(m_state.forNode(rightChild).resultType());
</ins><span class="cx">
</span><span class="cx"> JITSubGenerator gen(leftOperand, rightOperand, resultRegs, leftRegs, rightRegs,
</span><span class="cx"> leftFPR, rightFPR, scratchGPR, scratchFPR);
</span><span class="lines">@@ -3501,12 +3486,14 @@
</span><span class="cx"> SnippetOperand leftOperand(m_state.forNode(leftChild).resultType());
</span><span class="cx"> SnippetOperand rightOperand(m_state.forNode(rightChild).resultType());
</span><span class="cx">
</span><ins>+ // The snippet generator does not support both operands being constant. If the left
+ // operand is already const, we'll ignore the right operand's constness.
</ins><span class="cx"> if (leftChild->isInt32Constant())
</span><span class="cx"> leftOperand.setConstInt32(leftChild->asInt32());
</span><del>- if (rightChild->isInt32Constant())
</del><ins>+ else if (rightChild->isInt32Constant())
</ins><span class="cx"> rightOperand.setConstInt32(rightChild->asInt32());
</span><span class="cx">
</span><del>- RELEASE_ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
</del><ins>+ ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
</ins><span class="cx">
</span><span class="cx"> if (!leftOperand.isPositiveConstInt32()) {
</span><span class="cx"> left = JSValueOperand(this, leftChild);
</span><span class="lines">@@ -3531,8 +3518,7 @@
</span><span class="cx"> leftRegs = resultRegs;
</span><span class="cx"> int64_t leftConst = leftOperand.asConstInt32();
</span><span class="cx"> m_jit.moveValue(JSValue(leftConst), leftRegs);
</span><del>- }
- if (rightOperand.isPositiveConstInt32()) {
</del><ins>+ } else if (rightOperand.isPositiveConstInt32()) {
</ins><span class="cx"> rightRegs = resultRegs;
</span><span class="cx"> int64_t rightConst = rightOperand.asConstInt32();
</span><span class="cx"> m_jit.moveValue(JSValue(rightConst), rightRegs);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (193792 => 193793)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-12-09 00:08:09 UTC (rev 193792)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-12-09 00:12:48 UTC (rev 193793)
</span><span class="lines">@@ -1527,12 +1527,11 @@
</span><span class="cx"> SnippetOperand leftOperand(abstractValue(leftChild).resultType());
</span><span class="cx"> SnippetOperand rightOperand(abstractValue(rightChild).resultType());
</span><span class="cx">
</span><del>- // The DFG does not always fold the sum of 2 constant int operands together.
</del><span class="cx"> // Because the snippet does not support both operands being constant, if the left
</span><span class="cx"> // operand is already a constant, we'll just pretend the right operand is not.
</span><span class="cx"> if (leftChild->isInt32Constant())
</span><span class="cx"> leftOperand.setConstInt32(leftChild->asInt32());
</span><del>- if (!leftOperand.isConst() && rightChild->isInt32Constant())
</del><ins>+ else if (rightChild->isInt32Constant())
</ins><span class="cx"> rightOperand.setConstInt32(rightChild->asInt32());
</span><span class="cx">
</span><span class="cx"> // Arguments: id, bytes, target, numArgs, args...
</span><span class="lines">@@ -1852,9 +1851,11 @@
</span><span class="cx"> SnippetOperand leftOperand(abstractValue(leftChild).resultType());
</span><span class="cx"> SnippetOperand rightOperand(abstractValue(rightChild).resultType());
</span><span class="cx">
</span><ins>+ // Because the snippet does not support both operands being constant, if the left
+ // operand is already a constant, we'll just pretend the right operand is not.
</ins><span class="cx"> if (leftChild->isInt32Constant())
</span><span class="cx"> leftOperand.setConstInt32(leftChild->asInt32());
</span><del>- if (rightChild->isInt32Constant())
</del><ins>+ else if (rightChild->isInt32Constant())
</ins><span class="cx"> rightOperand.setConstInt32(rightChild->asInt32());
</span><span class="cx">
</span><span class="cx"> RELEASE_ASSERT(!leftOperand.isConst() || !rightOperand.isConst());
</span></span></pre>
</div>
</div>
</body>
</html>