B3 patchpoints should allow specifying output constraints
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Dec 2015 00:11:32 +0000 (00:11 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Dec 2015 00:11:32 +0000 (00:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151809

Reviewed by Benjamin Poulain.

JS call patchpoints should put their result into the result register, while most other patchpoints
should put their results into some register. I think that it's best if we just allow arbitrary
constraints on the result of a patchpoint. And by "arbitrary" I mean allowing the same kinds of
constraints as we allow on the stackmap children.

This also adds a large comment in B3StackmapValue.h that lays out the philosophy of our stackmaps
and patchpoints. I found it useful to write down the plan since it's pretty subtle.

* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::lower):
* b3/B3PatchpointSpecial.cpp:
(JSC::B3::PatchpointSpecial::isValid):
(JSC::B3::PatchpointSpecial::admitsStack):
* b3/B3PatchpointValue.cpp:
(JSC::B3::PatchpointValue::~PatchpointValue):
(JSC::B3::PatchpointValue::dumpMeta):
(JSC::B3::PatchpointValue::PatchpointValue):
* b3/B3PatchpointValue.h:
(JSC::B3::PatchpointValue::accepts):
* b3/B3Procedure.h:
(JSC::B3::Procedure::code):
* b3/B3StackmapSpecial.cpp:
(JSC::B3::StackmapSpecial::isValidImpl):
(JSC::B3::StackmapSpecial::appendRepsImpl):
(JSC::B3::StackmapSpecial::isArgValidForValue):
(JSC::B3::StackmapSpecial::isArgValidForRep):
(JSC::B3::StackmapSpecial::repForArg):
* b3/B3StackmapSpecial.h:
* b3/B3StackmapValue.h:
* b3/B3Validate.cpp:
* b3/B3ValueRep.h:
(JSC::B3::ValueRep::doubleValue):
* b3/testb3.cpp:
(JSC::B3::testPatchpointManyImms):
(JSC::B3::testPatchpointWithRegisterResult):
(JSC::B3::testPatchpointWithStackArgumentResult):
(JSC::B3::testPatchpointWithAnyResult):
(JSC::B3::testSimpleCheck):
(JSC::B3::run):
* jit/RegisterSet.h:

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

13 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3LowerToAir.cpp
Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp
Source/JavaScriptCore/b3/B3PatchpointValue.cpp
Source/JavaScriptCore/b3/B3PatchpointValue.h
Source/JavaScriptCore/b3/B3Procedure.h
Source/JavaScriptCore/b3/B3StackmapSpecial.cpp
Source/JavaScriptCore/b3/B3StackmapSpecial.h
Source/JavaScriptCore/b3/B3StackmapValue.h
Source/JavaScriptCore/b3/B3Validate.cpp
Source/JavaScriptCore/b3/B3ValueRep.h
Source/JavaScriptCore/b3/testb3.cpp
Source/JavaScriptCore/jit/RegisterSet.h

index ddab084..8d090b0 100644 (file)
@@ -1,3 +1,51 @@
+2015-12-03  Filip Pizlo  <fpizlo@apple.com>
+
+        B3 patchpoints should allow specifying output constraints
+        https://bugs.webkit.org/show_bug.cgi?id=151809
+
+        Reviewed by Benjamin Poulain.
+
+        JS call patchpoints should put their result into the result register, while most other patchpoints
+        should put their results into some register. I think that it's best if we just allow arbitrary
+        constraints on the result of a patchpoint. And by "arbitrary" I mean allowing the same kinds of
+        constraints as we allow on the stackmap children.
+
+        This also adds a large comment in B3StackmapValue.h that lays out the philosophy of our stackmaps
+        and patchpoints. I found it useful to write down the plan since it's pretty subtle.
+
+        * b3/B3LowerToAir.cpp:
+        (JSC::B3::Air::LowerToAir::lower):
+        * b3/B3PatchpointSpecial.cpp:
+        (JSC::B3::PatchpointSpecial::isValid):
+        (JSC::B3::PatchpointSpecial::admitsStack):
+        * b3/B3PatchpointValue.cpp:
+        (JSC::B3::PatchpointValue::~PatchpointValue):
+        (JSC::B3::PatchpointValue::dumpMeta):
+        (JSC::B3::PatchpointValue::PatchpointValue):
+        * b3/B3PatchpointValue.h:
+        (JSC::B3::PatchpointValue::accepts):
+        * b3/B3Procedure.h:
+        (JSC::B3::Procedure::code):
+        * b3/B3StackmapSpecial.cpp:
+        (JSC::B3::StackmapSpecial::isValidImpl):
+        (JSC::B3::StackmapSpecial::appendRepsImpl):
+        (JSC::B3::StackmapSpecial::isArgValidForValue):
+        (JSC::B3::StackmapSpecial::isArgValidForRep):
+        (JSC::B3::StackmapSpecial::repForArg):
+        * b3/B3StackmapSpecial.h:
+        * b3/B3StackmapValue.h:
+        * b3/B3Validate.cpp:
+        * b3/B3ValueRep.h:
+        (JSC::B3::ValueRep::doubleValue):
+        * b3/testb3.cpp:
+        (JSC::B3::testPatchpointManyImms):
+        (JSC::B3::testPatchpointWithRegisterResult):
+        (JSC::B3::testPatchpointWithStackArgumentResult):
+        (JSC::B3::testPatchpointWithAnyResult):
+        (JSC::B3::testSimpleCheck):
+        (JSC::B3::run):
+        * jit/RegisterSet.h:
+
 2015-12-03  Anders Carlsson  <andersca@apple.com>
 
         Remove Objective-C GC support
index 1e98b90..703a116 100644 (file)
@@ -1696,13 +1696,38 @@ private:
             ensureSpecial(m_patchpointSpecial);
             
             Inst inst(Patch, patchpointValue, Arg::special(m_patchpointSpecial));
-            
-            if (patchpointValue->type() != Void)
-                inst.args.append(tmp(patchpointValue));
+
+            Vector<Inst> after;
+            if (patchpointValue->type() != Void) {
+                switch (patchpointValue->resultConstraint.kind()) {
+                case ValueRep::Any:
+                case ValueRep::SomeRegister:
+                    inst.args.append(tmp(patchpointValue));
+                    break;
+                case ValueRep::Register: {
+                    Tmp reg = Tmp(patchpointValue->resultConstraint.reg());
+                    inst.args.append(reg);
+                    after.append(Inst(
+                        relaxedMoveForType(patchpointValue->type()), m_value, reg, tmp(patchpointValue)));
+                    break;
+                }
+                case ValueRep::StackArgument: {
+                    Arg arg = Arg::callArg(patchpointValue->resultConstraint.offsetFromSP());
+                    inst.args.append(arg);
+                    after.append(Inst(
+                        moveForType(patchpointValue->type()), m_value, arg, tmp(patchpointValue)));
+                    break;
+                }
+                default:
+                    RELEASE_ASSERT_NOT_REACHED();
+                    break;
+                }
+            }
             
             fillStackmap(inst, patchpointValue, 0);
             
             m_insts.last().append(WTF::move(inst));
+            m_insts.last().appendVector(after);
             return;
         }
 
