Title: [195654] trunk/Source/_javascript_Core
- Revision
- 195654
- Author
- [email protected]
- Date
- 2016-01-26 22:10:15 -0800 (Tue, 26 Jan 2016)
Log Message
[JSC] When lowering B3 to Air, preferRightForResult() should prefer values from the same block
https://bugs.webkit.org/show_bug.cgi?id=153477
Reviewed by Filip Pizlo.
In cases like this:
Block #0
@1 = something
@2 = Jump #1
Block #1
@3 = something else
@4 = Add(@3, @1)
...
@42 = Branch(@x, #1, #2)
B3LowerToAir would pick @1 for the argument copied
for what goes into the UseDef side of Add.
This created a bunch of moves that could never be coalesced.
In Kraken's imaging-desaturate, there were enough Moves to slow
down the hot loop.
Ideally, we should not use UseCount for lowering. We should use
the real liveness for preferRightForResult(), and a loop-weighted
use-count for effective addresses. The problem is keeping the cost
low for those simple helpers.
In this patch, I went with a simple heuristic: prioritize the value
defined in the same block for UseDef.
There is one other way that would be cheap but a bit invasive:
-Get rid of UseDef.
-Make every ops, 3 operands.
-Tell the register allocator to attempt aliasing of the 2 uses
with the def.
-If the allocator fails, just add a move as needed.
For now, the simple heuristic seems okay for the cases have.
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::preferRightForResult):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (195653 => 195654)
--- trunk/Source/_javascript_Core/ChangeLog 2016-01-27 03:49:35 UTC (rev 195653)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-01-27 06:10:15 UTC (rev 195654)
@@ -1,3 +1,48 @@
+2016-01-26 Benjamin Poulain <[email protected]>
+
+ [JSC] When lowering B3 to Air, preferRightForResult() should prefer values from the same block
+ https://bugs.webkit.org/show_bug.cgi?id=153477
+
+ Reviewed by Filip Pizlo.
+
+ In cases like this:
+
+ Block #0
+ @1 = something
+ @2 = Jump #1
+ Block #1
+ @3 = something else
+ @4 = Add(@3, @1)
+ ...
+ @42 = Branch(@x, #1, #2)
+
+ B3LowerToAir would pick @1 for the argument copied
+ for what goes into the UseDef side of Add.
+
+ This created a bunch of moves that could never be coalesced.
+ In Kraken's imaging-desaturate, there were enough Moves to slow
+ down the hot loop.
+
+ Ideally, we should not use UseCount for lowering. We should use
+ the real liveness for preferRightForResult(), and a loop-weighted
+ use-count for effective addresses. The problem is keeping the cost
+ low for those simple helpers.
+
+ In this patch, I went with a simple heuristic: prioritize the value
+ defined in the same block for UseDef.
+
+ There is one other way that would be cheap but a bit invasive:
+ -Get rid of UseDef.
+ -Make every ops, 3 operands.
+ -Tell the register allocator to attempt aliasing of the 2 uses
+ with the def.
+ -If the allocator fails, just add a move as needed.
+
+ For now, the simple heuristic seems okay for the cases have.
+
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::preferRightForResult):
+
2016-01-26 Filip Pizlo <[email protected]>
Tail duplication should break critical edges first
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (195653 => 195654)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-01-27 03:49:35 UTC (rev 195653)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-01-27 06:10:15 UTC (rev 195654)
@@ -583,18 +583,27 @@
// - A child might be a candidate for coalescing with this value.
//
// Currently, we have machinery in place to recognize super obvious forms of the latter issue.
-
- bool result = m_useCounts.numUsingInstructions(right) == 1;
// We recognize when a child is a Phi that has this value as one of its children. We're very
// conservative about this; for example we don't even consider transitive Phi children.
bool leftIsPhiWithThis = m_phiChildren[left].transitivelyUses(m_value);
bool rightIsPhiWithThis = m_phiChildren[right].transitivelyUses(m_value);
-
+
if (leftIsPhiWithThis != rightIsPhiWithThis)
- result = rightIsPhiWithThis;
+ return rightIsPhiWithThis;
- return result;
+ bool leftResult = m_useCounts.numUsingInstructions(left) == 1;
+ bool rightResult = m_useCounts.numUsingInstructions(right) == 1;
+ if (leftResult && rightResult) {
+ // If one operand is not in the block, it could be in a block dominating a loop
+ // containing m_value.
+ if (left->owner == m_value->owner)
+ return false;
+ if (right->owner == m_value->owner)
+ return true;
+ }
+
+ return rightResult;
}
template<Air::Opcode opcode32, Air::Opcode opcode64, Air::Opcode opcodeDouble, Air::Opcode opcodeFloat, Commutativity commutativity = NotCommutative>
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
