Changeset 192355 in webkit


Ignore:
Timestamp:
Nov 11, 2015, 11:38:00 PM (10 years ago)
Author:
benjamin@webkit.org
Message:

[JSC] Air: we have more register than what the allocator believed
​https://bugs.webkit.org/show_bug.cgi?id=151182

Patch by Benjamin Poulain <​bpoulain@apple.com> on 2015-11-11
Reviewed by Michael Saboff.

I was using GPRInfo/FPRInfo to get the number of register while coloring the interference graph.
The problem is, those classes are lying about register availability.

They don't report stuff reserved by the MacroAssembler and reserve some registers.
FPRInfo is the worst, reporting only 6 of the 15 registers we have.

The ground truth in our case is that we can color with all the registers returned
by regsInPriorityOrder(). I changed IteratedRegisterCoalescingAllocator to use that value.

The new test testSpillFP() covers simple spilling of doubles.

  • b3/air/AirIteratedRegisterCoalescing.cpp:

(JSC::B3::Air::IteratedRegisterCoalescingAllocator::IteratedRegisterCoalescingAllocator):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::makeWorkList):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::decrementDegree):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::precoloredCoalescingHeuristic):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::conservativeHeuristic):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::addWorkList):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::combine):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::freezeMoves):

  • b3/testb3.cpp:

(JSC::B3::testSpillFP):
(JSC::B3::run):

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r192353 r192355  
     12015-11-11  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        [JSC] Air: we have more register than what the allocator believed
     4        https://bugs.webkit.org/show_bug.cgi?id=151182
     5
     6        Reviewed by Michael Saboff.
     7
     8        I was using GPRInfo/FPRInfo to get the number of register while coloring the interference graph.
     9        The problem is, those classes are lying about register availability.
     10
     11        They don't report stuff reserved by the MacroAssembler and reserve some registers.
     12        FPRInfo is the worst, reporting only 6 of the 15 registers we have.
     13
     14        The ground truth in our case is that we can color with all the registers returned
     15        by regsInPriorityOrder(). I changed IteratedRegisterCoalescingAllocator to use that value.
     16
     17        The new test testSpillFP() covers simple spilling of doubles.
     18
     19        * b3/air/AirIteratedRegisterCoalescing.cpp:
     20        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::IteratedRegisterCoalescingAllocator):
     21        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::makeWorkList):
     22        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::decrementDegree):
     23        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::precoloredCoalescingHeuristic):
     24        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::conservativeHeuristic):
     25        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::addWorkList):
     26        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::combine):
     27        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::freezeMoves):
     28        * b3/testb3.cpp:
     29        (JSC::B3::testSpillFP):
     30        (JSC::B3::run):
     31
    1322015-11-11  Mark Lam  <mark.lam@apple.com>
    233

  • trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp

    r192292 r192355  
    129129
    130130template<Arg::Type type>
    131 struct Bank;
    132 
    133 template<>
    134 struct Bank<Arg::GP> {
    135     typedef GPRInfo Info;
    136 };
    137 
    138 template<>
    139 struct Bank<Arg::FP> {
    140     typedef FPRInfo Info;
    141 };
    142 
    143 template<Arg::Type type>
    144131class IteratedRegisterCoalescingAllocator {
    145132public:
    146133    IteratedRegisterCoalescingAllocator(Code& code)
     134        : m_numberOfRegisters(regsInPriorityOrder(type).size())
    147135    {
    148136        initializeDegrees(code);
    … …  
    325313            Tmp tmp = AbsoluteTmpHelper<type>::tmpFromAbsoluteIndex(i);
    326314
    327             if (degree >= Bank<type>::Info::numberOfRegisters)
     315            if (degree >= m_numberOfRegisters)
    328316                m_spillWorklist.add(tmp);
    329317            else if (!m_moveList[AbsoluteTmpHelper<type>::absoluteIndex(tmp)].isEmpty())
    … …  
    367355
    368356        unsigned oldDegree = m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(tmp)]--;
    369         if (oldDegree == Bank<type>::Info::numberOfRegisters) {
     357        if (oldDegree == m_numberOfRegisters) {
    370358            enableMovesOnValueAndAdjacents(tmp);
    371359            m_spillWorklist.remove(tmp);
    … …  
    473461            if (!adjacentTmp.isReg()
    474462                && !hasBeenSimplified(adjacentTmp)
    475                 && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= Bank<type>::Info::numberOfRegisters
     463                && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= m_numberOfRegisters
    476464                && !m_interferenceEdges.contains(InterferenceEdge(u, adjacentTmp)))
    477465                return false;
    … …  
    494482        auto adjacentsOfV = m_adjacencyList[AbsoluteTmpHelper<type>::absoluteIndex(v)];
    495483
    496         if (adjacentsOfU.size() + adjacentsOfV.size() < Bank<type>::Info::numberOfRegisters) {
     484        if (adjacentsOfU.size() + adjacentsOfV.size() < m_numberOfRegisters) {
    497485            // Shortcut: if the total number of adjacents is less than the number of register, the condition is always met.
    498486            return true;
    … …  
    504492            ASSERT(adjacentTmp != v);
    505493            ASSERT(adjacentTmp != u);
    506             if (!hasBeenSimplified(adjacentTmp) && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= Bank<type>::Info::numberOfRegisters) {
     494            if (!hasBeenSimplified(adjacentTmp) && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= m_numberOfRegisters) {
    507495                auto addResult = highOrderAdjacents.add(adjacentTmp);
    508                 if (addResult.isNewEntry && highOrderAdjacents.size() >= Bank<type>::Info::numberOfRegisters)
     496                if (addResult.isNewEntry && highOrderAdjacents.size() >= m_numberOfRegisters)
    509497                    return false;
    510498            }
    … …  
    513501            ASSERT(adjacentTmp != u);
    514502            ASSERT(adjacentTmp != v);
    515             if (!hasBeenSimplified(adjacentTmp) && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= Bank<type>::Info::numberOfRegisters) {
     503            if (!hasBeenSimplified(adjacentTmp) && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(adjacentTmp)] >= m_numberOfRegisters) {
    516504                auto addResult = highOrderAdjacents.add(adjacentTmp);
    517                 if (addResult.isNewEntry && highOrderAdjacents.size() >= Bank<type>::Info::numberOfRegisters)
     505                if (addResult.isNewEntry && highOrderAdjacents.size() >= m_numberOfRegisters)
    518506                    return false;
    519507            }
    520508        }
    521509
    522         ASSERT(highOrderAdjacents.size() < Bank<type>::Info::numberOfRegisters);
     510        ASSERT(highOrderAdjacents.size() < m_numberOfRegisters);
    523511        return true;
    524512    }
    … …  
    526514    void addWorkList(Tmp tmp)
    527515    {
    528         if (!tmp.isReg() && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(tmp)] < Bank<type>::Info::numberOfRegisters && !isMoveRelated(tmp)) {
     516        if (!tmp.isReg() && m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(tmp)] < m_numberOfRegisters && !isMoveRelated(tmp)) {
    529517            m_freezeWorklist.remove(tmp);
    530518            m_simplifyWorklist.append(tmp);
    … …  
    548536        });
    549537
    550         if (m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(u)] >= Bank<type>::Info::numberOfRegisters && m_freezeWorklist.remove(u))
     538        if (m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(u)] >= m_numberOfRegisters && m_freezeWorklist.remove(u))
    551539            m_spillWorklist.add(u);
    552540    }
    … …  
    566554
    567555            Tmp otherTmp = inst.args[0].tmp() != tmp ? inst.args[0].tmp() : inst.args[1].tmp();
    568             if (m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(otherTmp)] < Bank<type>::Info::numberOfRegisters && !isMoveRelated(otherTmp)) {
     556            if (m_degrees[AbsoluteTmpHelper<type>::absoluteIndex(otherTmp)] < m_numberOfRegisters && !isMoveRelated(otherTmp)) {
    569557                m_freezeWorklist.remove(otherTmp);
    570558                m_simplifyWorklist.append(otherTmp);
    … …  
    755743    typedef SimpleClassHashTraits<InterferenceEdge> InterferenceEdgeHashTraits;
    756744
     745    unsigned m_numberOfRegisters { 0 };
     746
    757747    // The interference graph.
    758748    HashSet<InterferenceEdge, InterferenceEdgeHash, InterferenceEdgeHashTraits> m_interferenceEdges;

  • trunk/Source/JavaScriptCore/b3/testb3.cpp

    r192347 r192355  
    20762076    root->appendNew<ControlValue>(proc, Return, Origin(), total);
    20772077    compileAndRun<int>(proc, 1, 2);
     2078}
     2079
     2080void testSpillFP()
     2081{
     2082    Procedure proc;
     2083    BasicBlock* root = proc.addBlock();
     2084
     2085    Vector<Value*> sources;
     2086    sources.append(root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0));
     2087    sources.append(root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR1));
     2088
     2089    for (unsigned i = 0; i < 30; ++i) {
     2090        sources.append(
     2091            root->appendNew<Value>(proc, Add, Origin(), sources[sources.size() - 1], sources[sources.size() - 2])
     2092        );
     2093    }
     2094
     2095    Value* total = root->appendNew<ConstDoubleValue>(proc, Origin(), 0.);
     2096    for (Value* value : sources)
     2097        total = root->appendNew<Value>(proc, Add, Origin(), total, value);
     2098
     2099    root->appendNew<ControlValue>(proc, Return, Origin(), total);
     2100    compileAndRun<double>(proc, 1.1, 2.5);
    20782101}
    20792102
    … …  
    38833906
    38843907    RUN(testSpillGP());
     3908    RUN(testSpillFP());
    38853909
    38863910    RUN(testCallSimple(1, 2));
Note: See TracChangeset for help on using the changeset viewer.