index aca0a6e..0fe6a6a 100644 (file)
@@ -64,9 +64,10 @@ bool PatchpointSpecial::isValid(Inst& inst)
 
     if (inst.args.size() < 2)
         return false;
-    if (inst.args[1].kind() != Arg::Tmp)
+    PatchpointValue* patchpoint = inst.origin->as<PatchpointValue>();
+    if (!isArgValidForValue(inst.args[1], patchpoint))
         return false;
-    if (!inst.args[1].isType(inst.origin->airType()))
+    if (!isArgValidForRep(code(), inst.args[1], patchpoint->resultConstraint))
         return false;
 
     return isValidImpl(0, 2, inst);
@@ -77,8 +78,19 @@ bool PatchpointSpecial::admitsStack(Inst& inst, unsigned argIndex)
     if (inst.origin->type() == Void)
         return admitsStackImpl(0, 1, inst, argIndex);
 
-    if (argIndex == 1)
-        return false;
+    if (argIndex == 1) {
+        switch (inst.origin->as<PatchpointValue>()->resultConstraint.kind()) {
+        case ValueRep::Any:
+        case ValueRep::StackArgument:
+            return true;
+        case ValueRep::SomeRegister:
+        case ValueRep::Register:
+            return false;
+        default:
+            RELEASE_ASSERT_NOT_REACHED();
+            return false;
+        }
+    }
 
     return admitsStackImpl(0, 2, inst, argIndex);
 }
