Changeset 192540 in webkit


Ignore:
Timestamp:
Nov 17, 2015, 2:31:40 PM (10 years ago)
Author:
fpizlo@apple.com
Message:

CheckAdd/Mul should have commutativity optimizations in B3->Air lowering
​https://bugs.webkit.org/show_bug.cgi?id=151214

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

This is an overhaul of CheckAdd/CheckSub/CheckMul that fixes bugs, improves codegen, and
simplifies the contract between B3 and its client.

Previously, the idea was that the operands to the Air BranchAdd/Sub/Mul matched the children of
the B3 CheckAdd/Sub/Mul, or at least, that's what the B3::CheckSpecial would make you believe.
This meant that B3/Air had to avoid optimizations that would break this protocol. This prevented
commutativity optimizations on CheckAdd/Mul and it also prevented strength reduction from
CheckMul(x, 2) to CheckAdd(x, x), for example. Those optimizations would break things because the
client's Stackmap generation callback was allowed to assume that the first entry in params.reps
was the first child. Also, there was a contract between B3 and its client that for CheckAdd/Sub,
the client would undo the operation by doing the opposite operation with the params.reps[0] as the
source and params.reps[1] as the destination.

This not only prevented commutativity optimizations, it also led to bugs. Here are two bugs that
we had:

  • Add(x, x) would not work. The client would be told to undo the add using %x as both the source and destination. The client would use a sub() instruction. The result would not be x - it would be zero.
  • Mul where the result got coalesced with one of the sources. You can't undo a multiplication, so you need to keep the inputs alive until after the result is computed - i.e. the inputs are late uses. I initially thought I worked around this by using a three-operand form of Mul, but of course that doesn't actually fix the issue.

This patch fixes these issues comprehensively by introducing the following changes:

The params.reps corresponding to the first two children of CheckAdd/Sub/Mul and the first child of
Check are always garbage: if you want to know the values of those children in the slow path, pass
them as additional stackmap children. This makes it clear to the compiler whose values you
actually care about, and frees the compiler to reorder and commute the non-stackmap children.

The "undo" of an Add or Sub is handled internally by B3: the client doesn't have to know anything
about undoing. B3 does it. The contract is simply that if a B3::Value* is a stackmap child of a
CheckXYZ, then the corresponding entry in params.reps inside the stackmap generator callback will
contain the value of that Value*. For Add and Sub, B3 undoes the operation. For Add(x, x), the
undo uses the carry flag and some shift magic. For Mul, B3 uses LateUse:

A new kind of Arg::Role called Arg::LateUse is introduced: This kind of use means that the use
happens at the start of the execution of the next instruction. None of the built-in Air opcodes
use LateUse, but it is used by B3::CheckSpecial. We use it to implement CheckMul.

Finally, this change fixes testComplex to actually create many live variables. This revealed a
really dumb pathology in allocateStack(), and this patch fixes it. Basically, it's a bad idea to
build interference graphs by repeatedly recreating the same cliques.

  • assembler/MacroAssemblerX86Common.h:

(JSC::MacroAssemblerX86Common::test32):
(JSC::MacroAssemblerX86Common::setCarry):
(JSC::MacroAssemblerX86Common::invert):

  • b3/B3CheckSpecial.cpp:

(JSC::B3::Air::numB3Args):
(JSC::B3::CheckSpecial::Key::Key):
(JSC::B3::CheckSpecial::Key::dump):
(JSC::B3::CheckSpecial::CheckSpecial):
(JSC::B3::CheckSpecial::forEachArg):
(JSC::B3::CheckSpecial::isValid):
(JSC::B3::CheckSpecial::generate):
(JSC::B3::CheckSpecial::dumpImpl):
(JSC::B3::CheckSpecial::deepDumpImpl):

  • b3/B3CheckSpecial.h:

(JSC::B3::CheckSpecial::Key::Key):
(JSC::B3::CheckSpecial::Key::operator==):
(JSC::B3::CheckSpecial::Key::operator!=):
(JSC::B3::CheckSpecial::Key::opcode):
(JSC::B3::CheckSpecial::Key::numArgs):
(JSC::B3::CheckSpecial::Key::stackmapRole):
(JSC::B3::CheckSpecial::Key::hash):

  • b3/B3CheckValue.cpp:

(JSC::B3::CheckValue::~CheckValue):
(JSC::B3::CheckValue::convertToAdd):
(JSC::B3::CheckValue::CheckValue):

  • b3/B3CheckValue.h:
  • b3/B3Const32Value.cpp:

(JSC::B3::Const32Value::checkMulConstant):
(JSC::B3::Const32Value::checkNegConstant):
(JSC::B3::Const32Value::divConstant):

  • b3/B3Const32Value.h:
  • b3/B3Const64Value.cpp:

(JSC::B3::Const64Value::checkMulConstant):
(JSC::B3::Const64Value::checkNegConstant):
(JSC::B3::Const64Value::divConstant):

  • b3/B3Const64Value.h:
  • b3/B3LowerToAir.cpp:

(JSC::B3::Air::LowerToAir::appendBinOp):
(JSC::B3::Air::LowerToAir::lower):

  • b3/B3Opcode.h:
  • b3/B3PatchpointSpecial.cpp:

(JSC::B3::PatchpointSpecial::~PatchpointSpecial):
(JSC::B3::PatchpointSpecial::forEachArg):
(JSC::B3::PatchpointSpecial::isValid):

  • b3/B3ReduceStrength.cpp:
  • b3/B3StackmapSpecial.cpp:

(JSC::B3::StackmapSpecial::forEachArgImpl):

  • b3/B3StackmapSpecial.h:
  • b3/B3StackmapValue.cpp:

(JSC::B3::StackmapValue::append):
(JSC::B3::StackmapValue::appendSomeRegister):
(JSC::B3::StackmapValue::setConstrainedChild):

  • b3/B3StackmapValue.h:
  • b3/B3Validate.cpp:
  • b3/B3Value.cpp:

(JSC::B3::Value::checkMulConstant):
(JSC::B3::Value::checkNegConstant):
(JSC::B3::Value::divConstant):

  • b3/B3Value.h:
  • b3/air/AirAllocateStack.cpp:

(JSC::B3::Air::allocateStack):

  • b3/air/AirArg.cpp:

(WTF::printInternal):

  • b3/air/AirArg.h:

(JSC::B3::Air::Arg::isAnyUse):
(JSC::B3::Air::Arg::isEarlyUse):
(JSC::B3::Air::Arg::isLateUse):
(JSC::B3::Air::Arg::isDef):
(JSC::B3::Air::Arg::forEachTmp):
(JSC::B3::Air::Arg::isUse): Deleted.

  • b3/air/AirGenerate.cpp:

(JSC::B3::Air::generate):

  • b3/air/AirIteratedRegisterCoalescing.cpp:

(JSC::B3::Air::IteratedRegisterCoalescingAllocator::build):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::allocate):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::InterferenceEdge::hash):
(JSC::B3::Air::IteratedRegisterCoalescingAllocator::InterferenceEdge::dump):
(JSC::B3::Air::addSpillAndFillToProgram):
(JSC::B3::Air::iteratedRegisterCoalescingOnType):
(JSC::B3::Air::iteratedRegisterCoalescing):

  • b3/air/AirLiveness.h:

(JSC::B3::Air::Liveness::Liveness):
(JSC::B3::Air::Liveness::LocalCalc::LocalCalc):
(JSC::B3::Air::Liveness::LocalCalc::live):
(JSC::B3::Air::Liveness::LocalCalc::takeLive):
(JSC::B3::Air::Liveness::LocalCalc::execute):

  • b3/air/AirOpcode.opcodes:
  • b3/air/AirReportUsedRegisters.cpp:

(JSC::B3::Air::reportUsedRegisters):

  • b3/air/AirSpillEverything.cpp:

(JSC::B3::Air::spillEverything):

  • b3/testb3.cpp:

(JSC::B3::testMulArg):
(JSC::B3::testMulArgStore):
(JSC::B3::testMulAddArg):
(JSC::B3::testMulArgs):
(JSC::B3::testComplex):
(JSC::B3::testSimpleCheck):
(JSC::B3::testCheckLessThan):
(JSC::B3::testCheckMegaCombo):
(JSC::B3::testCheckAddImm):
(JSC::B3::testCheckAddImmCommute):
(JSC::B3::testCheckAddImmSomeRegister):
(JSC::B3::testCheckAdd):
(JSC::B3::testCheckAdd64):
(JSC::B3::testCheckSubImm):
(JSC::B3::testCheckSubBadImm):
(JSC::B3::testCheckSub):
(JSC::B3::testCheckSub64):
(JSC::B3::testCheckNeg):
(JSC::B3::testCheckNeg64):
(JSC::B3::testCheckMul):
(JSC::B3::testCheckMulMemory):
(JSC::B3::testCheckMul2):
(JSC::B3::testCheckMul64):
(JSC::B3::run):

Source/WTF:

Disable my failed attempts at perfect forwarding, since they were incorrect, and caused compile
errors if you tried to pass an argument that bound to lvalue. This shouldn't affect performance of
anything we care about, and performance tests seem to confirm that it's all good.

  • wtf/ScopedLambda.h:
