Title: [188519] trunk/Source/_javascript_Core
- Revision
- 188519
- Author
- [email protected]
- Date
- 2015-08-16 01:13:50 -0700 (Sun, 16 Aug 2015)
Log Message
[JSC] Use (x + x) instead of (x * 2) when possible
https://bugs.webkit.org/show_bug.cgi?id=148051
Patch by Benjamin Poulain <[email protected]> on 2015-08-16
Reviewed by Michael Saboff.
When multiplying a number by 2, JSC was loading a constant "2"
in register and multiplying it with the first number:
mov $0x4000000000000000, %rcx
movd %rcx, %xmm0
mulsd %xmm0, %xmm1
This is a problem for a few reasons.
1) "movd %rcx, %xmm0" only set half of XMM0. This instruction
has to wait for any preceding instruction on XMM0 to finish
before executing.
2) The load and transform itself is large and unecessary.
To fix that, I added a StrengthReductionPhase to transform
multiplications by 2 into a addition.
Unfortunately, that turned the code into:
movsd %xmm0 %xmm1
mulsd %xmm1 %xmm0
The reason is GenerationInfo::canReuse() was not accounting
for nodes using other nodes multiple times.
After fixing that too, we now have the multiplications by 2
done as:
addsd %xmm0 %xmm0
* dfg/DFGGenerationInfo.h:
(JSC::DFG::GenerationInfo::useCount):
(JSC::DFG::GenerationInfo::canReuse): Deleted.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::FPRTemporary::FPRTemporary):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::canReuse):
(JSC::DFG::GPRTemporary::GPRTemporary):
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (188518 => 188519)
--- trunk/Source/_javascript_Core/ChangeLog 2015-08-16 03:46:47 UTC (rev 188518)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-08-16 08:13:50 UTC (rev 188519)
@@ -1,3 +1,48 @@
+2015-08-16 Benjamin Poulain <[email protected]>
+
+ [JSC] Use (x + x) instead of (x * 2) when possible
+ https://bugs.webkit.org/show_bug.cgi?id=148051
+
+ Reviewed by Michael Saboff.
+
+ When multiplying a number by 2, JSC was loading a constant "2"
+ in register and multiplying it with the first number:
+
+ mov $0x4000000000000000, %rcx
+ movd %rcx, %xmm0
+ mulsd %xmm0, %xmm1
+
+ This is a problem for a few reasons.
+ 1) "movd %rcx, %xmm0" only set half of XMM0. This instruction
+ has to wait for any preceding instruction on XMM0 to finish
+ before executing.
+ 2) The load and transform itself is large and unecessary.
+
+ To fix that, I added a StrengthReductionPhase to transform
+ multiplications by 2 into a addition.
+
+ Unfortunately, that turned the code into:
+ movsd %xmm0 %xmm1
+ mulsd %xmm1 %xmm0
+
+ The reason is GenerationInfo::canReuse() was not accounting
+ for nodes using other nodes multiple times.
+
+ After fixing that too, we now have the multiplications by 2
+ done as:
+ addsd %xmm0 %xmm0
+
+ * dfg/DFGGenerationInfo.h:
+ (JSC::DFG::GenerationInfo::useCount):
+ (JSC::DFG::GenerationInfo::canReuse): Deleted.
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::FPRTemporary::FPRTemporary):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::canReuse):
+ (JSC::DFG::GPRTemporary::GPRTemporary):
+ * dfg/DFGStrengthReductionPhase.cpp:
+ (JSC::DFG::StrengthReductionPhase::handleNode):
+
2015-08-14 Basile Clement <[email protected]>
Occasional failure in v8-v6/v8-raytrace.js.ftl-eager
Modified: trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h (188518 => 188519)
--- trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h 2015-08-16 03:46:47 UTC (rev 188518)
+++ trunk/Source/_javascript_Core/dfg/DFGGenerationInfo.h 2015-08-16 08:13:50 UTC (rev 188519)
@@ -189,10 +189,10 @@
// Used to check the operands of operations to see if they are on
// their last use; in some cases it may be safe to reuse the same
// machine register for the result of the operation.
- bool canReuse()
+ uint32_t useCount()
{
ASSERT(m_useCount);
- return m_useCount == 1;
+ return m_useCount;
}
// Get the format of the value in machine registers (or 'none').
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (188518 => 188519)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-08-16 03:46:47 UTC (rev 188518)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2015-08-16 08:13:50 UTC (rev 188519)
@@ -1186,6 +1186,8 @@
m_fpr = m_jit->reuse(op1.fpr());
else if (m_jit->canReuse(op2.node()))
m_fpr = m_jit->reuse(op2.fpr());
+ else if (m_jit->canReuse(op1.node(), op2.node()) && op1.fpr() == op2.fpr())
+ m_fpr = m_jit->reuse(op1.fpr());
else
m_fpr = m_jit->fprAllocate();
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (188518 => 188519)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2015-08-16 03:46:47 UTC (rev 188518)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2015-08-16 08:13:50 UTC (rev 188519)
@@ -162,8 +162,12 @@
// and its machine registers may be reused.
bool canReuse(Node* node)
{
- return generationInfo(node).canReuse();
+ return generationInfo(node).useCount() == 1;
}
+ bool canReuse(Node* nodeA, Node* nodeB)
+ {
+ return nodeA == nodeB && generationInfo(nodeA).useCount() == 2;
+ }
bool canReuse(Edge nodeUse)
{
return canReuse(nodeUse.node());
@@ -2717,6 +2721,8 @@
m_gpr = m_jit->reuse(op1.gpr());
else if (m_jit->canReuse(op2.node()))
m_gpr = m_jit->reuse(op2.gpr());
+ else if (m_jit->canReuse(op1.node(), op2.node()) && op1.gpr() == op2.gpr())
+ m_gpr = m_jit->reuse(op1.gpr());
else
m_gpr = m_jit->allocate();
}
Modified: trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp (188518 => 188519)
--- trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2015-08-16 03:46:47 UTC (rev 188518)
+++ trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2015-08-16 08:13:50 UTC (rev 188519)
@@ -114,10 +114,36 @@
}
break;
- case ArithMul:
+ case ArithMul: {
handleCommutativity();
+ Edge& child2 = m_node->child2();
+ if (child2->isNumberConstant() && child2->asNumber() == 2) {
+ switch (m_node->binaryUseKind()) {
+ case DoubleRepUse:
+ // It is always valuable to get rid of a double multiplication by 2.
+ // We won't have half-register dependencies issues on x86 and we won't have to load the constants.
+ m_node->setOp(ArithAdd);
+ child2.setNode(m_node->child1().node());
+ m_changed = true;
+ break;
+#if USE(JSVALUE64)
+ case Int52RepUse:
+#endif
+ case Int32Use:
+ // For integers, we can only convert compatible modes.
+ // ArithAdd does handle do negative zero check for example.
+ if (m_node->arithMode() == Arith::CheckOverflow || m_node->arithMode() == Arith::Unchecked) {
+ m_node->setOp(ArithAdd);
+ child2.setNode(m_node->child1().node());
+ m_changed = true;
+ }
+ break;
+ default:
+ break;
+ }
+ }
break;
-
+ }
case ArithSub:
if (m_node->child2()->isInt32Constant()
&& m_node->isBinaryUseKind(Int32Use)) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