index 8dec1e9..f67b53b 100644 (file)
@@ -34,9 +34,16 @@ PatchpointValue::~PatchpointValue()
 {
 }
 
+void PatchpointValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
+{
+    Base::dumpMeta(comma, out);
+    out.print(comma, "resultConstraint = ", resultConstraint);
+}
+
 PatchpointValue::PatchpointValue(unsigned index, Type type, Origin origin)
-    : StackmapValue(index, CheckedOpcode, Patchpoint, type, origin)
+    : Base(index, CheckedOpcode, Patchpoint, type, origin)
     , effects(Effects::forCall())
+    , resultConstraint(type == Void ? ValueRep::Any : ValueRep::SomeRegister)
 {
 }
 
index 13434f4..0421754 100644 (file)
@@ -36,12 +36,29 @@ namespace JSC { namespace B3 {
 
 class PatchpointValue : public StackmapValue {
 public:
+    typedef StackmapValue Base;
+    
     static bool accepts(Opcode opcode) { return opcode == Patchpoint; }
 
     ~PatchpointValue();
 
+    // The effects of the patchpoint. This defaults to Effects::forCall(), but you can set it to anything.
+    //
+    // If there are no effects, B3 is free to assume any use of this PatchpointValue can be replaced with
+    // a use of a different PatchpointValue, so long as the other one also has no effects and has the
+    // same children. Note that this comparison ignores child constraints, the result constraint, and all
+    // other StackmapValue meta-data. If there are read effects but not write effects, then this same sort
+    // of substitution could be made so long as there are no interfering writes.
     Effects effects;
 
+    // The input representation (i.e. constraint) of the return value. This defaults to Any if the type is
+    // Void and it defaults to SomeRegister otherwise. It's illegal to mess with this if the type is Void.
+    // Otherwise you can set this to any input constraint.
+    ValueRep resultConstraint;
+
+protected:
+    void dumpMeta(CommaPrinter&, PrintStream&) const override;
+
 private:
     friend class Procedure;
 
index 9e6c73f..5293cee 100644 (file)
@@ -224,7 +224,7 @@ public:
     const Air::Code& code() const { return *m_code; }
     Air::Code& code() { return *m_code; }
 
-    unsigned frameSize() const;
+    JS_EXPORT_PRIVATE unsigned frameSize() const;
     const RegisterAtOffsetList& calleeSaveRegisters() const;
 
 private:
index dc04263..3aa840a 100644 (file)
@@ -118,26 +118,8 @@ bool StackmapSpecial::isValidImpl(
     for (unsigned i = 0; i < inst.args.size() - numIgnoredAirArgs; ++i) {
         Value* child = value->child(i + numIgnoredB3Args);
         Arg& arg = inst.args[i + numIgnoredAirArgs];
-        
-        switch (arg.kind()) {
-        case Arg::Tmp:
-        case Arg::Imm:
-        case Arg::Imm64:
-        case Arg::Stack:
-        case Arg::CallArg:
-            break; // OK
-        case Arg::Addr:
-            if (arg.base() != Tmp(GPRInfo::callFrameRegister)
-                && arg.base() != Tmp(MacroAssembler::stackPointerRegister))
-                return false;
-            break;
-        default:
-            return false;
-        }
-        
-        Arg::Type type = Arg::typeForB3Type(child->type());
 
-        if (!arg.isType(type))
+        if (!isArgValidForValue(arg, child))
             return false;
     }
 
@@ -149,39 +131,8 @@ bool StackmapSpecial::isValidImpl(
         ValueRep& rep = value->m_reps[i];
         Arg& arg = inst.args[i - numIgnoredB3Args + numIgnoredAirArgs];
 
-        switch (rep.kind()) {
-        case ValueRep::Any:
-            // We already verified this above.
-            break;
-        case ValueRep::SomeRegister:
-            if (!arg.isTmp())
-                return false;
-            break;
-        case ValueRep::Register:
-            if (arg != Tmp(rep.reg()))
-                return false;
-            break;
-        case ValueRep::Stack:
-            // This is not a valid input representation.
-            ASSERT_NOT_REACHED();
-            break;
-        case ValueRep::StackArgument:
-            if (arg == Arg::callArg(rep.offsetFromSP()))
-                break;
-            if (arg.isAddr() && code().frameSize()) {
-                if (arg.base() == Tmp(GPRInfo::callFrameRegister)
-                    && arg.offset() == rep.offsetFromSP() - code().frameSize())
-                    break;
-                if (arg.base() == Tmp(MacroAssembler::stackPointerRegister)
-                    && arg.offset() == rep.offsetFromSP())
-                    break;
-            }
+        if (!isArgValidForRep(code(), arg, rep))
             return false;
-        case ValueRep::Constant:
-            // This is not a valid input representation.
-            ASSERT_NOT_REACHED();
-            break;
-        }
     }
 
     return true;
@@ -216,6 +167,57 @@ void StackmapSpecial::appendRepsImpl(
         result.append(repForArg(*context.code, inst.args[i]));
 }
 
+bool StackmapSpecial::isArgValidForValue(const Arg::Arg& arg, Value* value)
+{
+    switch (arg.kind()) {
+    case Arg::Tmp:
+    case Arg::Imm:
+    case Arg::Imm64:
+    case Arg::Stack:
+    case Arg::CallArg:
+        break; // OK
+    case Arg::Addr:
+        if (arg.base() != Tmp(GPRInfo::callFrameRegister)
+            && arg.base() != Tmp(MacroAssembler::stackPointerRegister))
+            return false;
+        break;
+    default:
+        return false;
+    }
+    
+    Arg::Type type = Arg::typeForB3Type(value->type());
+
+    return arg.isType(type);
+}
+
+bool StackmapSpecial::isArgValidForRep(Air::Code& code, const Air::Arg& arg, const ValueRep& rep)
+{
+    switch (rep.kind()) {
+    case ValueRep::Any:
+        // We already verified by isArgValidForValue().
+        return true;
+    case ValueRep::SomeRegister:
+        return arg.isTmp();
+    case ValueRep::Register:
+        return arg == Tmp(rep.reg());
+    case ValueRep::StackArgument:
+        if (arg == Arg::callArg(rep.offsetFromSP()))
+            return true;
+        if (arg.isAddr() && code.frameSize()) {
+            if (arg.base() == Tmp(GPRInfo::callFrameRegister)
+                && arg.offset() == rep.offsetFromSP() - code.frameSize())
+                return true;
+            if (arg.base() == Tmp(MacroAssembler::stackPointerRegister)
+                && arg.offset() == rep.offsetFromSP())
+                return true;
+        }
+        return false;
+    default:
+        RELEASE_ASSERT_NOT_REACHED();
+        return false;
+    }
+}
+
 ValueRep StackmapSpecial::repForArg(Code& code, const Arg& arg)
 {
     switch (arg.kind()) {
index 400181c..0b25205 100644 (file)
@@ -66,6 +66,8 @@ protected:
     // Appends the reps for the Inst's args, starting with numIgnoredArgs, to the given vector.
     void appendRepsImpl(Air::GenerationContext&, unsigned numIgnoredArgs, Air::Inst&, Vector<ValueRep>&);
 
+    static bool isArgValidForValue(const Air::Arg&, Value*);
+    static bool isArgValidForRep(Air::Code&, const Air::Arg&, const ValueRep&);
     static ValueRep repForArg(Air::Code&, const Air::Arg&);
 };
 
index 1926d7c..13a51fa 100644 (file)
@@ -97,6 +97,79 @@ public:
 
     const Vector<ValueRep>& reps() const { return m_reps; }
 
+    // Stackmaps allow you to specify that the operation may clobber some registers. Clobbering a register
+    // means that the operation appears to store a value into the register, but the compiler doesn't
+    // assume to know anything about what kind of value might have been stored. In B3's model of
+    // execution, registers are read or written at instruction boundaries rather than inside the
+    // instructions themselves. A register could be read or written immediately before the instruction
+    // executes, or immediately after. Note that at a boundary between instruction A and instruction B we
+    // simultaneously look at what A does after it executes and what B does before it executes. This is
+    // because when the compiler considers what happens to registers, it views the boundary between two
+    // instructions as a kind of atomic point where the late effects of A happen at the same time as the
+    // early effects of B.
+    //
+    // The compiler views a stackmap as a single instruction, even though of course the stackmap may be
+    // composed of any number of instructions (if it's a Patchpoint). You can claim that a stackmap value
+    // clobbers a set of registers before the stackmap's instruction or after. Clobbering before is called
+    // early clobber, while clobbering after is called late clobber.
+    //
+    // This is quite flexible but it has its limitations. Any register listed as an early clobber will
+    // interfere with all uses of the stackmap. Any register listed as a late clobber will interfere with
+    // all defs of the stackmap (i.e. the result). This means that it's currently not possible to claim
+    // to clobber a register while still allowing that register to be used for both an input and an output
+    // of the instruction. It just so happens that B3's sole client (the FTL) currently never wants to
+    // convey such a constraint, but it will want it eventually (FIXME:
+    // https://bugs.webkit.org/show_bug.cgi?id=151823).
+    //
+    // Note that a common use case of early clobber sets is to indicate that this is the set of registers
+    // that shall not be used for inputs to the value. But B3 supports two different ways of specifying
+    // this, the other being LateUse in combination with late clobber (not yet available to stackmaps
+    // directly, FIXME: https://bugs.webkit.org/show_bug.cgi?id=151335). A late use makes the use of that
+    // value appear to happen after the instruction. This means that a late use cannot use the same
+    // register as the result and it cannot use the same register as either early or late clobbered
+    // registers. Late uses are usually a better way of saying that a clobbered register cannot be used
+    // for an input. Early clobber means that some register(s) interfere with *all* inputs, while LateUse
+    // means that some value interferes with whatever is live after the instruction. Below is a list of
+    // examples of how the FTL can handle its various kinds of scenarios using a combination of early
+    // clobber, late clobber, and late use. These examples are for X86_64, w.l.o.g.
+    //
+    // Basic ById patchpoint: Early and late clobber of r11. Early clobber prevents any inputs from using
+    // r11 since that would mess with the MacroAssembler's assumptions when we
+    // AllowMacroScratchRegisterUsage. Late clobber tells B3 that the patchpoint may overwrite r11.
+    //
+    // ById patchpoint in a try block with some live state: This might throw an exception after already
+    // assigning to the result. So, this should LateUse all stackmap values to ensure that the stackmap
+    // values don't interfere with the result. Note that we do not LateUse the non-OSR inputs of the ById
+    // since LateUse implies that the use is cold: the register allocator will assume that the use is not
+    // important for the critical path. Also, early and late clobber of r11.
+    //
+    // Basic ByIdFlush patchpoint: We could do Flush the same way we did it with LLVM: ignore it and let
+    // PolymorphicAccess figure it out. Or, we could add internal clobber support (FIXME:
+    // https://bugs.webkit.org/show_bug.cgi?id=151823). Or, we could do it by early clobbering r11, late
+    // clobbering all volatile registers, and constraining the result to some register. Or, we could do
+    // that but leave the result constrained to SomeRegister, which will cause it to use a callee-save
+    // register. Internal clobber support would allow us to use SomeRegister while getting the result into
+    // a volatile register.
+    //
+    // ByIdFlush patchpoint in a try block with some live state: LateUse all for-OSR stackmap values,
+    // early clobber of r11 to prevent the other inputs from using r11, and late clobber of all volatile
+    // registers to make way for the call. To handle the result, we could do any of what is listed in the
+    // previous paragraph.
+    //
+    // Basic JS call: Force all non-OSR inputs into specific locations (register, stack, whatever).
+    // All volatile registers are late-clobbered. The output is constrained to a register as well.
+    //
+    // JS call in a try block with some live state: LateUse all for-OSR stackmap values, fully constrain
+    // all non-OSR inputs and the result, and late clobber all volatile registers.
+    //
+    // JS tail call: Pass all inputs as a warm variant of Any (FIXME:
+    // https://bugs.webkit.org/show_bug.cgi?id=151811).
+    //
+    // Note that we cannot yet do all of these things because although Air already supports all of these
+    // various forms of uses (LateUse and warm unconstrained use), B3 doesn't yet expose all of it. The
+    // bugs are:
+    // https://bugs.webkit.org/show_bug.cgi?id=151335 (LateUse)
+    // https://bugs.webkit.org/show_bug.cgi?id=151811 (warm Any)
     void clobberEarly(const RegisterSet& set)
     {
         m_earlyClobbered.merge(set);
index 18c37d2..abf2f2e 100644 (file)
@@ -290,6 +290,10 @@ public:
                 // return type.
                 break;
             case Patchpoint:
+                if (value->type() == Void)
+                    VALIDATE(value->as<PatchpointValue>()->resultConstraint == ValueRep::Any, ("At ", *value));
+                else
+                    validateStackmapConstraint(value, ConstrainedValue(value, value->as<PatchpointValue>()->resultConstraint));
                 validateStackmap(value);
                 break;
             case CheckAdd:
@@ -338,14 +342,26 @@ private:
         StackmapValue* stackmap = value->as<StackmapValue>();
         VALIDATE(stackmap, ("At ", *value));
         VALIDATE(stackmap->numChildren() >= stackmap->reps().size(), ("At ", *stackmap));
-        for (unsigned i = 0; i < stackmap->reps().size(); ++i) {
-            const ValueRep& rep = stackmap->reps()[i];
-            if (rep.kind() != ValueRep::Register)
-                continue;
-            if (rep.reg().isGPR())
-                VALIDATE(isInt(stackmap->child(i)->type()), ("At ", *stackmap));
+        for (ConstrainedValue child : stackmap->constrainedChildren())
+            validateStackmapConstraint(stackmap, child);
+    }
+    
+    void validateStackmapConstraint(Value* context, const ConstrainedValue& value)
+    {
+        switch (value.rep().kind()) {
+        case ValueRep::Any:
+        case ValueRep::SomeRegister:
+        case ValueRep::StackArgument:
+            break;
+        case ValueRep::Register:
+            if (value.rep().reg().isGPR())
+                VALIDATE(isInt(value.value()->type()), ("At ", *context, ": ", value));
             else
-                VALIDATE(isFloat(stackmap->child(i)->type()), ("At ", *stackmap));
+                VALIDATE(isFloat(value.value()->type()), ("At ", *context, ": ", value));
+            break;
+        default:
+            VALIDATE(false, ("At ", *context, ": ", value));
+            break;
         }
     }
 
index b161420..4ef7cfd 100644 (file)
@@ -194,7 +194,7 @@ public:
         return bitwise_cast<double>(value());
     }
 
-    void dump(PrintStream&) const;
+    JS_EXPORT_PRIVATE void dump(PrintStream&) const;
 
 private:
     Kind m_kind;
index 315759d..4316810 100644 (file)
@@ -3720,6 +3720,87 @@ void testPatchpointManyImms()
     CHECK(!compileAndRun<int>(proc));
 }
 
+void testPatchpointWithRegisterResult()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+    PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+    patchpoint->append(ConstrainedValue(arg1, ValueRep::SomeRegister));
+    patchpoint->append(ConstrainedValue(arg2, ValueRep::SomeRegister));
+    patchpoint->resultConstraint = ValueRep::reg(GPRInfo::nonArgGPR0);
+    patchpoint->setGenerator(
+        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0] == ValueRep::reg(GPRInfo::nonArgGPR0));
+            CHECK(params.reps[1].isGPR());
+            CHECK(params.reps[2].isGPR());
+            jit.move(params.reps[1].gpr(), GPRInfo::nonArgGPR0);
+            jit.add32(params.reps[2].gpr(), GPRInfo::nonArgGPR0);
+        });
+    root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
+
+    CHECK(compileAndRun<int>(proc, 1, 2) == 3);
+}
+
+void testPatchpointWithStackArgumentResult()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+    PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+    patchpoint->append(ConstrainedValue(arg1, ValueRep::SomeRegister));
+    patchpoint->append(ConstrainedValue(arg2, ValueRep::SomeRegister));
+    patchpoint->resultConstraint = ValueRep::stackArgument(0);
+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint->setGenerator(
+        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0] == ValueRep::stack(-static_cast<intptr_t>(proc.frameSize())));
+            CHECK(params.reps[1].isGPR());
+            CHECK(params.reps[2].isGPR());
+            jit.store32(params.reps[1].gpr(), CCallHelpers::Address(CCallHelpers::stackPointerRegister, 0));
+            jit.add32(params.reps[2].gpr(), CCallHelpers::Address(CCallHelpers::stackPointerRegister, 0));
+        });
+    root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
+
+    CHECK(compileAndRun<int>(proc, 1, 2) == 3);
+}
+
+void testPatchpointWithAnyResult()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+    PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Double, Origin());
+    patchpoint->append(ConstrainedValue(arg1, ValueRep::SomeRegister));
+    patchpoint->append(ConstrainedValue(arg2, ValueRep::SomeRegister));
+    patchpoint->resultConstraint = ValueRep::Any;
+    patchpoint->clobberLate(RegisterSet::allFPRs());
+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
+    patchpoint->clobber(RegisterSet(GPRInfo::regT0));
+    patchpoint->setGenerator(
+        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0].isStack());
+            CHECK(params.reps[1].isGPR());
+            CHECK(params.reps[2].isGPR());
+            jit.move(params.reps[1].gpr(), GPRInfo::regT0);
+            jit.add32(params.reps[2].gpr(), GPRInfo::regT0);
+            jit.convertInt32ToDouble(GPRInfo::regT0, FPRInfo::fpRegT0);
+            jit.storeDouble(FPRInfo::fpRegT0, CCallHelpers::Address(GPRInfo::callFrameRegister, params.reps[0].offsetFromFP()));
+        });
+    root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
+
+    CHECK(compileAndRun<double>(proc, 1, 2) == 3);
+}
+
 void testSimpleCheck()
 {
     Procedure proc;
@@ -5979,6 +6060,9 @@ void run(const char* filter)
     RUN(testPatchpointAny());
     RUN(testPatchpointAnyImm());
     RUN(testPatchpointManyImms());
+    RUN(testPatchpointWithRegisterResult());
+    RUN(testPatchpointWithStackArgumentResult());
+    RUN(testPatchpointWithAnyResult());
     RUN(testSimpleCheck());
     RUN(testCheckLessThan());
     RUN(testCheckMegaCombo());
index c9710fd..aab9612 100644 (file)
@@ -59,7 +59,7 @@ public:
 #endif
     static RegisterSet volatileRegistersForJSCall();
     static RegisterSet stubUnavailableRegisters(); // The union of callee saves and special registers.
-    static RegisterSet macroScratchRegisters();
+    JS_EXPORT_PRIVATE static RegisterSet macroScratchRegisters();
     JS_EXPORT_PRIVATE static RegisterSet allGPRs();
     JS_EXPORT_PRIVATE static RegisterSet allFPRs();
     static RegisterSet allRegisters();