Changeset 195654 in webkit


Ignore:
Timestamp:
Jan 26, 2016 10:10:15 PM (8 years ago)
Author:
benjamin@webkit.org
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):

Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r195653 r195654  
     12016-01-26  Benjamin Poulain  <benjamin@webkit.org>
     2
     3        [JSC] When lowering B3 to Air, preferRightForResult() should prefer values from the same block
     4        https://bugs.webkit.org/show_bug.cgi?id=153477
     5
     6        Reviewed by Filip Pizlo.
     7
     8        In cases like this:
     9
     10        Block #0
     11            @1 = something
     12            @2 = Jump #1
     13        Block #1
     14            @3 = something else
     15            @4 = Add(@3, @1)
     16            ...
     17            @42 = Branch(@x, #1, #2)
     18
     19        B3LowerToAir would pick @1 for the argument copied
     20        for what goes into the UseDef side of Add.
     21
     22        This created a bunch of moves that could never be coalesced.
     23        In Kraken's imaging-desaturate, there were enough Moves to slow
     24        down the hot loop.
     25
     26        Ideally, we should not use UseCount for lowering. We should use
     27        the real liveness for preferRightForResult(), and a loop-weighted
     28        use-count for effective addresses. The problem is keeping the cost
     29        low for those simple helpers.
     30
     31        In this patch, I went with a simple heuristic: prioritize the value
     32        defined in the same block for UseDef.
     33
     34        There is one other way that would be cheap but a bit invasive:
     35        -Get rid of UseDef.
     36        -Make every ops, 3 operands.
     37        -Tell the register allocator to attempt aliasing of the 2 uses
     38         with the def.
     39        -If the allocator fails, just add a move as needed.
     40
     41        For now, the simple heuristic seems okay for the cases have.
     42
     43        * b3/B3LowerToAir.cpp:
     44        (JSC::B3::Air::LowerToAir::preferRightForResult):
     45
    1462016-01-26  Filip Pizlo  <fpizlo@apple.com>
    247
  • trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp

    r195620 r195654  
    584584        //
    585585        // Currently, we have machinery in place to recognize super obvious forms of the latter issue.
    586 
    587         bool result = m_useCounts.numUsingInstructions(right) == 1;
    588586       
    589587        // We recognize when a child is a Phi that has this value as one of its children. We're very
     
    591589        bool leftIsPhiWithThis = m_phiChildren[left].transitivelyUses(m_value);
    592590        bool rightIsPhiWithThis = m_phiChildren[right].transitivelyUses(m_value);
    593        
     591
    594592        if (leftIsPhiWithThis != rightIsPhiWithThis)
    595             result = rightIsPhiWithThis;
    596 
    597         return result;
     593            return rightIsPhiWithThis;
     594
     595        bool leftResult = m_useCounts.numUsingInstructions(left) == 1;
     596        bool rightResult = m_useCounts.numUsingInstructions(right) == 1;
     597        if (leftResult && rightResult) {
     598            // If one operand is not in the block, it could be in a block dominating a loop
     599            // containing m_value.
     600            if (left->owner == m_value->owner)
     601                return false;
     602            if (right->owner == m_value->owner)
     603                return true;
     604        }
     605
     606        return rightResult;
    598607    }
    599608
Note: See TracChangeset for help on using the changeset viewer.