Location:
trunk
Files:
34 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r192539 r192540  
     12015-11-17  Filip Pizlo  <fpizlo@apple.com>
     2
     3        CheckAdd/Mul should have commutativity optimizations in B3->Air lowering
     4        https://bugs.webkit.org/show_bug.cgi?id=151214
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        This is an overhaul of CheckAdd/CheckSub/CheckMul that fixes bugs, improves codegen, and
     9        simplifies the contract between B3 and its client.
     10
     11        Previously, the idea was that the operands to the Air BranchAdd/Sub/Mul matched the children of
     12        the B3 CheckAdd/Sub/Mul, or at least, that's what the B3::CheckSpecial would make you believe.
     13        This meant that B3/Air had to avoid optimizations that would break this protocol. This prevented
     14        commutativity optimizations on CheckAdd/Mul and it also prevented strength reduction from
     15        CheckMul(x, 2) to CheckAdd(x, x), for example. Those optimizations would break things because the
     16        client's Stackmap generation callback was allowed to assume that the first entry in params.reps
     17        was the first child. Also, there was a contract between B3 and its client that for CheckAdd/Sub,
     18        the client would undo the operation by doing the opposite operation with the params.reps[0] as the
     19        source and params.reps[1] as the destination.
     20
     21        This not only prevented commutativity optimizations, it also led to bugs. Here are two bugs that
     22        we had:
     23
     24        - Add(x, x) would not work. The client would be told to undo the add using %x as both the source
     25          and destination. The client would use a sub() instruction. The result would not be x - it would
     26          be zero.
     27
     28        - Mul where the result got coalesced with one of the sources. You can't undo a multiplication, so
     29          you need to keep the inputs alive until after the result is computed - i.e. the inputs are late
     30          uses. I initially thought I worked around this by using a three-operand form of Mul, but of
     31          course that doesn't actually fix the issue.
     32
     33        This patch fixes these issues comprehensively by introducing the following changes:
     34
     35        The params.reps corresponding to the first two children of CheckAdd/Sub/Mul and the first child of
     36        Check are always garbage: if you want to know the values of those children in the slow path, pass
     37        them as additional stackmap children. This makes it clear to the compiler whose values you
     38        actually care about, and frees the compiler to reorder and commute the non-stackmap children.
     39
     40        The "undo" of an Add or Sub is handled internally by B3: the client doesn't have to know anything
     41        about undoing. B3 does it. The contract is simply that if a B3::Value* is a stackmap child of a
     42        CheckXYZ, then the corresponding entry in params.reps inside the stackmap generator callback will
     43        contain the value of that Value*. For Add and Sub, B3 undoes the operation. For Add(x, x), the
     44        undo uses the carry flag and some shift magic. For Mul, B3 uses LateUse:
     45
     46        A new kind of Arg::Role called Arg::LateUse is introduced: This kind of use means that the use
     47        happens at the start of the execution of the next instruction. None of the built-in Air opcodes
     48        use LateUse, but it is used by B3::CheckSpecial. We use it to implement CheckMul.
     49
     50        Finally, this change fixes testComplex to actually create many live variables. This revealed a
     51        really dumb pathology in allocateStack(), and this patch fixes it. Basically, it's a bad idea to
     52        build interference graphs by repeatedly recreating the same cliques.
     53
     54        * assembler/MacroAssemblerX86Common.h:
     55        (JSC::MacroAssemblerX86Common::test32):
     56        (JSC::MacroAssemblerX86Common::setCarry):
     57        (JSC::MacroAssemblerX86Common::invert):
     58        * b3/B3CheckSpecial.cpp:
     59        (JSC::B3::Air::numB3Args):
     60        (JSC::B3::CheckSpecial::Key::Key):
     61        (JSC::B3::CheckSpecial::Key::dump):
     62        (JSC::B3::CheckSpecial::CheckSpecial):
     63        (JSC::B3::CheckSpecial::forEachArg):
     64        (JSC::B3::CheckSpecial::isValid):
     65        (JSC::B3::CheckSpecial::generate):
     66        (JSC::B3::CheckSpecial::dumpImpl):
     67        (JSC::B3::CheckSpecial::deepDumpImpl):
     68        * b3/B3CheckSpecial.h:
     69        (JSC::B3::CheckSpecial::Key::Key):
     70        (JSC::B3::CheckSpecial::Key::operator==):
     71        (JSC::B3::CheckSpecial::Key::operator!=):
     72        (JSC::B3::CheckSpecial::Key::opcode):
     73        (JSC::B3::CheckSpecial::Key::numArgs):
     74        (JSC::B3::CheckSpecial::Key::stackmapRole):
     75        (JSC::B3::CheckSpecial::Key::hash):
     76        * b3/B3CheckValue.cpp:
     77        (JSC::B3::CheckValue::~CheckValue):
     78        (JSC::B3::CheckValue::convertToAdd):
     79        (JSC::B3::CheckValue::CheckValue):
     80        * b3/B3CheckValue.h:
     81        * b3/B3Const32Value.cpp:
     82        (JSC::B3::Const32Value::checkMulConstant):
     83        (JSC::B3::Const32Value::checkNegConstant):
     84        (JSC::B3::Const32Value::divConstant):
     85        * b3/B3Const32Value.h:
     86        * b3/B3Const64Value.cpp:
     87        (JSC::B3::Const64Value::checkMulConstant):
     88        (JSC::B3::Const64Value::checkNegConstant):
     89        (JSC::B3::Const64Value::divConstant):
     90        * b3/B3Const64Value.h:
     91        * b3/B3LowerToAir.cpp:
     92        (JSC::B3::Air::LowerToAir::appendBinOp):
     93        (JSC::B3::Air::LowerToAir::lower):
     94        * b3/B3Opcode.h:
     95        * b3/B3PatchpointSpecial.cpp:
     96        (JSC::B3::PatchpointSpecial::~PatchpointSpecial):
     97        (JSC::B3::PatchpointSpecial::forEachArg):
     98        (JSC::B3::PatchpointSpecial::isValid):
     99        * b3/B3ReduceStrength.cpp:
     100        * b3/B3StackmapSpecial.cpp:
     101        (JSC::B3::StackmapSpecial::forEachArgImpl):
     102        * b3/B3StackmapSpecial.h:
     103        * b3/B3StackmapValue.cpp:
     104        (JSC::B3::StackmapValue::append):
     105        (JSC::B3::StackmapValue::appendSomeRegister):
     106        (JSC::B3::StackmapValue::setConstrainedChild):
     107        * b3/B3StackmapValue.h:
     108        * b3/B3Validate.cpp:
     109        * b3/B3Value.cpp:
     110        (JSC::B3::Value::checkMulConstant):
     111        (JSC::B3::Value::checkNegConstant):
     112        (JSC::B3::Value::divConstant):
     113        * b3/B3Value.h:
     114        * b3/air/AirAllocateStack.cpp:
     115        (JSC::B3::Air::allocateStack):
     116        * b3/air/AirArg.cpp:
     117        (WTF::printInternal):
     118        * b3/air/AirArg.h:
     119        (JSC::B3::Air::Arg::isAnyUse):
     120        (JSC::B3::Air::Arg::isEarlyUse):
     121        (JSC::B3::Air::Arg::isLateUse):
     122        (JSC::B3::Air::Arg::isDef):
     123        (JSC::B3::Air::Arg::forEachTmp):
     124        (JSC::B3::Air::Arg::isUse): Deleted.
     125        * b3/air/AirGenerate.cpp:
     126        (JSC::B3::Air::generate):
     127        * b3/air/AirIteratedRegisterCoalescing.cpp:
     128        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::build):
     129        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::allocate):
     130        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::InterferenceEdge::hash):
     131        (JSC::B3::Air::IteratedRegisterCoalescingAllocator::InterferenceEdge::dump):
     132        (JSC::B3::Air::addSpillAndFillToProgram):
     133        (JSC::B3::Air::iteratedRegisterCoalescingOnType):
     134        (JSC::B3::Air::iteratedRegisterCoalescing):
     135        * b3/air/AirLiveness.h:
     136        (JSC::B3::Air::Liveness::Liveness):
     137        (JSC::B3::Air::Liveness::LocalCalc::LocalCalc):
     138        (JSC::B3::Air::Liveness::LocalCalc::live):
     139        (JSC::B3::Air::Liveness::LocalCalc::takeLive):
     140        (JSC::B3::Air::Liveness::LocalCalc::execute):
     141        * b3/air/AirOpcode.opcodes:
     142        * b3/air/AirReportUsedRegisters.cpp:
     143        (JSC::B3::Air::reportUsedRegisters):
     144        * b3/air/AirSpillEverything.cpp:
     145        (JSC::B3::Air::spillEverything):
     146        * b3/testb3.cpp:
     147        (JSC::B3::testMulArg):
     148        (JSC::B3::testMulArgStore):
     149        (JSC::B3::testMulAddArg):
     150        (JSC::B3::testMulArgs):
     151        (JSC::B3::testComplex):
     152        (JSC::B3::testSimpleCheck):
     153        (JSC::B3::testCheckLessThan):
     154        (JSC::B3::testCheckMegaCombo):
     155        (JSC::B3::testCheckAddImm):
     156        (JSC::B3::testCheckAddImmCommute):
     157        (JSC::B3::testCheckAddImmSomeRegister):
     158        (JSC::B3::testCheckAdd):
     159        (JSC::B3::testCheckAdd64):
     160        (JSC::B3::testCheckSubImm):
     161        (JSC::B3::testCheckSubBadImm):
     162        (JSC::B3::testCheckSub):
     163        (JSC::B3::testCheckSub64):
     164        (JSC::B3::testCheckNeg):
     165        (JSC::B3::testCheckNeg64):
     166        (JSC::B3::testCheckMul):
     167        (JSC::B3::testCheckMulMemory):
     168        (JSC::B3::testCheckMul2):
     169        (JSC::B3::testCheckMul64):
     170        (JSC::B3::run):
     171
    11722015-11-17  Filip Pizlo  <fpizlo@apple.com>
    2173

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h

    r192535 r192540  
    16121612    }
    16131613
     1614    void setCarry(RegisterID dest)
     1615    {
     1616        set32(X86Assembler::ConditionC, dest);
     1617    }
     1618
    16141619    // Invert a relational condition, e.g. == becomes !=, < becomes >=, etc.
    16151620    static RelationalCondition invert(RelationalCondition cond)

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

    r192400 r192540  
    4040namespace {
    4141
    42 unsigned numB3Args(Inst& inst)
    43 {
    44     switch (inst.origin->opcode()) {
     42unsigned numB3Args(B3::Opcode opcode)
     43{
     44    switch (opcode) {
    4545    case CheckAdd:
    4646    case CheckSub:
    … …  
    5555}
    5656
     57unsigned numB3Args(Value* value)
     58{
     59    return numB3Args(value->opcode());
     60}
     61
     62unsigned numB3Args(Inst& inst)
     63{
     64    return numB3Args(inst.origin);
     65}
     66
    5767} // anonymous namespace
    5868
    … …  
    6171    m_opcode = inst.opcode;
    6272    m_numArgs = inst.args.size();
     73    m_stackmapRole = Arg::Use;
    6374}
    6475
    6576void CheckSpecial::Key::dump(PrintStream& out) const
    6677{
    67     out.print(m_opcode, "(", m_numArgs, ")");
    68 }
    69 
    70 CheckSpecial::CheckSpecial(Air::Opcode opcode, unsigned numArgs)
     78    out.print(m_opcode, "(", m_numArgs, ",", m_stackmapRole, ")");
     79}
     80
     81CheckSpecial::CheckSpecial(Air::Opcode opcode, unsigned numArgs, Arg::Role stackmapRole)
    7182    : m_checkOpcode(opcode)
     83    , m_stackmapRole(stackmapRole)
    7284    , m_numCheckArgs(numArgs)
    7385{
    … …  
    7688
    7789CheckSpecial::CheckSpecial(const CheckSpecial::Key& key)
    78     : CheckSpecial(key.opcode(), key.numArgs())
     90    : CheckSpecial(key.opcode(), key.numArgs(), key.stackmapRole())
    7991{
    8092}
    … …  
    106118    hidden.forEachArg(callback);
    107119    commitHiddenBranch(inst, hidden);
    108     forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, callback);
     120    forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, m_stackmapRole, callback);
    109121}
    110122
    … …  
    131143
    132144    Vector<ValueRep> reps;
    133     if (isCheckMath(value->opcode())) {
    134         if (value->opcode() == CheckMul) {
    135             reps.append(repForArg(*context.code, inst.args[2]));
    136             reps.append(repForArg(*context.code, inst.args[3]));
    137         } else {
    138             if (value->opcode() == CheckSub && value->child(0)->isInt(0))
    139                 reps.append(ValueRep::constant(0));
    140             else
    141                 reps.append(repForArg(*context.code, inst.args[3]));
    142             reps.append(repForArg(*context.code, inst.args[2]));
    143         }
    144     } else {
    145         ASSERT(value->opcode() == Check);
    146         reps.append(ValueRep::constant(1));
    147     }
     145    for (unsigned i = numB3Args(value); i--;)
     146        reps.append(ValueRep());
    148147
    149148    appendRepsImpl(context, m_numCheckArgs + 1, inst, reps);
    150    
     149
     150    // Set aside the args that are relevant to undoing the operation. This is because we don't want to
     151    // capture all of inst in the closure below.
     152    Vector<Arg, 3> args;
     153    for (unsigned i = 0; i < m_numCheckArgs; ++i)
     154        args.append(inst.args[1 + i]);
     155
    151156    context.latePaths.append(
    152157        createSharedTask<GenerationContext::LatePathFunction>(
    153             [=] (CCallHelpers& jit, GenerationContext&) {
     158            [=] (CCallHelpers& jit, GenerationContext& context) {
    154159                fail.link(&jit);
     160
     161                // If necessary, undo the operation.
     162                switch (m_checkOpcode) {
     163                case BranchAdd32:
     164                    if (args[1] == args[2]) {
     165                        // This is ugly, but that's fine - we won't have to do this very often.
     166                        ASSERT(args[1].isGPR());
     167                        GPRReg valueGPR = args[1].gpr();
     168                        GPRReg scratchGPR = CCallHelpers::selectScratchGPR(valueGPR);
     169                        jit.pushToSave(scratchGPR);
     170                        jit.setCarry(scratchGPR);
     171                        jit.lshift32(CCallHelpers::TrustedImm32(31), scratchGPR);
     172                        jit.urshift32(CCallHelpers::TrustedImm32(1), valueGPR);
     173                        jit.or32(scratchGPR, valueGPR);
     174                        jit.popToRestore(scratchGPR);
     175                        break;
     176                    }
     177                    Inst(Sub32, nullptr, args[1], args[2]).generate(jit, context);
     178                    break;
     179                case BranchAdd64:
     180                    if (args[1] == args[2]) {
     181                        // This is ugly, but that's fine - we won't have to do this very often.
     182                        ASSERT(args[1].isGPR());
     183                        GPRReg valueGPR = args[1].gpr();
     184                        GPRReg scratchGPR = CCallHelpers::selectScratchGPR(valueGPR);
     185                        jit.pushToSave(scratchGPR);
     186                        jit.setCarry(scratchGPR);
     187                        jit.lshift64(CCallHelpers::TrustedImm32(63), scratchGPR);
     188                        jit.urshift64(CCallHelpers::TrustedImm32(1), valueGPR);
     189                        jit.or64(scratchGPR, valueGPR);
     190                        jit.popToRestore(scratchGPR);
     191                        break;
     192                    }
     193                    Inst(Sub64, nullptr, args[1], args[2]).generate(jit, context);
     194                    break;
     195                case BranchSub32:
     196                    Inst(Add32, nullptr, args[1], args[2]).generate(jit, context);
     197                    break;
     198                case BranchSub64:
     199                    Inst(Add64, nullptr, args[1], args[2]).generate(jit, context);
     200                    break;
     201                case BranchNeg32:
     202                    Inst(Neg32, nullptr, args[1]).generate(jit, context);
     203                    break;
     204                case BranchNeg64:
     205                    Inst(Neg64, nullptr, args[1]).generate(jit, context);
     206                    break;
     207                default:
     208                    break;
     209                }
    155210               
    156211                StackmapGenerationParams params;
    … …  
    167222void CheckSpecial::dumpImpl(PrintStream& out) const
    168223{
    169     out.print(m_checkOpcode, "(", m_numCheckArgs, ")");
     224    out.print(m_checkOpcode, "(", m_numCheckArgs, ",", m_stackmapRole, ")");
    170225}
    171226

  • trunk/Source/JavaScriptCore/b3/B3CheckSpecial.h

    r192257 r192540  
    2929#if ENABLE(B3_JIT)
    3030
     31#include "AirArg.h"
    3132#include "AirOpcode.h"
    3233#include "B3StackmapSpecial.h"
    … …  
    5758        Key()
    5859            : m_opcode(Air::Nop)
     60            , m_stackmapRole(Air::Arg::Use)
    5961            , m_numArgs(0)
    6062        {
    6163        }
    6264       
    63         Key(Air::Opcode opcode, unsigned numArgs)
     65        Key(Air::Opcode opcode, unsigned numArgs, Air::Arg::Role stackmapRole = Air::Arg::Use)
    6466            : m_opcode(opcode)
     67            , m_stackmapRole(stackmapRole)
    6568            , m_numArgs(numArgs)
    6669        {
    … …  
    7275        {
    7376            return m_opcode == other.m_opcode
    74                 && m_numArgs == other.m_numArgs;
     77                && m_numArgs == other.m_numArgs
     78                && m_stackmapRole == other.m_stackmapRole;
    7579        }
    7680
    … …  
    8488        Air::Opcode opcode() const { return m_opcode; }
    8589        unsigned numArgs() const { return m_numArgs; }
     90        Air::Arg::Role stackmapRole() const { return m_stackmapRole; }
    8691
    8792        void dump(PrintStream& out) const;
    … …  
    8994        Key(WTF::HashTableDeletedValueType)
    9095            : m_opcode(Air::Nop)
     96            , m_stackmapRole(Air::Arg::Use)
    9197            , m_numArgs(1)
    9298        {
    … …  
    101107        {
    102108            // Seriously, we don't need to be smart here. It just doesn't matter.
    103             return m_opcode + m_numArgs;
     109            return m_opcode + m_numArgs + m_stackmapRole;
    104110        }
    105111       
    106112    private:
    107113        Air::Opcode m_opcode;
     114        Air::Arg::Role m_stackmapRole;
    108115        unsigned m_numArgs;
    109116    };
    110117   
    111     CheckSpecial(Air::Opcode, unsigned numArgs);
     118    CheckSpecial(Air::Opcode, unsigned numArgs, Air::Arg::Role stackmapRole = Air::Arg::Use);
    112119    CheckSpecial(const Key&);
    113120    ~CheckSpecial();
    … …  
    134141private:
    135142    Air::Opcode m_checkOpcode;
     143    Air::Arg::Role m_stackmapRole;
    136144    unsigned m_numCheckArgs;
    137145};

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

    r191994 r192540  
    3535}
    3636
     37void CheckValue::convertToAdd()
     38{
     39    RELEASE_ASSERT(opcode() == CheckAdd || opcode() == CheckSub || opcode() == CheckMul);
     40    m_opcode = CheckAdd;
     41}
     42
    3743// Use this form for CheckAdd, CheckSub, and CheckMul.
    3844CheckValue::CheckValue(unsigned index, Opcode opcode, Origin origin, Value* left, Value* right)
    … …  
    4248    ASSERT(left->type() == right->type());
    4349    ASSERT(opcode == CheckAdd || opcode == CheckSub || opcode == CheckMul);
    44     append(ConstrainedValue(left, ValueRep::SomeRegister));
    45     append(ConstrainedValue(right, ValueRep::SomeRegister));
     50    append(ConstrainedValue(left, ValueRep::Any));
     51    append(ConstrainedValue(right, ValueRep::Any));
    4652}
    4753
    … …  
    5157{
    5258    ASSERT(opcode == Check);
    53     append(predicate);
     59    append(ConstrainedValue(predicate, ValueRep::Any));
    5460}
    5561

  • trunk/Source/JavaScriptCore/b3/B3CheckValue.h

    r191993 r192540  
    5050    ~CheckValue();
    5151
     52    void convertToAdd();
     53
    5254private:
    5355    friend class Procedure;

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

    r192400 r192540  
    9999}
    100100
     101Value* Const32Value::checkNegConstant(Procedure& proc) const
     102{
     103    if (m_value == -m_value)
     104        return nullptr;
     105    return negConstant(proc);
     106}
     107
    101108Value* Const32Value::divConstant(Procedure& proc, const Value* other) const
    102109{

  • trunk/Source/JavaScriptCore/b3/B3Const32Value.h

    r192400 r192540  
    4949    Value* checkSubConstant(Procedure&, const Value* other) const override;
    5050    Value* checkMulConstant(Procedure&, const Value* other) const override;
     51    Value* checkNegConstant(Procedure&) const override;
    5152    Value* divConstant(Procedure&, const Value* other) const override;
    5253    Value* bitAndConstant(Procedure&, const Value* other) const override;

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

    r192400 r192540  
    9999}
    100100
     101Value* Const64Value::checkNegConstant(Procedure& proc) const
     102{
     103    if (m_value == -m_value)
     104        return nullptr;
     105    return negConstant(proc);
     106}
     107
    101108Value* Const64Value::divConstant(Procedure& proc, const Value* other) const
    102109{

  • trunk/Source/JavaScriptCore/b3/B3Const64Value.h

    r192400 r192540  
    4949    Value* checkSubConstant(Procedure&, const Value* other) const override;
    5050    Value* checkMulConstant(Procedure&, const Value* other) const override;
     51    Value* checkNegConstant(Procedure&) const override;
    5152    Value* divConstant(Procedure&, const Value* other) const override;
    5253    Value* bitAndConstant(Procedure&, const Value* other) const override;

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

    r192524 r192540  
    599599        }
    600600
     601        // FIXME: If we're going to use a two-operand instruction, and the operand is commutative, we
     602        // should coalesce the result with the operand that is killed.
     603        // https://bugs.webkit.org/show_bug.cgi?id=151321
     604       
    601605        append(relaxedMoveForType(m_value->type()), tmp(left), result);
    602606        append(opcode, tmp(right), result);
    … …  
    15311535        case CheckAdd:
    15321536        case CheckSub: {
    1533             // FIXME: Make this support commutativity. That will let us leverage more instruction forms
    1534             // and it let us commute to maximize coalescing.
    1535             // https://bugs.webkit.org/show_bug.cgi?id=151214
    1536 
    15371537            CheckValue* checkValue = m_value->as<CheckValue>();
    15381538
    … …  
    15441544            // Handle checked negation.
    15451545            if (checkValue->opcode() == CheckSub && left->isInt(0)) {
    1546                 append(relaxedMoveForType(checkValue->type()), tmp(right), result);
     1546                append(Move, tmp(right), result);
    15471547
    15481548                Air::Opcode opcode =
    … …  
    15601560            }
    15611561
    1562             append(relaxedMoveForType(m_value->type()), tmp(left), result);
     1562            // FIXME: Use commutativity of CheckAdd to increase the likelihood of coalescing.
     1563            // https://bugs.webkit.org/show_bug.cgi?id=151321
     1564
     1565            append(Move, tmp(left), result);
    15631566           
    15641567            Air::Opcode opcode = Air::Oops;
    1565             CheckSpecial* special = nullptr;
    15661568            switch (m_value->opcode()) {
    15671569            case CheckAdd:
    15681570                opcode = opcodeForType(BranchAdd32, BranchAdd64, Air::Oops, m_value->type());
    1569                 special = ensureCheckSpecial(opcode, 3);
    15701571                break;
    15711572            case CheckSub:
    15721573                opcode = opcodeForType(BranchSub32, BranchSub64, Air::Oops, m_value->type());
    1573                 special = ensureCheckSpecial(opcode, 3);
    15741574                break;
    15751575            default:
    … …  
    15781578            }
    15791579
    1580             Inst inst(Patch, checkValue, Arg::special(special));
    1581 
    1582             inst.args.append(Arg::resCond(MacroAssembler::Overflow));
    1583 
    15841580            // FIXME: It would be great to fuse Loads into these. We currently don't do it because the
    15851581            // rule for stackmaps is that all addresses are just stack addresses. Maybe we could relax
    15861582            // this rule here.
    15871583            // https://bugs.webkit.org/show_bug.cgi?id=151228
    1588            
     1584
     1585            Arg source;
    15891586            if (imm(right) && isValidForm(opcode, Arg::ResCond, Arg::Imm, Arg::Tmp))
    1590                 inst.args.append(imm(right));
     1587                source = imm(right);
    15911588            else
    1592                 inst.args.append(tmp(right));
     1589                source = tmp(right);
     1590
     1591            // There is a really hilarious case that arises when we do BranchAdd32(%x, %x). We won't emit
     1592            // such code, but the coalescing in our register allocator also does copy propagation, so
     1593            // although we emit:
     1594            //
     1595            //     Move %tmp1, %tmp2
     1596            //     BranchAdd32 %tmp1, %tmp2
     1597            //
     1598            // The register allocator may turn this into:
     1599            //
     1600            //     BranchAdd32 %rax, %rax
     1601            //
     1602            // Currently we handle this by ensuring that even this kind of addition can be undone. We can
     1603            // undo it by using the carry flag. It's tempting to get rid of that code and just "fix" this
     1604            // here by forcing LateUse on the stackmap. If we did that unconditionally, we'd lose a lot of
     1605            // performance. So it's tempting to do it only if left == right. But that creates an awkward
     1606            // constraint on Air: it means that Air would not be allowed to do any copy propagation.
     1607            // Notice that the %rax,%rax situation happened after Air copy-propagated the Move we are
     1608            // emitting. We know that copy-propagating over that Move causes add-to-self. But what if we
     1609            // emit something like a Move - or even do other kinds of copy-propagation on tmp's -
     1610            // somewhere else in this code. The add-to-self situation may only emerge after some other Air
     1611            // optimizations remove other Move's or identity-like operations. That's why we don't use
     1612            // LateUse here to take care of add-to-self.
     1613           
     1614            CheckSpecial* special = ensureCheckSpecial(opcode, 3);
     1615           
     1616            Inst inst(Patch, checkValue, Arg::special(special));
     1617
     1618            inst.args.append(Arg::resCond(MacroAssembler::Overflow));
     1619
     1620            inst.args.append(source);
    15931621            inst.args.append(result);
    1594            
     1622
    15951623            fillStackmap(inst, checkValue, 2);
    15961624
    … …  
    16001628
    16011629        case CheckMul: {
    1602             // Handle multiplication separately. Multiplication is hard because we have to preserve
    1603             // both inputs. This requires using three-operand multiplication, even on platforms where
    1604             // this requires an additional Move.
    1605 
    16061630            CheckValue* checkValue = m_value->as<CheckValue>();
    16071631
    … …  
    16131637            Air::Opcode opcode =
    16141638                opcodeForType(BranchMul32, BranchMul64, Air::Oops, checkValue->type());
    1615             CheckSpecial* special = ensureCheckSpecial(opcode, 4);
     1639            CheckSpecial* special = ensureCheckSpecial(opcode, 3, Arg::LateUse);
    16161640
    16171641            // FIXME: Handle immediates.
    16181642            // https://bugs.webkit.org/show_bug.cgi?id=151230
    16191643
     1644            append(Move, tmp(left), result);
     1645
    16201646            Inst inst(Patch, checkValue, Arg::special(special));
    16211647            inst.args.append(Arg::resCond(MacroAssembler::Overflow));
    1622             inst.args.append(tmp(left));
    16231648            inst.args.append(tmp(right));
    16241649            inst.args.append(result);

  • trunk/Source/JavaScriptCore/b3/B3Opcode.h

    r192295 r192540  
    164164    // after the first CheckAdd executes, the second CheckAdd could not have possibly taken slow
    165165    // path. Therefore, the second CheckAdd's callback is irrelevant.
     166    //
     167    // Note that the first two children of these operations have ValueRep's, both as input constraints and
     168    // in the reps provided to the generator. The output constraints could be anything, and should not be
     169    // inspected for meaning. If you want to capture the values of the inputs, use stackmap arguments.
    166170    CheckAdd,
    167171    CheckSub,
    … …  
    171175    // stackmap with a generation callback. This takes an int argument that this branches on, with
    172176    // full branch fusion in the instruction selector. A true value jumps to the generator's slow
    173     // path.
     177    // path. Note that the predicate child is has both an input and output ValueRep. The input constraint
     178    // must be Any, and the output could be anything.
    174179    Check,
    175180

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

    r191993 r192540  
    4444}
    4545
    46 void PatchpointSpecial::forEachArg(
    47     Inst& inst, const ScopedLambda<Inst::EachArgCallback>& callback)
     46void PatchpointSpecial::forEachArg(Inst& inst, const ScopedLambda<Inst::EachArgCallback>& callback)
    4847{
     48    // FIXME: Allow B3 Patchpoints to specify LateUse.
     49    // https://bugs.webkit.org/show_bug.cgi?id=151335
     50   
    4951    if (inst.origin->type() == Void) {
    50         forEachArgImpl(0, 1, inst, callback);
     52        forEachArgImpl(0, 1, inst, Arg::Use, callback);
    5153        return;
    5254    }
    5355
    5456    callback(inst.args[1], Arg::Def, inst.origin->airType());
    55     forEachArgImpl(0, 2, inst, callback);
     57    forEachArgImpl(0, 2, inst, Arg::Use, callback);
    5658}
    5759

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

    r192400 r192540  
    634634
    635635        case CheckAdd:
    636             // FIXME: Handle commutativity.
    637             // https://bugs.webkit.org/show_bug.cgi?id=151214
    638            
    639636            if (replaceWithNewValue(m_value->child(0)->checkAddConstant(m_proc, m_value->child(1))))
    640637                break;
    641            
    642             if (m_value->child(0)->isInt(0)) {
    643                 m_value->replaceWithIdentity(m_value->child(1));
    644                 m_changed = true;
    645                 break;
    646             }
     638
     639            handleCommutativity();
    647640           
    648641            if (m_value->child(1)->isInt(0)) {
    … …  
    662655                break;
    663656            }
     657
     658            if (Value* negatedConstant = m_value->child(1)->checkNegConstant(m_proc)) {
     659                m_insertionSet.insertValue(m_index, negatedConstant);
     660                m_value->as<CheckValue>()->convertToAdd();
     661                m_value->child(1) = negatedConstant;
     662                m_changed = true;
     663                break;
     664            }
    664665            break;
    665666
    666667        case CheckMul:
    667             // FIXME: Handle commutativity.
    668             // https://bugs.webkit.org/show_bug.cgi?id=151214
    669            
    670668            if (replaceWithNewValue(m_value->child(0)->checkMulConstant(m_proc, m_value->child(1))))
    671669                break;
    672670
    673             if (m_value->child(0)->isInt(1)) {
    674                 m_value->replaceWithIdentity(m_value->child(1));
    675                 m_changed = true;
    676                 break;
    677             }
    678            
    679             if (m_value->child(1)->isInt(1)) {
    680                 m_value->replaceWithIdentity(m_value->child(0));
    681                 m_changed = true;
    682                 break;
    683             }
    684 
    685             if (m_value->child(0)->isInt(0) || m_value->child(1)->isInt(0)) {
    686                 replaceWithNewValue(m_proc.addIntConstant(m_value, 0));
    687                 break;
     671            handleCommutativity();
     672
     673            if (m_value->child(1)->hasInt()) {
     674                bool modified = true;
     675                switch (m_value->child(1)->asInt()) {
     676                case 0:
     677                    replaceWithNewValue(m_proc.addIntConstant(m_value, 0));
     678                    break;
     679                case 1:
     680                    m_value->replaceWithIdentity(m_value->child(0));
     681                    m_changed = true;
     682                    break;
     683                case 2:
     684                    m_value->as<CheckValue>()->convertToAdd();
     685                    m_value->child(1) = m_value->child(0);
     686                    m_changed = true;
     687                    break;
     688                default:
     689                    modified = false;
     690                    break;
     691                }
     692                if (modified)
     693                    break;
    688694            }
    689695            break;
    … …  
    762768    void handleCommutativity()
    763769    {
     770        // Note that we have commutative operations that take more than two children. Those operations may
     771        // commute their first two children while leaving the rest unaffected.
     772        ASSERT(m_value->numChildren() >= 2);
     773       
    764774        // Leave it alone if the right child is a constant.
    765775        if (m_value->child(1)->isConstant())

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

    r192345 r192540  
    6666void StackmapSpecial::forEachArgImpl(
    6767    unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
    68     Inst& inst, const ScopedLambda<Inst::EachArgCallback>& callback)
     68    Inst& inst, Arg::Role role, const ScopedLambda<Inst::EachArgCallback>& callback)
    6969{
    7070    Value* value = inst.origin;
    … …  
    7979        Value* child = value->child(i + numIgnoredB3Args);
    8080
    81         callback(arg, Arg::Use, Arg::typeForB3Type(child->type()));
     81        callback(arg, role, Arg::typeForB3Type(child->type()));
    8282    }
    8383}

  • trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h

    r191993 r192540  
    2929#if ENABLE(B3_JIT)
    3030
     31#include "AirArg.h"
    3132#include "AirSpecial.h"
    3233#include "B3ValueRep.h"
    … …  
    5455    void forEachArgImpl(
    5556        unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
    56         Air::Inst&, const ScopedLambda<Air::Inst::EachArgCallback>&);
     57        Air::Inst&, Air::Arg::Role role, const ScopedLambda<Air::Inst::EachArgCallback>&);
    5758    bool isValidImpl(
    5859        unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,

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

    r191994 r192540  
    4949}
    5050
     51void StackmapValue::appendSomeRegister(Value* value)
     52{
     53    append(ConstrainedValue(value, ValueRep::SomeRegister));
     54}
     55
    5156void StackmapValue::setConstrainedChild(unsigned index, const ConstrainedValue& constrainedValue)
    5257{

  • trunk/Source/JavaScriptCore/b3/B3StackmapValue.h

    r191994 r192540  
    7878    void append(const ConstrainedValue&);
    7979
     80    // This is a helper for something you might do a lot of: append a value that should be constrained
     81    // to SomeRegister.
     82    void appendSomeRegister(Value*);
     83
    8084    const Vector<ValueRep>& reps() const { return m_reps; }
    8185

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

    r192295 r192540  
    279279                VALIDATE(isInt(value->child(0)->type()), ("At ", *value));
    280280                VALIDATE(isInt(value->child(1)->type()), ("At ", *value));
     281                VALIDATE(value->as<StackmapValue>()->constrainedChild(0).rep() == ValueRep::Any, ("At ", *value));
     282                VALIDATE(value->as<StackmapValue>()->constrainedChild(1).rep() == ValueRep::Any, ("At ", *value));
    281283                validateStackmap(value);
    282284                break;

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

    r192400 r192540  
    151151
    152152Value* Value::checkMulConstant(Procedure&, const Value*) const
     153{
     154    return nullptr;
     155}
     156
     157Value* Value::checkNegConstant(Procedure&) const
    153158{
    154159    return nullptr;

  • trunk/Source/JavaScriptCore/b3/B3Value.h

    r192400 r192540  
    4242
    4343class BasicBlock;
     44class CheckValue;
    4445class Procedure;
    4546
    … …  
    119120    virtual Value* checkSubConstant(Procedure&, const Value* other) const;
    120121    virtual Value* checkMulConstant(Procedure&, const Value* other) const;
     122    virtual Value* checkNegConstant(Procedure&) const;
    121123    virtual Value* divConstant(Procedure&, const Value* other) const; // This chooses ChillDiv semantics for integers.
    122124    virtual Value* bitAndConstant(Procedure&, const Value* other) const;
    … …  
    273275
    274276private:
     277    friend class CheckValue; // CheckValue::convertToAdd() modifies m_opcode.
     278   
    275279    static Type typeFor(Opcode, Value* firstChild);
    276280

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

    r191865 r192540  
    147147            if (verbose)
    148148                dataLog("Interfering: ", pointerListDump(localCalc.live()), "\n");
    149            
    150             // Form a clique of stack slots that interfere. First find the list of stack slots
    151             // that are live right now.
    152             slots.resize(0);
    153             for (StackSlot* slot : localCalc.live()) {
    154                 if (slot->kind() == StackSlotKind::Anonymous)
    155                     slots.append(slot);
    156             }
    157 
    158             // We mustn't mandate that the input code is optimal. Therefore, it may have dead stores
    159             // to the stack. We need to treat these as interfering.
     149
    160150            inst.forEachArg(
    161151                [&] (Arg& arg, Arg::Role role, Arg::Type) {
    162                     if (Arg::isDef(role) && arg.isStack()) {
    163                         StackSlot* slot = arg.stackSlot();
    164                         if (slot->kind() == StackSlotKind::Anonymous
    165                             && !localCalc.live().contains(slot))
    166                             slots.append(slot);
     152                    if (!Arg::isDef(role))
     153                        return;
     154                    if (!arg.isStack())
     155                        return;
     156                    StackSlot* slot = arg.stackSlot();
     157                    if (slot->kind() != StackSlotKind::Anonymous)
     158                        return;
     159
     160                    for (StackSlot* otherSlot : localCalc.live()) {
     161                        interference[slot].add(otherSlot);
     162                        interference[otherSlot].add(slot);
    167163                    }
    168164                });
    169            
    170             if (verbose)
    171                 dataLog("    Slots: ", pointerListDump(slots), "\n");
    172             for (unsigned i = 0; i < slots.size(); ++i) {
    173                 StackSlot* outer = slots[i];
    174                 for (unsigned j = i + 1; j < slots.size(); ++j) {
    175                     StackSlot* inner = slots[j];
    176 
    177                     interference[inner].add(outer);
    178                     interference[outer].add(inner);
    179                 }
    180             }
    181165        };
    182166
    … …  
    186170            Inst& inst = block->at(instIndex);
    187171            interfere(inst);
    188             localCalc.execute(inst);
     172            localCalc.execute(instIndex);
    189173        }
    190174        Inst nop;

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

    r192072 r192540  
    182182        out.print("UseAddr");
    183183        return;
     184    case Arg::LateUse:
     185        out.print("LateUse");
     186        return;
    184187    }
    185188

  • trunk/Source/JavaScriptCore/b3/air/AirArg.h

    r192539 r192540  
    8484        Use,
    8585
     86        // LateUse means that the Inst will read from this value after doing its Def's. Note that LateUse
     87        // on an Addr or Index still means Use on the internal temporaries.
     88        LateUse,
     89
    8690        // Def means that the Inst will write to this value after doing everything else.
    8791        //
    … …  
    124128
    125129    // Returns true if the Role implies that the Inst will Use the Arg. It's deliberately false for
    126     // UseAddr, since isUse() for an Arg::addr means that we are loading from the address.
    127     static bool isUse(Role role)
     130    // UseAddr, since isAnyUse() for an Arg::addr means that we are loading from the address.
     131    static bool isAnyUse(Role role)
    128132    {
    129133        switch (role) {
    130134        case Use:
    131135        case UseDef:
     136        case LateUse:
    132137            return true;
    133138        case Def:
    … …  
    135140            return false;
    136141        }
     142    }
     143
     144    // Returns true if the Role implies that the Inst will Use the Arg before doing anything else.
     145    static bool isEarlyUse(Role role)
     146    {
     147        switch (role) {
     148        case Use:
     149        case UseDef:
     150            return true;
     151        case Def:
     152        case UseAddr:
     153        case LateUse:
     154            return false;
     155        }
     156    }
     157
     158    // Returns true if the Role implies that the Inst will Use the Arg after doing everything else.
     159    static bool isLateUse(Role role)
     160    {
     161        return role == LateUse;
    137162    }
    138163
    … …  
    143168        case Use:
    144169        case UseAddr:
     170        case LateUse:
    145171            return false;
    146172        case Def:
    … …  
    667693        switch (m_kind) {
    668694        case Tmp:
    669             ASSERT(isUse(argRole) || isDef(argRole));
     695            ASSERT(isAnyUse(argRole) || isDef(argRole));
    670696            functor(m_base, argRole, argType);
    671697            break;

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

    r192539 r192540  
    7373    //
    7474    // For debugging, you can use spillEverything() to put everything to the stack between each Inst.
    75     iteratedRegisterCoalescing(code);
     75    if (false)
     76        spillEverything(code);
     77    else
     78        iteratedRegisterCoalescing(code);
    7679
    7780    // Prior to this point the prologue and epilogue is implicit. This makes it explicit. It also

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

    r192516 r192540  
    3535#include "AirPhaseScope.h"
    3636#include "AirRegisterPriority.h"
     37#include <wtf/ListDump.h>
    3738#include <wtf/ListHashSet.h>
    3839
    … …  
    4142static bool debug = false;
    4243static bool traceDebug = false;
     44static bool reportStats = false;
    4345
    4446template<Arg::Type type>
    … …  
    169171                    defTmp = argTmp;
    170172                else {
    171                     ASSERT(Arg::isUse(role));
     173                    ASSERT(Arg::isEarlyUse(role));
    172174                    useTmp = argTmp;
    173175                }
    … …  
    205207
    206208        if (debug) {
     209            dataLog("Interference: ", listDump(m_interferenceEdges), "\n");
    207210            dumpInterferenceGraphInDot(WTF::dataFile());
    208211            dataLog("Initial work list\n");
    … …  
    741744        {
    742745            return WTF::IntHash<uint64_t>::hash(m_value);
     746        }
     747
     748        void dump(PrintStream& out) const
     749        {
     750            out.print(first(), "<=>", second());
    743751        }
    744752
    … …  
    952960                Opcode move = type == Arg::GP ? Move : MoveDouble;
    953961
    954                 if (Arg::isUse(role)) {
     962                if (Arg::isAnyUse(role)) {
    955963                    Tmp newTmp = code.newTmp(type);
    956964                    insertionSet.insert(instIndex, move, inst.origin, arg, newTmp);
    … …  
    969977
    970978template<Arg::Type type>
    971 static void iteratedRegisterCoalescingOnType(Code& code, HashSet<Tmp>& unspillableTmps)
     979static void iteratedRegisterCoalescingOnType(Code& code, HashSet<Tmp>& unspillableTmps, unsigned& numIterations)
    972980{
    973981    while (true) {
     982        numIterations++;
    974983        IteratedRegisterCoalescingAllocator<type> allocator(code, unspillableTmps);
    975984        Liveness<Tmp> liveness(code);
    … …  
    979988                Inst& inst = block->at(instIndex);
    980989                allocator.build(inst, localCalc);
    981                 localCalc.execute(inst);
     990                localCalc.execute(instIndex);
    982991            }
    983992        }
    … …  
    9981007    bool gpIsColored = false;
    9991008    bool fpIsColored = false;
     1009    unsigned numIterations = 0;
    10001010
    10011011    HashSet<Tmp> unspillableGPs;
    … …  
    10041014    // First we run both allocator together as long as they both spill.
    10051015    while (!gpIsColored && !fpIsColored) {
     1016        numIterations++;
    10061017        IteratedRegisterCoalescingAllocator<Arg::GP> gpAllocator(code, unspillableGPs);
    10071018        IteratedRegisterCoalescingAllocator<Arg::FP> fpAllocator(code, unspillableFPs);
    … …  
    10181029                fpAllocator.build(inst, localCalc);
    10191030
    1020                 localCalc.execute(inst);
     1031                localCalc.execute(instIndex);
    10211032            }
    10221033        }
    … …  
    10391050
    10401051    if (!gpIsColored)
    1041         iteratedRegisterCoalescingOnType<Arg::GP>(code, unspillableGPs);
     1052        iteratedRegisterCoalescingOnType<Arg::GP>(code, unspillableGPs, numIterations);
    10421053    if (!fpIsColored)
    1043         iteratedRegisterCoalescingOnType<Arg::FP>(code, unspillableFPs);
     1054        iteratedRegisterCoalescingOnType<Arg::FP>(code, unspillableFPs, numIterations);
     1055
     1056    if (reportStats)
     1057        dataLog("Num iterations = ", numIterations, "\n");
    10441058}
    10451059

  • trunk/Source/JavaScriptCore/b3/air/AirLiveness.h

    r192121 r192540  
    5050        m_liveAtTail.resize(code.size());
    5151
     52        // The liveAtTail of each block automatically contains the LateUse's of the terminal.
     53        for (BasicBlock* block : code) {
     54            HashSet<Thing>& live = m_liveAtTail[block];
     55            block->last().forEach<Thing>(
     56                [&] (Thing& thing, Arg::Role role, Arg::Type) {
     57                    if (Arg::isLateUse(role))
     58                        live.add(thing);
     59                });
     60        }
     61
    5262        IndexSet<BasicBlock> seen;
    5363
    … …  
    6272                LocalCalc localCalc(*this, block);
    6373                for (size_t instIndex = block->size(); instIndex--;)
    64                     localCalc.execute(block->at(instIndex));
     74                    localCalc.execute(instIndex);
    6575                bool firstTime = seen.add(block);
    6676                if (!firstTime && localCalc.live() == m_liveAtHead[block])
    … …  
    91101        LocalCalc(Liveness& liveness, BasicBlock* block)
    92102            : m_live(liveness.liveAtTail(block))
     103            , m_block(block)
    93104        {
    94105        }
    … …  
    97108        HashSet<Thing>&& takeLive() { return WTF::move(m_live); }
    98109
    99         void execute(Inst& inst)
     110        void execute(unsigned instIndex)
    100111        {
     112            Inst& inst = m_block->at(instIndex);
     113           
    101114            // First handle def's.
    102115            inst.forEach<Thing>(
    … …  
    108121                });
    109122
    110             // Finally handle use's.
     123            // Then handle use's.
    111124            inst.forEach<Thing>(
    112125                [this] (Thing& arg, Arg::Role role, Arg::Type) {
    113126                    if (!isAlive(arg))
    114127                        return;
    115                     if (Arg::isUse(role))
     128                    if (Arg::isEarlyUse(role))
    116129                        m_live.add(arg);
    117130                });
     131
     132            // And finally, handle the late use's of the previous instruction.
     133            if (instIndex) {
     134                Inst& prevInst = m_block->at(instIndex - 1);
     135
     136                prevInst.forEach<Thing>(
     137                    [this] (Thing& arg, Arg::Role role, Arg::Type) {
     138                        if (!Arg::isLateUse(role))
     139                            return;
     140                        if (isAlive(arg))
     141                            m_live.add(arg);
     142                    });
     143            }
    118144        }
    119145
    120146    private:
    121147        HashSet<Thing> m_live;
     148        BasicBlock* m_block;
    122149    };
    123    
     150
    124151private:
    125152    IndexMap<BasicBlock, HashSet<Thing>> m_liveAtHead;

  • trunk/Source/JavaScriptCore/b3/air/AirOpcode.opcodes

    r192539 r192540  
    332332    ResCond, Addr, Tmp
    333333
    334 BranchMul32 U:G, U:G, U:G, D:G /branch
    335     ResCond, Tmp, Tmp, Tmp
    336 
    337334BranchMul64 U:G, U:G, UD:G /branch
    338335    ResCond, Tmp, Tmp
    339 
    340 BranchMul64 U:G, U:G, U:G, D:G /branch
    341     ResCond, Tmp, Tmp, Tmp
    342336
    343337BranchSub32 U:G, U:G, UD:G /branch

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

    r191845 r192540  
    5757                inst.reportUsedRegisters(registerSet);
    5858            }
    59             localCalc.execute(inst);
     59            localCalc.execute(instIndex);
    6060        }
    6161    }

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

    r192497 r192540  
    7070            Inst& inst = block->at(instIndex);
    7171            setUsedRegisters(instIndex + 1, inst);
    72             localCalc.execute(inst);
     72            localCalc.execute(instIndex);
    7373        }
    7474
    … …  
    141141                        break;
    142142                    case Arg::UseDef:
     143                    case Arg::LateUse:
    143144                        for (Reg reg : regsInPriorityOrder(type)) {
    144145                            if (!setBefore.get(reg) && !setAfter.get(reg)) {
    … …  
    161162                    Opcode move = type == Arg::GP ? Move : MoveDouble;
    162163
    163                     if (Arg::isUse(role)) {
     164                    if (Arg::isAnyUse(role)) {
    164165                        insertionSet.insert(
    165166                            instIndex, move, inst.origin, arg, tmp);

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

    r192524 r192540  
    298298    Procedure proc;
    299299    BasicBlock* root = proc.addBlock();
    300     Value* value = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
     300    Value* value = root->appendNew<Value>(
     301        proc, Trunc, Origin(), root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
    301302    root->appendNew<ControlValue>(
    302303        proc, Return, Origin(),
    … …  
    304305
    305306    CHECK(compileAndRun<int>(proc, a) == a * a);
     307}
     308
     309void testMulArgStore(int a)
     310{
     311    Procedure proc;
     312    BasicBlock* root = proc.addBlock();
     313
     314    int mulSlot;
     315    int valueSlot;
     316   
     317    Value* value = root->appendNew<Value>(
     318        proc, Trunc, Origin(),
     319        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     320    Value* mul = root->appendNew<Value>(proc, Mul, Origin(), value, value);
     321
     322    root->appendNew<MemoryValue>(
     323        proc, Store, Origin(), value,
     324        root->appendNew<ConstPtrValue>(proc, Origin(), &valueSlot));
     325    root->appendNew<MemoryValue>(
     326        proc, Store, Origin(), mul,
     327        root->appendNew<ConstPtrValue>(proc, Origin(), &mulSlot));
     328
     329    root->appendNew<ControlValue>(
     330        proc, Return, Origin(), root->appendNew<Const32Value>(proc, Origin(), 0));
     331
     332    CHECK(!compileAndRun<int>(proc, a));
     333    CHECK(mulSlot == a * a);
     334    CHECK(valueSlot == a);
     335}
     336
     337void testMulAddArg(int a)
     338{
     339    Procedure proc;
     340    BasicBlock* root = proc.addBlock();
     341    Value* value = root->appendNew<Value>(
     342        proc, Trunc, Origin(),
     343        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     344    root->appendNew<ControlValue>(
     345        proc, Return, Origin(),
     346        root->appendNew<Value>(
     347            proc, Add, Origin(),
     348            root->appendNew<Value>(proc, Mul, Origin(), value, value),
     349            value));
     350
     351    CHECK(compileAndRun<int>(proc, a) == a * a + a);
    306352}
    307353
    … …  
    27552801        if (i & 1) {
    27562802            // Control flow diamond.
    2757             unsigned predicateVarIndex = (i >> 1) % numVars;
    2758             unsigned thenIncVarIndex = ((i >> 1) + 1) % numVars;
    2759             unsigned elseIncVarIndex = ((i >> 1) + 2) % numVars;
     2803            unsigned predicateVarIndex = ((i >> 1) + 2) % numVars;
     2804            unsigned thenIncVarIndex = ((i >> 1) + 0) % numVars;
     2805            unsigned elseIncVarIndex = ((i >> 1) + 1) % numVars;
    27602806
    27612807            BasicBlock* thenBlock = proc.addBlock();
    … …  
    28022848            BasicBlock* continuation = proc.addBlock();
    28032849           
    2804             Value* startIndex = vars[(i >> 1) % numVars];
     2850            Value* startIndex = vars[((i >> 1) + 1) % numVars];
    28052851            Value* startSum = current->appendNew<Const32Value>(proc, Origin(), 0);
    28062852            current->appendNew<ControlValue>(
    … …  
    28562902
    28572903            current = continuation;
    2858             vars[((i >> 1) + 1) % numVars] = finalSum;
     2904            vars[((i >> 1) + 0) % numVars] = finalSum;
    28592905        }
    28602906    }
    … …  
    31843230        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    31853231            CHECK(params.reps.size() == 1);
    3186             CHECK(params.reps[0].isConstant());
    3187             CHECK(params.reps[0].value() == 1);
    31883232
    31893233            // This should always work because a function this simple should never have callee
    … …  
    32173261        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    32183262            CHECK(params.reps.size() == 1);
    3219             CHECK(params.reps[0].isConstant());
    3220             CHECK(params.reps[0].value() == 1);
    32213263
    32223264            // This should always work because a function this simple should never have callee
    … …  
    32643306        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    32653307            CHECK(params.reps.size() == 1);
    3266             CHECK(params.reps[0].isConstant());
    3267             CHECK(params.reps[0].value() == 1);
    32683308
    32693309            // This should always work because a function this simple should never have callee
    … …  
    33003340    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 42);
    33013341    CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), arg1, arg2);
     3342    checkAdd->append(arg1);
     3343    checkAdd->append(arg2);
    33023344    checkAdd->setGenerator(
    33033345        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3304             CHECK(params.reps.size() == 2);
    3305             CHECK(params.reps[0].isGPR());
    3306             CHECK(params.reps[1].isConstant());
    3307             CHECK(params.reps[1].value() == 42);
    3308             jit.sub32(CCallHelpers::TrustedImm32(42), params.reps[0].gpr());
    3309             jit.convertInt32ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
     3346            CHECK(params.reps.size() == 4);
     3347            CHECK(params.reps[2].isGPR());
     3348            CHECK(params.reps[3].isConstant());
     3349            CHECK(params.reps[3].value() == 42);
     3350            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
    33103351            jit.convertInt32ToDouble(CCallHelpers::TrustedImm32(42), FPRInfo::fpRegT1);
     3352            jit.addDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
     3353            jit.emitFunctionEpilogue();
     3354            jit.ret();
     3355        });
     3356    root->appendNew<ControlValue>(
     3357        proc, Return, Origin(),
     3358        root->appendNew<Value>(proc, IToD, Origin(), checkAdd));
     3359
     3360    auto code = compile(proc);
     3361
     3362    CHECK(invoke<double>(*code, 0) == 42.0);
     3363    CHECK(invoke<double>(*code, 1) == 43.0);
     3364    CHECK(invoke<double>(*code, 42) == 84.0);
     3365    CHECK(invoke<double>(*code, 2147483647) == 2147483689.0);
     3366}
     3367
     3368void testCheckAddImmCommute()
     3369{
     3370    Procedure proc;
     3371    BasicBlock* root = proc.addBlock();
     3372    Value* arg1 = root->appendNew<Value>(
     3373        proc, Trunc, Origin(),
     3374        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     3375    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 42);
     3376    CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), arg2, arg1);
     3377    checkAdd->append(arg1);
     3378    checkAdd->append(arg2);
     3379    checkAdd->setGenerator(
     3380        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
     3381            CHECK(params.reps.size() == 4);
     3382            CHECK(params.reps[2].isGPR());
     3383            CHECK(params.reps[3].isConstant());
     3384            CHECK(params.reps[3].value() == 42);
     3385            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3386            jit.convertInt32ToDouble(CCallHelpers::TrustedImm32(42), FPRInfo::fpRegT1);
     3387            jit.addDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
     3388            jit.emitFunctionEpilogue();
     3389            jit.ret();
     3390        });
     3391    root->appendNew<ControlValue>(
     3392        proc, Return, Origin(),
     3393        root->appendNew<Value>(proc, IToD, Origin(), checkAdd));
     3394
     3395    auto code = compile(proc);
     3396
     3397    CHECK(invoke<double>(*code, 0) == 42.0);
     3398    CHECK(invoke<double>(*code, 1) == 43.0);
     3399    CHECK(invoke<double>(*code, 42) == 84.0);
     3400    CHECK(invoke<double>(*code, 2147483647) == 2147483689.0);
     3401}
     3402
     3403void testCheckAddImmSomeRegister()
     3404{
     3405    Procedure proc;
     3406    BasicBlock* root = proc.addBlock();
     3407    Value* arg1 = root->appendNew<Value>(
     3408        proc, Trunc, Origin(),
     3409        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     3410    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 42);
     3411    CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), arg1, arg2);
     3412    checkAdd->appendSomeRegister(arg1);
     3413    checkAdd->appendSomeRegister(arg2);
     3414    checkAdd->setGenerator(
     3415        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
     3416            CHECK(params.reps.size() == 4);
     3417            CHECK(params.reps[2].isGPR());
     3418            CHECK(params.reps[3].isGPR());
     3419            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3420            jit.convertInt32ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    33113421            jit.addDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    33123422            jit.emitFunctionEpilogue();
    … …  
    33363446        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
    33373447    CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), arg1, arg2);
     3448    checkAdd->appendSomeRegister(arg1);
     3449    checkAdd->appendSomeRegister(arg2);
    33383450    checkAdd->setGenerator(
    33393451        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3340             CHECK(params.reps.size() == 2);
    3341             CHECK(params.reps[0].isGPR());
    3342             CHECK(params.reps[1].isGPR());
    3343             jit.sub32(params.reps[1].gpr(), params.reps[0].gpr());
    3344             jit.convertInt32ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3345             jit.convertInt32ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3452            CHECK(params.reps.size() == 4);
     3453            CHECK(params.reps[2].isGPR());
     3454            CHECK(params.reps[3].isGPR());
     3455            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3456            jit.convertInt32ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    33463457            jit.addDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    33473458            jit.emitFunctionEpilogue();
    … …  
    33673478    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
    33683479    CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), arg1, arg2);
     3480    checkAdd->appendSomeRegister(arg1);
     3481    checkAdd->appendSomeRegister(arg2);
    33693482    checkAdd->setGenerator(
    33703483        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3371             CHECK(params.reps.size() == 2);
    3372             CHECK(params.reps[0].isGPR());
    3373             CHECK(params.reps[1].isGPR());
    3374             jit.sub64(params.reps[1].gpr(), params.reps[0].gpr());
    3375             jit.convertInt64ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3376             jit.convertInt64ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3484            CHECK(params.reps.size() == 4);
     3485            CHECK(params.reps[2].isGPR());
     3486            CHECK(params.reps[3].isGPR());
     3487            jit.convertInt64ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3488            jit.convertInt64ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    33773489            jit.addDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    33783490            jit.emitFunctionEpilogue();
    … …  
    34383550    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 42);
    34393551    CheckValue* checkSub = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3552    checkSub->append(arg1);
     3553    checkSub->append(arg2);
    34403554    checkSub->setGenerator(
    34413555        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3442             CHECK(params.reps.size() == 2);
    3443             CHECK(params.reps[0].isGPR());
    3444             CHECK(params.reps[1].isConstant());
    3445             CHECK(params.reps[1].value() == 42);
    3446             jit.add32(CCallHelpers::TrustedImm32(42), params.reps[0].gpr());
    3447             jit.convertInt32ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
     3556            CHECK(params.reps.size() == 4);
     3557            CHECK(params.reps[2].isGPR());
     3558            CHECK(params.reps[3].isConstant());
     3559            CHECK(params.reps[3].value() == 42);
     3560            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
    34483561            jit.convertInt32ToDouble(CCallHelpers::TrustedImm32(42), FPRInfo::fpRegT1);
    34493562            jit.subDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    … …  
    34613574    CHECK(invoke<double>(*code, 42) == 0.0);
    34623575    CHECK(invoke<double>(*code, -2147483647) == -2147483689.0);
     3576}
     3577
     3578void testCheckSubBadImm()
     3579{
     3580    Procedure proc;
     3581    BasicBlock* root = proc.addBlock();
     3582    Value* arg1 = root->appendNew<Value>(
     3583        proc, Trunc, Origin(),
     3584        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     3585    int32_t badImm = std::numeric_limits<int>::min();
     3586    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), badImm);
     3587    CheckValue* checkSub = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3588    checkSub->append(arg1);
     3589    checkSub->append(arg2);
     3590    checkSub->setGenerator(
     3591        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
     3592            CHECK(params.reps.size() == 4);
     3593            CHECK(params.reps[2].isGPR());
     3594            CHECK(params.reps[3].isConstant());
     3595            CHECK(params.reps[3].value() == badImm);
     3596            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3597            jit.convertInt32ToDouble(CCallHelpers::TrustedImm32(badImm), FPRInfo::fpRegT1);
     3598            jit.subDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
     3599            jit.emitFunctionEpilogue();
     3600            jit.ret();
     3601        });
     3602    root->appendNew<ControlValue>(
     3603        proc, Return, Origin(),
     3604        root->appendNew<Value>(proc, IToD, Origin(), checkSub));
     3605
     3606    auto code = compile(proc);
     3607
     3608    CHECK(invoke<double>(*code, 0) == -static_cast<double>(badImm));
     3609    CHECK(invoke<double>(*code, -1) == -static_cast<double>(badImm) - 1);
     3610    CHECK(invoke<double>(*code, 1) == -static_cast<double>(badImm) + 1);
     3611    CHECK(invoke<double>(*code, 42) == -static_cast<double>(badImm) + 42);
    34633612}
    34643613
    … …  
    34743623        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
    34753624    CheckValue* checkSub = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3625    checkSub->append(arg1);
     3626    checkSub->append(arg2);
    34763627    checkSub->setGenerator(
    34773628        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3478             CHECK(params.reps.size() == 2);
    3479             CHECK(params.reps[0].isGPR());
    3480             CHECK(params.reps[1].isGPR());
    3481             jit.add32(params.reps[1].gpr(), params.reps[0].gpr());
    3482             jit.convertInt32ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3483             jit.convertInt32ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3629            CHECK(params.reps.size() == 4);
     3630            CHECK(params.reps[2].isGPR());
     3631            CHECK(params.reps[3].isGPR());
     3632            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3633            jit.convertInt32ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    34843634            jit.subDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    34853635            jit.emitFunctionEpilogue();
    … …  
    35103660    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
    35113661    CheckValue* checkSub = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3662    checkSub->append(arg1);
     3663    checkSub->append(arg2);
    35123664    checkSub->setGenerator(
    35133665        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3514             CHECK(params.reps.size() == 2);
    3515             CHECK(params.reps[0].isGPR());
    3516             CHECK(params.reps[1].isGPR());
    3517             jit.add64(params.reps[1].gpr(), params.reps[0].gpr());
    3518             jit.convertInt64ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3519             jit.convertInt64ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3666            CHECK(params.reps.size() == 4);
     3667            CHECK(params.reps[2].isGPR());
     3668            CHECK(params.reps[3].isGPR());
     3669            jit.convertInt64ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3670            jit.convertInt64ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    35203671            jit.subDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    35213672            jit.emitFunctionEpilogue();
    … …  
    35813732        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
    35823733    CheckValue* checkNeg = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3734    checkNeg->append(arg2);
    35833735    checkNeg->setGenerator(
    35843736        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3585             CHECK(params.reps.size() == 2);
    3586             CHECK(params.reps[0] == ValueRep::constant(0));
    3587             CHECK(params.reps[1].isGPR());
    3588             jit.neg32(params.reps[1].gpr());
    3589             jit.convertInt32ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3737            CHECK(params.reps.size() == 3);
     3738            CHECK(params.reps[2].isGPR());
     3739            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT1);
    35903740            jit.negateDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    35913741            jit.emitFunctionEpilogue();
    … …  
    36113761    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
    36123762    CheckValue* checkNeg = root->appendNew<CheckValue>(proc, CheckSub, Origin(), arg1, arg2);
     3763    checkNeg->append(arg2);
    36133764    checkNeg->setGenerator(
    36143765        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3615             CHECK(params.reps.size() == 2);
    3616             CHECK(params.reps[0] == ValueRep::constant(0));
    3617             CHECK(params.reps[1].isGPR());
    3618             jit.neg64(params.reps[1].gpr());
    3619             jit.convertInt64ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3766            CHECK(params.reps.size() == 3);
     3767            CHECK(params.reps[2].isGPR());
     3768            jit.convertInt64ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT1);
    36203769            jit.negateDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    36213770            jit.emitFunctionEpilogue();
    … …  
    36453794        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
    36463795    CheckValue* checkMul = root->appendNew<CheckValue>(proc, CheckMul, Origin(), arg1, arg2);
     3796    checkMul->append(arg1);
     3797    checkMul->append(arg2);
    36473798    checkMul->setGenerator(
    36483799        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3649             CHECK(params.reps.size() == 2);
    3650             CHECK(params.reps[0].isGPR());
    3651             CHECK(params.reps[1].isGPR());
    3652             jit.convertInt32ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3653             jit.convertInt32ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3800            CHECK(params.reps.size() == 4);
     3801            CHECK(params.reps[2].isGPR());
     3802            CHECK(params.reps[3].isGPR());
     3803            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3804            jit.convertInt32ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    36543805            jit.mulDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    36553806            jit.emitFunctionEpilogue();
    … …  
    36683819}
    36693820
     3821void testCheckMulMemory()
     3822{
     3823    Procedure proc;
     3824    BasicBlock* root = proc.addBlock();
     3825
     3826    int left;
     3827    int right;
     3828   
     3829    Value* arg1 = root->appendNew<MemoryValue>(
     3830        proc, Load, Int32, Origin(),
     3831        root->appendNew<ConstPtrValue>(proc, Origin(), &left));
     3832    Value* arg2 = root->appendNew<MemoryValue>(
     3833        proc, Load, Int32, Origin(),
     3834        root->appendNew<ConstPtrValue>(proc, Origin(), &right));
     3835    CheckValue* checkMul = root->appendNew<CheckValue>(proc, CheckMul, Origin(), arg1, arg2);
     3836    checkMul->append(arg1);
     3837    checkMul->append(arg2);
     3838    checkMul->setGenerator(
     3839        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
     3840            CHECK(params.reps.size() == 4);
     3841            CHECK(params.reps[2].isGPR());
     3842            CHECK(params.reps[3].isGPR());
     3843            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3844            jit.convertInt32ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
     3845            jit.mulDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
     3846            jit.emitFunctionEpilogue();
     3847            jit.ret();
     3848        });
     3849    root->appendNew<ControlValue>(
     3850        proc, Return, Origin(),
     3851        root->appendNew<Value>(proc, IToD, Origin(), checkMul));
     3852
     3853    auto code = compile(proc);
     3854
     3855    left = 0;
     3856    right = 42;
     3857    CHECK(invoke<double>(*code) == 0.0);
     3858   
     3859    left = 1;
     3860    right = 42;
     3861    CHECK(invoke<double>(*code) == 42.0);
     3862
     3863    left = 42;
     3864    right = 42;
     3865    CHECK(invoke<double>(*code) == 42.0 * 42.0);
     3866
     3867    left = 2147483647;
     3868    right = 42;
     3869    CHECK(invoke<double>(*code) == 2147483647.0 * 42.0);
     3870}
     3871
     3872void testCheckMul2()
     3873{
     3874    Procedure proc;
     3875    BasicBlock* root = proc.addBlock();
     3876    Value* arg1 = root->appendNew<Value>(
     3877        proc, Trunc, Origin(),
     3878        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     3879    Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 2);
     3880    CheckValue* checkMul = root->appendNew<CheckValue>(proc, CheckMul, Origin(), arg1, arg2);
     3881    checkMul->append(arg1);
     3882    checkMul->append(arg2);
     3883    checkMul->setGenerator(
     3884        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
     3885            CHECK(params.reps.size() == 4);
     3886            CHECK(params.reps[2].isGPR());
     3887            CHECK(params.reps[3].isConstant());
     3888            CHECK(params.reps[3].value() == 2);
     3889            jit.convertInt32ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3890            jit.convertInt32ToDouble(CCallHelpers::TrustedImm32(2), FPRInfo::fpRegT1);
     3891            jit.mulDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
     3892            jit.emitFunctionEpilogue();
     3893            jit.ret();
     3894        });
     3895    root->appendNew<ControlValue>(
     3896        proc, Return, Origin(),
     3897        root->appendNew<Value>(proc, IToD, Origin(), checkMul));
     3898
     3899    auto code = compile(proc);
     3900
     3901    CHECK(invoke<double>(*code, 0) == 0.0);
     3902    CHECK(invoke<double>(*code, 1) == 2.0);
     3903    CHECK(invoke<double>(*code, 42) == 42.0 * 2.0);
     3904    CHECK(invoke<double>(*code, 2147483647) == 2147483647.0 * 2.0);
     3905}
     3906
    36703907void testCheckMul64()
    36713908{
    … …  
    36753912    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
    36763913    CheckValue* checkMul = root->appendNew<CheckValue>(proc, CheckMul, Origin(), arg1, arg2);
     3914    checkMul->append(arg1);
     3915    checkMul->append(arg2);
    36773916    checkMul->setGenerator(
    36783917        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
    3679             CHECK(params.reps.size() == 2);
    3680             CHECK(params.reps[0].isGPR());
    3681             CHECK(params.reps[1].isGPR());
    3682             jit.convertInt64ToDouble(params.reps[0].gpr(), FPRInfo::fpRegT0);
    3683             jit.convertInt64ToDouble(params.reps[1].gpr(), FPRInfo::fpRegT1);
     3918            CHECK(params.reps.size() == 4);
     3919            CHECK(params.reps[2].isGPR());
     3920            CHECK(params.reps[3].isGPR());
     3921            jit.convertInt64ToDouble(params.reps[2].gpr(), FPRInfo::fpRegT0);
     3922            jit.convertInt64ToDouble(params.reps[3].gpr(), FPRInfo::fpRegT1);
    36843923            jit.mulDouble(FPRInfo::fpRegT1, FPRInfo::fpRegT0);
    36853924            jit.emitFunctionEpilogue();
    … …  
    44264665
    44274666    RUN(testMulArg(5));
     4667    RUN(testMulAddArg(5));
     4668    RUN(testMulAddArg(85));
     4669    RUN(testMulArgStore(5));
     4670    RUN(testMulArgStore(85));
    44284671    RUN(testMulArgs(1, 1));
    44294672    RUN(testMulArgs(1, 2));
    … …  
    48815124    RUN(testCheckMegaCombo());
    48825125    RUN(testCheckAddImm());
     5126    RUN(testCheckAddImmCommute());
     5127    RUN(testCheckAddImmSomeRegister());
    48835128    RUN(testCheckAdd());
    48845129    RUN(testCheckAdd64());
    … …  
    48865131    RUN(testCheckAddFoldFail(2147483647, 100));
    48875132    RUN(testCheckSubImm());
     5133    RUN(testCheckSubBadImm());
    48885134    RUN(testCheckSub());
    48895135    RUN(testCheckSub64());
    … …  
    48935139    RUN(testCheckNeg64());
    48945140    RUN(testCheckMul());
     5141    RUN(testCheckMulMemory());
     5142    RUN(testCheckMul2());
    48955143    RUN(testCheckMul64());
    48965144    RUN(testCheckMulFold(100, 200));

  • trunk/Source/WTF/ChangeLog

    r192539 r192540  
     12015-11-17  Filip Pizlo  <fpizlo@apple.com>
     2
     3        CheckAdd/Mul should have commutativity optimizations in B3->Air lowering
     4        https://bugs.webkit.org/show_bug.cgi?id=151214
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Disable my failed attempts at perfect forwarding, since they were incorrect, and caused compile
     9        errors if you tried to pass an argument that bound to lvalue. This shouldn't affect performance of
     10        anything we care about, and performance tests seem to confirm that it's all good.
     11
     12        * wtf/ScopedLambda.h:
     13
    1142015-11-17  Filip Pizlo  <fpizlo@apple.com>
    215

  • trunk/Source/WTF/wtf/ScopedLambda.h

    r191705 r192540  
    4545class ScopedLambda<ResultType (ArgumentTypes...)> {
    4646public:
    47     ScopedLambda(ResultType (*impl)(void* arg, ArgumentTypes&&...) = nullptr, void* arg = nullptr)
     47    ScopedLambda(ResultType (*impl)(void* arg, ArgumentTypes...) = nullptr, void* arg = nullptr)
    4848        : m_impl(impl)
    4949        , m_arg(arg)
    5050    {
    5151    }
    52    
    53     ResultType operator()(ArgumentTypes&&... arguments) const
     52
     53    template<typename... PassedArgumentTypes>
     54    ResultType operator()(PassedArgumentTypes&&... arguments) const
    5455    {
    55         return m_impl(m_arg, std::forward<ArgumentTypes>(arguments)...);
     56        return m_impl(m_arg, std::forward<PassedArgumentTypes>(arguments)...);
    5657    }
    5758
    5859private:
    59     ResultType (*m_impl)(void* arg, ArgumentTypes&&...);
     60    ResultType (*m_impl)(void* arg, ArgumentTypes...);
    6061    void *m_arg;
    6162};
    … …  
    7374
    7475private:
    75     static ResultType implFunction(void* argument, ArgumentTypes&&... arguments)
     76    static ResultType implFunction(void* argument, ArgumentTypes... arguments)
    7677    {
    77         return static_cast<ScopedLambdaFunctor*>(argument)->m_functor(
    78             std::forward<ArgumentTypes>(arguments)...);
     78        return static_cast<ScopedLambdaFunctor*>(argument)->m_functor(arguments...);
    7979    }
    8080

  • trunk/Tools/ReducedFTL/ComplexTest.cpp

    r191865 r192540  
    103103        if (i & 1) {
    104104            // Control flow diamond.
    105             unsigned predicateVarIndex = (i >> 1) % numVars;
    106             unsigned thenIncVarIndex = ((i >> 1) + 1) % numVars;
    107             unsigned elseIncVarIndex = ((i >> 1) + 2) % numVars;
     105            unsigned predicateVarIndex = ((i >> 1) + 2) % numVars;
     106            unsigned thenIncVarIndex = ((i >> 1) + 0) % numVars;
     107            unsigned elseIncVarIndex = ((i >> 1) + 1) % numVars;
    108108
    109109            BasicBlock* thenBlock = BasicBlock::Create(context, "", function);
    … …  
    139139            BasicBlock* continuation = BasicBlock::Create(context, "", function);
    140140
    141             Value* startIndex = vars[(i >> 1) % numVars];
     141            Value* startIndex = vars[((i >> 1) + 1) % numVars];
    142142            Value* startSum = builder->getInt32(0);
    143143            builder->CreateCondBr(
    … …  
    177177            finalSum->addIncoming(newBodySum, loopBody);
    178178            current = continuation;
     179            vars[((i >> 1) + 0) % numVars] = finalSum;
    179180        }
    180181    }
Note: See TracChangeset for help on using the changeset viewer.