⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 242857 in webkit


Ignore:
Timestamp:
Mar 13, 2019, 1:24:38 AM (7 years ago)
Author:
bshafiei@apple.com
Message:

Cherry-pick r242569. rdar://problem/48839273

Air::reportUsedRegisters must padInterference
​https://bugs.webkit.org/show_bug.cgi?id=195303
<rdar://problem/48270343>

Reviewed by Keith Miller.

JSTests:

  • stress/optional-def-arg-width-should-be-both-early-and-late-use.js: Added.

Source/JavaScriptCore:

reportUsedRegisters uses reg liveness to eliminate loads/moves into dead
registers. However, liveness can report incorrect results in certain
scenarios when considering liveness at instruction boundaries. For example,
it can go wrong when an Inst has a LateUse of a register and the following
Inst has an EarlyDef of that same register. Such a scenario could lead us
to incorrectly say the register is not live-in to the first Inst. Pad
interference inserts Nops between such instruction boundaries that cause
this issue.

The test with this patch fixes the issue in reportUsedRegisters. This patch
also conservatively makes it so that lowerAfterRegAlloc calls padInterference
since it also reasons about liveness.

  • b3/air/AirLowerAfterRegAlloc.cpp: (JSC::B3::Air::lowerAfterRegAlloc):
  • b3/air/AirPadInterference.h:
  • b3/air/AirReportUsedRegisters.cpp: (JSC::B3::Air::reportUsedRegisters):
  • b3/testb3.cpp: (JSC::B3::testReportUsedRegistersLateUseNotDead): (JSC::B3::run):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@242569 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-607-branch
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-607-branch/JSTests/ChangeLog

    r242855 r242857  
     12019-03-13  Babak Shafiei  <bshafiei@apple.com>
     2
     3        Cherry-pick r242569. rdar://problem/48839273
     4
     5    Air::reportUsedRegisters must padInterference
     6    https://bugs.webkit.org/show_bug.cgi?id=195303
     7    <rdar://problem/48270343>
     8   
     9    Reviewed by Keith Miller.
     10   
     11    JSTests:
     12   
     13    * stress/optional-def-arg-width-should-be-both-early-and-late-use.js: Added.
     14   
     15    Source/JavaScriptCore:
     16   
     17    reportUsedRegisters uses reg liveness to eliminate loads/moves into dead
     18    registers. However, liveness can report incorrect results in certain
     19    scenarios when considering liveness at instruction boundaries. For example,
     20    it can go wrong when an Inst has a LateUse of a register and the following
     21    Inst has an EarlyDef of that same register. Such a scenario could lead us
     22    to incorrectly say the register is not live-in to the first Inst. Pad
     23    interference inserts Nops between such instruction boundaries that cause
     24    this issue.
     25   
     26    The test with this patch fixes the issue in reportUsedRegisters. This patch
     27    also conservatively makes it so that lowerAfterRegAlloc calls padInterference
     28    since it also reasons about liveness.
     29   
     30    * b3/air/AirLowerAfterRegAlloc.cpp:
     31    (JSC::B3::Air::lowerAfterRegAlloc):
     32    * b3/air/AirPadInterference.h:
     33    * b3/air/AirReportUsedRegisters.cpp:
     34    (JSC::B3::Air::reportUsedRegisters):
     35    * b3/testb3.cpp:
     36    (JSC::B3::testReportUsedRegistersLateUseNotDead):
     37    (JSC::B3::run):
     38   
     39   
     40    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242569 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     41
     42    2019-03-06  Saam Barati  <sbarati@apple.com>
     43
     44            Air::reportUsedRegisters must padInterference
     45            https://bugs.webkit.org/show_bug.cgi?id=195303
     46            <rdar://problem/48270343>
     47
     48            Reviewed by Keith Miller.
     49
     50            * stress/optional-def-arg-width-should-be-both-early-and-late-use.js: Added.
     51
    1522019-03-13  Babak Shafiei  <bshafiei@apple.com>
    253

  • branches/safari-607-branch/Source/JavaScriptCore/ChangeLog

    r242855 r242857  
     12019-03-13  Babak Shafiei  <bshafiei@apple.com>
     2
     3        Cherry-pick r242569. rdar://problem/48839273
     4
     5    Air::reportUsedRegisters must padInterference
     6    https://bugs.webkit.org/show_bug.cgi?id=195303
     7    <rdar://problem/48270343>
     8   
     9    Reviewed by Keith Miller.
     10   
     11    JSTests:
     12   
     13    * stress/optional-def-arg-width-should-be-both-early-and-late-use.js: Added.
     14   
     15    Source/JavaScriptCore:
     16   
     17    reportUsedRegisters uses reg liveness to eliminate loads/moves into dead
     18    registers. However, liveness can report incorrect results in certain
     19    scenarios when considering liveness at instruction boundaries. For example,
     20    it can go wrong when an Inst has a LateUse of a register and the following
     21    Inst has an EarlyDef of that same register. Such a scenario could lead us
     22    to incorrectly say the register is not live-in to the first Inst. Pad
     23    interference inserts Nops between such instruction boundaries that cause
     24    this issue.
     25   
     26    The test with this patch fixes the issue in reportUsedRegisters. This patch
     27    also conservatively makes it so that lowerAfterRegAlloc calls padInterference
     28    since it also reasons about liveness.
     29   
     30    * b3/air/AirLowerAfterRegAlloc.cpp:
     31    (JSC::B3::Air::lowerAfterRegAlloc):
     32    * b3/air/AirPadInterference.h:
     33    * b3/air/AirReportUsedRegisters.cpp:
     34    (JSC::B3::Air::reportUsedRegisters):
     35    * b3/testb3.cpp:
     36    (JSC::B3::testReportUsedRegistersLateUseNotDead):
     37    (JSC::B3::run):
     38   
     39   
     40    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242569 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     41
     42    2019-03-06  Saam Barati  <sbarati@apple.com>
     43
     44            Air::reportUsedRegisters must padInterference
     45            https://bugs.webkit.org/show_bug.cgi?id=195303
     46            <rdar://problem/48270343>
     47
     48            Reviewed by Keith Miller.
     49
     50            reportUsedRegisters uses reg liveness to eliminate loads/moves into dead
     51            registers. However, liveness can report incorrect results in certain
     52            scenarios when considering liveness at instruction boundaries. For example,
     53            it can go wrong when an Inst has a LateUse of a register and the following
     54            Inst has an EarlyDef of that same register. Such a scenario could lead us
     55            to incorrectly say the register is not live-in to the first Inst. Pad
     56            interference inserts Nops between such instruction boundaries that cause
     57            this issue.
     58
     59            The test with this patch fixes the issue in reportUsedRegisters. This patch
     60            also conservatively makes it so that lowerAfterRegAlloc calls padInterference
     61            since it also reasons about liveness.
     62
     63            * b3/air/AirLowerAfterRegAlloc.cpp:
     64            (JSC::B3::Air::lowerAfterRegAlloc):
     65            * b3/air/AirPadInterference.h:
     66            * b3/air/AirReportUsedRegisters.cpp:
     67            (JSC::B3::Air::reportUsedRegisters):
     68            * b3/testb3.cpp:
     69            (JSC::B3::testReportUsedRegistersLateUseNotDead):
     70            (JSC::B3::run):
     71
    1722019-03-13  Babak Shafiei  <bshafiei@apple.com>
    273

  • branches/safari-607-branch/Source/JavaScriptCore/b3/air/AirLowerAfterRegAlloc.cpp

    r221954 r242857  
    3535#include "AirInsertionSet.h"
    3636#include "AirInstInlines.h"
     37#include "AirPadInterference.h"
    3738#include "AirRegLiveness.h"
    3839#include "AirPhaseScope.h"
    … …  
    7778    if (!haveAnyRelevant)
    7879        return;
     80
     81    padInterference(code);
    7982
    8083    HashMap<Inst*, RegisterSet> usedRegisters;

  • branches/safari-607-branch/Source/JavaScriptCore/b3/air/AirPadInterference.h

    r207434 r242857  
    3333
    3434// This isn't a phase - it's meant to be a utility that other phases use. Air reasons about liveness by
    35 // reasoning about interference at boundaries between instructions. This can go wrong - for example, a
     35// reasoning about interference at boundaries between instructions. This is convenient because it works
     36// great in the most common case: early uses and late defs. However, this can go wrong - for example, a
    3637// late use in one instruction doesn't actually interfere with an early def of the next instruction, but
    37 // Air thinks that it does. This is convenient because it works great in the most common case: early uses
    38 // and late defs. In practice, only the register allocators need to use this, since only they need to be
    39 // able to color the interference graph using a bounded number of colors.
     38// Air thinks that it does. It can also go wrong by having liveness incorrectly report that something is
     39// dead when it isn't.
    4040//
    4141// See https://bugs.webkit.org/show_bug.cgi?id=163548#c2 for more info.

  • branches/safari-607-branch/Source/JavaScriptCore/b3/air/AirReportUsedRegisters.cpp

    r225893 r242857  
    3232#include "AirCode.h"
    3333#include "AirInstInlines.h"
     34#include "AirPadInterference.h"
    3435#include "AirRegLiveness.h"
    3536#include "AirPhaseScope.h"
    … …  
    4243   
    4344    static constexpr bool verbose = false;
     45
     46    padInterference(code);
    4447   
    4548    if (verbose)

  • branches/safari-607-branch/Source/JavaScriptCore/b3/testb3.cpp

    r239187 r242857  
    1627916279    demoteValues(proc, valuesToDemote);
    1628016280    validate(proc);
     16281}
     16282
     16283void testReportUsedRegistersLateUseFollowedByEarlyDefDoesNotMarkUseAsDead()
     16284{
     16285    Procedure proc;
     16286    if (proc.optLevel() < 2)
     16287        return;
     16288    BasicBlock* root = proc.addBlock();
     16289
     16290    RegisterSet allRegs = RegisterSet::allGPRs();
     16291    allRegs.exclude(RegisterSet::stackRegisters());
     16292    allRegs.exclude(RegisterSet::reservedHardwareRegisters());
     16293
     16294    {
     16295        // Make every reg 42 (just needs to be a value other than 10).
     16296        PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Void, Origin());
     16297        Value* const42 = root->appendNew<Const32Value>(proc, Origin(), 42);
     16298        for (Reg reg : allRegs)
     16299            patchpoint->append(const42, ValueRep::reg(reg));
     16300        patchpoint->setGenerator([&] (CCallHelpers&, const StackmapGenerationParams&) { });
     16301    }
     16302
     16303    {
     16304        PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Void, Origin());
     16305        Value* const10 = root->appendNew<Const32Value>(proc, Origin(), 10);
     16306        for (Reg reg : allRegs)
     16307            patchpoint->append(const10, ValueRep::lateReg(reg));
     16308        patchpoint->setGenerator([&] (CCallHelpers& jit, const StackmapGenerationParams&) {
     16309            for (Reg reg : allRegs) {
     16310                auto done = jit.branch32(CCallHelpers::Equal, reg.gpr(), CCallHelpers::TrustedImm32(10));
     16311                jit.breakpoint();
     16312                done.link(&jit);
     16313            }
     16314        });
     16315    }
     16316
     16317    {
     16318        PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
     16319        patchpoint->resultConstraint = ValueRep::SomeEarlyRegister;
     16320        patchpoint->setGenerator([&] (CCallHelpers&, const StackmapGenerationParams& params) {
     16321            RELEASE_ASSERT(allRegs.contains(params[0].gpr()));
     16322        });
     16323    }
     16324
     16325    root->appendNewControlValue(proc, Return, Origin());
     16326
     16327    compileAndRun<void>(proc);
    1628116328}
    1628216329
    … …  
    1787617923    }
    1787717924
     17925    RUN(testReportUsedRegistersLateUseFollowedByEarlyDefDoesNotMarkUseAsDead());
     17926
    1787817927    if (tasks.isEmpty())
    1787917928        usage();
Note: See TracChangeset for help on using the changeset viewer.