B3 Patchpoint and Check opcodes should be able to specify WarmAny, ColdAny, and LateC...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Dec 2015 03:29:39 +0000 (03:29 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Dec 2015 03:29:39 +0000 (03:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151335

Reviewed by Geoffrey Garen.

This removes ValueRep::Any and replaces it with ValueRep::WarmAny, ValueRep::ColdAny, and
ValueRep::LateColdAny. I think that conceptually the most obvious users of patchpoints are inline
caches, which would use WarmAny for their non-OSR inputs. For this reason, I make WarmAny the
default.

However, the StackmapValue optimization that provides a default ValueRep for any that are missing
was meant for OSR. So, this optimization now uses ColdAny.

This patch wires this change through the whole compiler and adds some tests.

* b3/B3CheckSpecial.cpp:
(JSC::B3::CheckSpecial::Key::Key):
(JSC::B3::CheckSpecial::Key::dump):
(JSC::B3::CheckSpecial::CheckSpecial):
* b3/B3CheckSpecial.h:
(JSC::B3::CheckSpecial::Key::Key):
(JSC::B3::CheckSpecial::Key::opcode):
(JSC::B3::CheckSpecial::Key::numArgs):
(JSC::B3::CheckSpecial::Key::stackmapRole):
* b3/B3CheckValue.cpp:
(JSC::B3::CheckValue::CheckValue):
* b3/B3ConstrainedValue.h:
(JSC::B3::ConstrainedValue::ConstrainedValue):
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::fillStackmap):
(JSC::B3::Air::LowerToAir::lower):
* b3/B3MoveConstants.cpp:
* b3/B3PatchpointSpecial.cpp:
(JSC::B3::PatchpointSpecial::forEachArg):
(JSC::B3::PatchpointSpecial::isValid):
(JSC::B3::PatchpointSpecial::admitsStack):
* b3/B3PatchpointValue.cpp:
(JSC::B3::PatchpointValue::PatchpointValue):
* b3/B3PatchpointValue.h:
* b3/B3StackmapSpecial.cpp:
(JSC::B3::StackmapSpecial::forEachArgImpl):
(JSC::B3::StackmapSpecial::admitsStackImpl):
(JSC::B3::StackmapSpecial::isArgValidForRep):
(WTF::printInternal):
* b3/B3StackmapSpecial.h:
* b3/B3StackmapValue.cpp:
(JSC::B3::StackmapValue::append):
(JSC::B3::StackmapValue::setConstraint):
* b3/B3StackmapValue.h:
* b3/B3Validate.cpp:
* b3/B3ValueRep.cpp:
(JSC::B3::ValueRep::dump):
(WTF::printInternal):
* b3/B3ValueRep.h:
(JSC::B3::ValueRep::ValueRep):
(JSC::B3::ValueRep::reg):
(JSC::B3::ValueRep::operator!=):
(JSC::B3::ValueRep::operator bool):
(JSC::B3::ValueRep::isAny):
(JSC::B3::ValueRep::isSomeRegister):
* b3/testb3.cpp:
(JSC::B3::compileAndRun):
(JSC::B3::add32):
(JSC::B3::test42):
(JSC::B3::testSimplePatchpoint):
(JSC::B3::testPatchpointWithEarlyClobber):
(JSC::B3::testPatchpointFixedRegister):
(JSC::B3::testPatchpointAny):
(JSC::B3::testPatchpointLotsOfLateAnys):
(JSC::B3::testPatchpointAnyImm):
(JSC::B3::testPatchpointManyImms):
(JSC::B3::testPatchpointWithRegisterResult):
(JSC::B3::testPatchpointWithAnyResult):
(JSC::B3::run):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::blessSpeculation):

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

19 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3CheckSpecial.cpp
Source/JavaScriptCore/b3/B3CheckSpecial.h
Source/JavaScriptCore/b3/B3CheckValue.cpp
Source/JavaScriptCore/b3/B3ConstrainedValue.h
Source/JavaScriptCore/b3/B3LowerToAir.cpp
Source/JavaScriptCore/b3/B3MoveConstants.cpp
Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp
Source/JavaScriptCore/b3/B3PatchpointValue.cpp
Source/JavaScriptCore/b3/B3PatchpointValue.h
Source/JavaScriptCore/b3/B3StackmapSpecial.cpp
Source/JavaScriptCore/b3/B3StackmapSpecial.h
Source/JavaScriptCore/b3/B3StackmapValue.cpp
Source/JavaScriptCore/b3/B3StackmapValue.h
Source/JavaScriptCore/b3/B3Validate.cpp
Source/JavaScriptCore/b3/B3ValueRep.cpp
Source/JavaScriptCore/b3/B3ValueRep.h
Source/JavaScriptCore/b3/testb3.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

index 8d090b0..2d831ec 100644 (file)
@@ -1,5 +1,84 @@
 2015-12-03  Filip Pizlo  <fpizlo@apple.com>
 
+        B3 Patchpoint and Check opcodes should be able to specify WarmAny, ColdAny, and LateColdAny
+        https://bugs.webkit.org/show_bug.cgi?id=151335
+
+        Reviewed by Geoffrey Garen.
+
+        This removes ValueRep::Any and replaces it with ValueRep::WarmAny, ValueRep::ColdAny, and
+        ValueRep::LateColdAny. I think that conceptually the most obvious users of patchpoints are inline
+        caches, which would use WarmAny for their non-OSR inputs. For this reason, I make WarmAny the
+        default.
+
+        However, the StackmapValue optimization that provides a default ValueRep for any that are missing
+        was meant for OSR. So, this optimization now uses ColdAny.
+
+        This patch wires this change through the whole compiler and adds some tests.
+
+        * b3/B3CheckSpecial.cpp:
+        (JSC::B3::CheckSpecial::Key::Key):
+        (JSC::B3::CheckSpecial::Key::dump):
+        (JSC::B3::CheckSpecial::CheckSpecial):
+        * b3/B3CheckSpecial.h:
+        (JSC::B3::CheckSpecial::Key::Key):
+        (JSC::B3::CheckSpecial::Key::opcode):
+        (JSC::B3::CheckSpecial::Key::numArgs):
+        (JSC::B3::CheckSpecial::Key::stackmapRole):
+        * b3/B3CheckValue.cpp:
+        (JSC::B3::CheckValue::CheckValue):
+        * b3/B3ConstrainedValue.h:
+        (JSC::B3::ConstrainedValue::ConstrainedValue):
+        * b3/B3LowerToAir.cpp:
+        (JSC::B3::Air::LowerToAir::fillStackmap):
+        (JSC::B3::Air::LowerToAir::lower):
+        * b3/B3MoveConstants.cpp:
+        * b3/B3PatchpointSpecial.cpp:
+        (JSC::B3::PatchpointSpecial::forEachArg):
+        (JSC::B3::PatchpointSpecial::isValid):
+        (JSC::B3::PatchpointSpecial::admitsStack):
+        * b3/B3PatchpointValue.cpp:
+        (JSC::B3::PatchpointValue::PatchpointValue):
+        * b3/B3PatchpointValue.h:
+        * b3/B3StackmapSpecial.cpp:
+        (JSC::B3::StackmapSpecial::forEachArgImpl):
+        (JSC::B3::StackmapSpecial::admitsStackImpl):
+        (JSC::B3::StackmapSpecial::isArgValidForRep):
+        (WTF::printInternal):
+        * b3/B3StackmapSpecial.h:
+        * b3/B3StackmapValue.cpp:
+        (JSC::B3::StackmapValue::append):
+        (JSC::B3::StackmapValue::setConstraint):
+        * b3/B3StackmapValue.h:
+        * b3/B3Validate.cpp:
+        * b3/B3ValueRep.cpp:
+        (JSC::B3::ValueRep::dump):
+        (WTF::printInternal):
+        * b3/B3ValueRep.h:
+        (JSC::B3::ValueRep::ValueRep):
+        (JSC::B3::ValueRep::reg):
+        (JSC::B3::ValueRep::operator!=):
+        (JSC::B3::ValueRep::operator bool):
+        (JSC::B3::ValueRep::isAny):
+        (JSC::B3::ValueRep::isSomeRegister):
+        * b3/testb3.cpp:
+        (JSC::B3::compileAndRun):
+        (JSC::B3::add32):
+        (JSC::B3::test42):
+        (JSC::B3::testSimplePatchpoint):
+        (JSC::B3::testPatchpointWithEarlyClobber):
+        (JSC::B3::testPatchpointFixedRegister):
+        (JSC::B3::testPatchpointAny):
+        (JSC::B3::testPatchpointLotsOfLateAnys):
+        (JSC::B3::testPatchpointAnyImm):
+        (JSC::B3::testPatchpointManyImms):
+        (JSC::B3::testPatchpointWithRegisterResult):
+        (JSC::B3::testPatchpointWithAnyResult):
+        (JSC::B3::run):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::DFG::LowerDFGToLLVM::blessSpeculation):
+
+2015-12-03  Filip Pizlo  <fpizlo@apple.com>
+
         B3 patchpoints should allow specifying output constraints
         https://bugs.webkit.org/show_bug.cgi?id=151809
 
index e2de864..87219d8 100644 (file)
@@ -70,7 +70,7 @@ CheckSpecial::Key::Key(const Inst& inst)
 {
     m_opcode = inst.opcode;
     m_numArgs = inst.args.size();
-    m_stackmapRole = Arg::Use;
+    m_stackmapRole = SameAsRep;
 }
 
 void CheckSpecial::Key::dump(PrintStream& out) const
@@ -78,7 +78,7 @@ void CheckSpecial::Key::dump(PrintStream& out) const
     out.print(m_opcode, "(", m_numArgs, ",", m_stackmapRole, ")");
 }
 
-CheckSpecial::CheckSpecial(Air::Opcode opcode, unsigned numArgs, Arg::Role stackmapRole)
+CheckSpecial::CheckSpecial(Air::Opcode opcode, unsigned numArgs, RoleMode stackmapRole)
     : m_checkOpcode(opcode)
     , m_stackmapRole(stackmapRole)
     , m_numCheckArgs(numArgs)
index 2f9747e..c289895 100644 (file)
@@ -57,12 +57,12 @@ public:
     public:
         Key()
             : m_opcode(Air::Nop)
-            , m_stackmapRole(Air::Arg::Use)
+            , m_stackmapRole(SameAsRep)
             , m_numArgs(0)
         {
         }
         
-        Key(Air::Opcode opcode, unsigned numArgs, Air::Arg::Role stackmapRole = Air::Arg::Use)
+        Key(Air::Opcode opcode, unsigned numArgs, RoleMode stackmapRole = SameAsRep)
             : m_opcode(opcode)
             , m_stackmapRole(stackmapRole)
             , m_numArgs(numArgs)
@@ -87,13 +87,13 @@ public:
 
         Air::Opcode opcode() const { return m_opcode; }
         unsigned numArgs() const { return m_numArgs; }
-        Air::Arg::Role stackmapRole() const { return m_stackmapRole; }
+        RoleMode stackmapRole() const { return m_stackmapRole; }
 
         void dump(PrintStream& out) const;
 
         Key(WTF::HashTableDeletedValueType)
             : m_opcode(Air::Nop)
-            , m_stackmapRole(Air::Arg::Use)
+            , m_stackmapRole(SameAsRep)
             , m_numArgs(1)
         {
         }
@@ -111,11 +111,11 @@ public:
         
     private:
         Air::Opcode m_opcode;
-        Air::Arg::Role m_stackmapRole;
+        RoleMode m_stackmapRole;
         unsigned m_numArgs;
     };
     
-    CheckSpecial(Air::Opcode, unsigned numArgs, Air::Arg::Role stackmapRole = Air::Arg::Use);
+    CheckSpecial(Air::Opcode, unsigned numArgs, RoleMode stackmapRole = SameAsRep);
     CheckSpecial(const Key&);
     ~CheckSpecial();
 
@@ -140,7 +140,7 @@ protected:
 
 private:
     Air::Opcode m_checkOpcode;
-    Air::Arg::Role m_stackmapRole;
+    RoleMode m_stackmapRole;
     unsigned m_numCheckArgs;
 };
 
index 55b791a..99ce426 100644 (file)
@@ -47,8 +47,8 @@ CheckValue::CheckValue(unsigned index, Opcode opcode, Origin origin, Value* left
     ASSERT(B3::isInt(type()));
     ASSERT(left->type() == right->type());
     ASSERT(opcode == CheckAdd || opcode == CheckSub || opcode == CheckMul);
-    append(ConstrainedValue(left, ValueRep::Any));
-    append(ConstrainedValue(right, ValueRep::Any));
+    append(ConstrainedValue(left, ValueRep::WarmAny));
+    append(ConstrainedValue(right, ValueRep::WarmAny));
 }
 
 // Use this form for Check.
@@ -56,7 +56,7 @@ CheckValue::CheckValue(unsigned index, Opcode opcode, Origin origin, Value* pred
     : StackmapValue(index, CheckedOpcode, opcode, Void, origin)
 {
     ASSERT(opcode == Check);
-    append(ConstrainedValue(predicate, ValueRep::Any));
+    append(ConstrainedValue(predicate, ValueRep::WarmAny));
 }
 
 } } // namespace JSC::B3
index c77d035..b23f41e 100644 (file)
@@ -42,7 +42,7 @@ public:
 
     ConstrainedValue(Value* value)
         : m_value(value)
-        , m_rep(ValueRep::Any)
+        , m_rep(ValueRep::WarmAny)
     {
     }
 
index 703a116..0fabe49 100644 (file)
@@ -797,7 +797,9 @@ private:
 
             Arg arg;
             switch (value.rep().kind()) {
-            case ValueRep::Any:
+            case ValueRep::WarmAny:
+            case ValueRep::ColdAny:
+            case ValueRep::LateColdAny:
                 if (imm(value.value()))
                     arg = imm(value.value());
                 else if (value.value()->hasInt64())
@@ -1700,7 +1702,9 @@ private:
             Vector<Inst> after;
             if (patchpointValue->type() != Void) {
                 switch (patchpointValue->resultConstraint.kind()) {
-                case ValueRep::Any:
+                case ValueRep::WarmAny:
+                case ValueRep::ColdAny:
+                case ValueRep::LateColdAny:
                 case ValueRep::SomeRegister:
                     inst.args.append(tmp(patchpointValue));
                     break;
@@ -1761,7 +1765,7 @@ private:
 
             Air::Opcode opcode = Air::Oops;
             Commutativity commutativity = NotCommutative;
-            Arg::Role stackmapRole = Arg::Use;
+            StackmapSpecial::RoleMode stackmapRole = StackmapSpecial::SameAsRep;
             switch (m_value->opcode()) {
             case CheckAdd:
                 opcode = opcodeForType(BranchAdd32, BranchAdd64, Air::Oops, m_value->type());
@@ -1772,7 +1776,7 @@ private:
                 break;
             case CheckMul:
                 opcode = opcodeForType(BranchMul32, BranchMul64, Air::Oops, checkValue->type());
-                stackmapRole = Arg::LateUse;
+                stackmapRole = StackmapSpecial::ForceLateUse;
                 break;
             default:
                 RELEASE_ASSERT_NOT_REACHED();
index 7cdb375..133853c 100644 (file)
@@ -93,7 +93,7 @@ public:
                     ValueKey key = child->key();
                     if (stackmap
                         && goesInTable(key)
-                        && stackmap->constrainedChild(childIndex).rep() == ValueRep::Any) {
+                        && stackmap->constrainedChild(childIndex).rep().isAny()) {
                         // This is a weird special case. When we constant-fold an argument to a
                         // stackmap, and that argument has the Any constraint, we want to just
                         // tell the stackmap's generator that the argument is a constant rather
index 0fe6a6a..9785f83 100644 (file)
@@ -49,12 +49,12 @@ void PatchpointSpecial::forEachArg(Inst& inst, const ScopedLambda<Inst::EachArgC
     // https://bugs.webkit.org/show_bug.cgi?id=151335
     
     if (inst.origin->type() == Void) {
-        forEachArgImpl(0, 1, inst, Arg::Use, callback);
+        forEachArgImpl(0, 1, inst, SameAsRep, callback);
         return;
     }
 
     callback(inst.args[1], Arg::Def, inst.origin->airType());
-    forEachArgImpl(0, 2, inst, Arg::Use, callback);
+    forEachArgImpl(0, 2, inst, SameAsRep, callback);
 }
 
 bool PatchpointSpecial::isValid(Inst& inst)
@@ -80,7 +80,7 @@ bool PatchpointSpecial::admitsStack(Inst& inst, unsigned argIndex)
 
     if (argIndex == 1) {
         switch (inst.origin->as<PatchpointValue>()->resultConstraint.kind()) {
-        case ValueRep::Any:
+        case ValueRep::WarmAny:
         case ValueRep::StackArgument:
             return true;
         case ValueRep::SomeRegister:
index f67b53b..b5fdf23 100644 (file)
@@ -43,7 +43,7 @@ void PatchpointValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
 PatchpointValue::PatchpointValue(unsigned index, Type type, Origin origin)
     : Base(index, CheckedOpcode, Patchpoint, type, origin)
     , effects(Effects::forCall())
-    , resultConstraint(type == Void ? ValueRep::Any : ValueRep::SomeRegister)
+    , resultConstraint(type == Void ? ValueRep::WarmAny : ValueRep::SomeRegister)
 {
 }
 
index 0421754..c93cb74 100644 (file)
@@ -51,9 +51,9 @@ public:
     // 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.
+    // The input representation (i.e. constraint) of the return value. This defaults to WarmAny 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:
index 3aa840a..770cba5 100644 (file)
@@ -73,7 +73,7 @@ const RegisterSet& StackmapSpecial::extraEarlyClobberedRegs(Inst& inst)
 
 void StackmapSpecial::forEachArgImpl(
     unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
-    Inst& inst, Arg::Role role, const ScopedLambda<Inst::EachArgCallback>& callback)
+    Inst& inst, RoleMode roleMode, const ScopedLambda<Inst::EachArgCallback>& callback)
 {
     StackmapValue* value = inst.origin->as<StackmapValue>();
     ASSERT(value);
@@ -87,11 +87,30 @@ void StackmapSpecial::forEachArgImpl(
         Arg& arg = inst.args[i + numIgnoredAirArgs];
         ConstrainedValue child = value->constrainedChild(i + numIgnoredB3Args);
 
-        Arg::Role thisRole = role;
-        
-        // Cool down the role if the use is cold.
-        if (child.rep().kind() == ValueRep::Any && thisRole == Arg::Use)
-            thisRole = Arg::ColdUse;
+        Arg::Role role;
+        switch (roleMode) {
+        case SameAsRep:
+            switch (child.rep().kind()) {
+            case ValueRep::WarmAny:
+            case ValueRep::SomeRegister:
+            case ValueRep::Register:
+            case ValueRep::Stack:
+            case ValueRep::StackArgument:
+            case ValueRep::Constant:
+                role = Arg::Use;
+                break;
+            case ValueRep::ColdAny:
+                role = Arg::ColdUse;
+                break;
+            case ValueRep::LateColdAny:
+                role = Arg::LateUse;
+                break;
+            }
+            break;
+        case ForceLateUse:
+            role = Arg::LateUse;
+            break;
+        }
         
         callback(arg, role, Arg::typeForB3Type(child.value()->type()));
     }
@@ -154,7 +173,7 @@ bool StackmapSpecial::admitsStackImpl(
     
     // We only admit stack for Any's, since Stack is not a valid input constraint, and StackArgument
     // translates to a CallArg in Air.
-    if (value->m_reps[stackmapArgIndex].kind() == ValueRep::Any)
+    if (value->m_reps[stackmapArgIndex].isAny())
         return true;
 
     return false;
@@ -193,7 +212,9 @@ bool StackmapSpecial::isArgValidForValue(const Arg::Arg& arg, Value* value)
 bool StackmapSpecial::isArgValidForRep(Air::Code& code, const Air::Arg& arg, const ValueRep& rep)
 {
     switch (rep.kind()) {
-    case ValueRep::Any:
+    case ValueRep::WarmAny:
+    case ValueRep::ColdAny:
+    case ValueRep::LateColdAny:
         // We already verified by isArgValidForValue().
         return true;
     case ValueRep::SomeRegister:
@@ -241,4 +262,23 @@ ValueRep StackmapSpecial::repForArg(Code& code, const Arg& arg)
 
 } } // namespace JSC::B3
 
+namespace WTF {
+
+using namespace JSC::B3;
+
+void printInternal(PrintStream& out, StackmapSpecial::RoleMode mode)
+{
+    switch (mode) {
+    case StackmapSpecial::SameAsRep:
+        out.print("SameAsRep");
+        return;
+    case StackmapSpecial::ForceLateUse:
+        out.print("ForceLateUse");
+        return;
+    }
+    RELEASE_ASSERT_NOT_REACHED();
+}
+
+} // namespace WTF
+
 #endif // ENABLE(B3_JIT)
index 0b25205..ff7d29b 100644 (file)
@@ -45,6 +45,11 @@ public:
     StackmapSpecial();
     virtual ~StackmapSpecial();
 
+    enum RoleMode : int8_t {
+        SameAsRep,
+        ForceLateUse
+    };
+
 protected:
     void reportUsedRegisters(Air::Inst&, const RegisterSet&) override;
     const RegisterSet& extraEarlyClobberedRegs(Air::Inst&) override;
@@ -52,10 +57,10 @@ protected:
 
     // Note that this does not override generate() or dumpImpl()/deepDumpImpl(). We have many some
     // subclasses that implement that.
-
     void forEachArgImpl(
         unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
-        Air::Inst&, Air::Arg::Role role, const ScopedLambda<Air::Inst::EachArgCallback>&);
+        Air::Inst&, RoleMode, const ScopedLambda<Air::Inst::EachArgCallback>&);
+    
     bool isValidImpl(
         unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
         Air::Inst&);
@@ -73,6 +78,12 @@ protected:
 
 } } // namespace JSC::B3
 
+namespace WTF {
+
+void printInternal(PrintStream&, JSC::B3::StackmapSpecial::RoleMode);
+
+} // namespace WTF
+
 #endif // ENABLE(B3_JIT)
 
 #endif // B3StackmapSpecial_h
index 01ea180..9247c45 100644 (file)
@@ -36,13 +36,13 @@ StackmapValue::~StackmapValue()
 
 void StackmapValue::append(const ConstrainedValue& constrainedValue)
 {
-    if (constrainedValue.rep() == ValueRep(ValueRep::Any)) {
+    if (constrainedValue.rep() == ValueRep::ColdAny) {
         children().append(constrainedValue.value());
         return;
     }
 
     while (m_reps.size() < numChildren())
-        m_reps.append(ValueRep::Any);
+        m_reps.append(ValueRep::ColdAny);
 
     children().append(constrainedValue.value());
     m_reps.append(constrainedValue.rep());
@@ -61,11 +61,11 @@ void StackmapValue::setConstrainedChild(unsigned index, const ConstrainedValue&
 
 void StackmapValue::setConstraint(unsigned index, const ValueRep& rep)
 {
-    if (rep == ValueRep(ValueRep::Any))
+    if (rep == ValueRep(ValueRep::ColdAny))
         return;
 
     while (m_reps.size() <= index)
-        m_reps.append(ValueRep::Any);
+        m_reps.append(ValueRep::ColdAny);
 
     m_reps[index] = rep;
 }
index 13a51fa..39aa13f 100644 (file)
@@ -84,11 +84,12 @@ public:
     // children().append(). That will work fine, but it's not recommended.
     void append(const ConstrainedValue&);
 
+    // Helper for appending cold any's. This often used by clients to implement OSR.
     template<typename VectorType>
-    void appendAnys(const VectorType& vector)
+    void appendColdAnys(const VectorType& vector)
     {
         for (Value* value : vector)
-            append(value);
+            append(ConstrainedValue(value, ValueRep::ColdAny));
     }
 
     // This is a helper for something you might do a lot of: append a value that should be constrained
@@ -202,7 +203,7 @@ public:
 
     ConstrainedValue constrainedChild(unsigned index) const
     {
-        return ConstrainedValue(child(index), index < m_reps.size() ? m_reps[index] : ValueRep());
+        return ConstrainedValue(child(index), index < m_reps.size() ? m_reps[index] : ValueRep::ColdAny);
     }
 
     void setConstrainedChild(unsigned index, const ConstrainedValue&);
index abf2f2e..10641a0 100644 (file)
@@ -291,9 +291,21 @@ public:
                 break;
             case Patchpoint:
                 if (value->type() == Void)
-                    VALIDATE(value->as<PatchpointValue>()->resultConstraint == ValueRep::Any, ("At ", *value));
-                else
+                    VALIDATE(value->as<PatchpointValue>()->resultConstraint == ValueRep::WarmAny, ("At ", *value));
+                else {
+                    switch (value->as<PatchpointValue>()->resultConstraint.kind()) {
+                    case ValueRep::WarmAny:
+                    case ValueRep::SomeRegister:
+                    case ValueRep::Register:
+                    case ValueRep::StackArgument:
+                        break;
+                    default:
+                        VALIDATE(false, ("At ", *value));
+                        break;
+                    }
+                    
                     validateStackmapConstraint(value, ConstrainedValue(value, value->as<PatchpointValue>()->resultConstraint));
+                }
                 validateStackmap(value);
                 break;
             case CheckAdd:
@@ -302,14 +314,14 @@ public:
                 VALIDATE(value->numChildren() >= 2, ("At ", *value));
                 VALIDATE(isInt(value->child(0)->type()), ("At ", *value));
                 VALIDATE(isInt(value->child(1)->type()), ("At ", *value));
-                VALIDATE(value->as<StackmapValue>()->constrainedChild(0).rep() == ValueRep::Any, ("At ", *value));
-                VALIDATE(value->as<StackmapValue>()->constrainedChild(1).rep() == ValueRep::Any, ("At ", *value));
+                VALIDATE(value->as<StackmapValue>()->constrainedChild(0).rep() == ValueRep::WarmAny, ("At ", *value));
+                VALIDATE(value->as<StackmapValue>()->constrainedChild(1).rep() == ValueRep::WarmAny, ("At ", *value));
                 validateStackmap(value);
                 break;
             case Check:
                 VALIDATE(value->numChildren() >= 1, ("At ", *value));
                 VALIDATE(isInt(value->child(0)->type()), ("At ", *value));
-                VALIDATE(value->as<StackmapValue>()->constrainedChild(0).rep() == ValueRep::Any, ("At ", *value));
+                VALIDATE(value->as<StackmapValue>()->constrainedChild(0).rep() == ValueRep::WarmAny, ("At ", *value));
                 validateStackmap(value);
                 break;
             case Upsilon:
@@ -349,7 +361,9 @@ private:
     void validateStackmapConstraint(Value* context, const ConstrainedValue& value)
     {
         switch (value.rep().kind()) {
-        case ValueRep::Any:
+        case ValueRep::WarmAny:
+        case ValueRep::ColdAny:
+        case ValueRep::LateColdAny:
         case ValueRep::SomeRegister:
         case ValueRep::StackArgument:
             break;
index d24c957..b8e9b3d 100644 (file)
@@ -34,7 +34,9 @@ void ValueRep::dump(PrintStream& out) const
 {
     out.print(m_kind);
     switch (m_kind) {
-    case Any:
+    case WarmAny:
+    case ColdAny:
+    case LateColdAny:
     case SomeRegister:
         return;
     case Register:
@@ -62,8 +64,14 @@ using namespace JSC::B3;
 void printInternal(PrintStream& out, ValueRep::Kind kind)
 {
     switch (kind) {
-    case ValueRep::Any:
-        out.print("Any");
+    case ValueRep::WarmAny:
+        out.print("WarmAny");
+        return;
+    case ValueRep::ColdAny:
+        out.print("ColdAny");
+        return;
+    case ValueRep::LateColdAny:
+        out.print("LateColdAny");
         return;
     case ValueRep::SomeRegister:
         out.print("SomeRegister");
index 4ef7cfd..0bb0a06 100644 (file)
@@ -43,11 +43,18 @@ namespace JSC { namespace B3 {
 class ValueRep {
 public:
     enum Kind {
-        // As an input representation, this means that B3 can pick any representation. It also currently
-        // implies that the use is cold: the register allocator doesn't count it. As an output
+        // As an input representation, this means that B3 can pick any representation. As an output
         // representation, this means that we don't know. This will only arise as an output
         // representation for the active arguments of Check/CheckAdd/CheckSub/CheckMul.
-        Any,
+        WarmAny,
+
+        // Same as WarmAny, but implies that the use is cold. A cold use is not counted as a use for
+        // computing the priority of the used temporary.
+        ColdAny,
+
+        // Same as ColdAny, but also implies that the use occurs after all other effects of the stackmap
+        // value.
+        LateColdAny,
 
         // As an input representation, this means that B3 should pick some register. It could be a
         // register that this claims to clobber!
@@ -70,7 +77,7 @@ public:
     };
     
     ValueRep()
-        : m_kind(Any)
+        : m_kind(WarmAny)
     {
     }
 
@@ -83,7 +90,7 @@ public:
     ValueRep(Kind kind)
         : m_kind(kind)
     {
-        ASSERT(kind == Any || kind == SomeRegister);
+        ASSERT(kind == WarmAny || kind == ColdAny || kind == LateColdAny || kind == SomeRegister);
     }
 
     static ValueRep reg(Reg reg)
@@ -145,9 +152,9 @@ public:
         return !(*this == other);
     }
 
-    explicit operator bool() const { return kind() != Any; }
+    explicit operator bool() const { return kind() != WarmAny; }
 
-    bool isAny() const { return kind() == Any; }
+    bool isAny() const { return kind() == WarmAny || kind() == ColdAny || kind() == LateColdAny; }
 
     bool isSomeRegister() const { return kind() == SomeRegister; }
     
index 4316810..3ca0c80 100644 (file)
@@ -92,6 +92,16 @@ T compileAndRun(Procedure& procedure, Arguments... arguments)
     return invoke<T>(*compile(procedure), arguments...);
 }
 
+void add32(CCallHelpers& jit, GPRReg src1, GPRReg src2, GPRReg dest)
+{
+    if (src2 == dest)
+        jit.add32(src1, dest);
+    else {
+        jit.move(src1, dest);
+        jit.add32(src2, dest);
+    }
+}
+
 void test42()
 {
     Procedure proc;
@@ -3384,8 +3394,7 @@ void testSimplePatchpoint()
             CHECK(params.reps[0].isGPR());
             CHECK(params.reps[1].isGPR());
             CHECK(params.reps[2].isGPR());
-            jit.move(params.reps[1].gpr(), params.reps[0].gpr());
-            jit.add32(params.reps[2].gpr(), params.reps[0].gpr());
+            add32(jit, params.reps[1].gpr(), params.reps[2].gpr(), params.reps[0].gpr());
         });
     root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
 
@@ -3560,14 +3569,8 @@ void testPatchpointWithEarlyClobber()
             [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
                 CHECK((params.reps[1].gpr() == GPRInfo::argumentGPR0) == arg1InArgGPR);
                 CHECK((params.reps[2].gpr() == GPRInfo::argumentGPR1) == arg2InArgGPR);
-
-                if (params.reps[0].gpr() == params.reps[2].gpr()) {
-                    jit.move(params.reps[2].gpr(), params.reps[0].gpr());
-                    jit.add32(params.reps[1].gpr(), params.reps[0].gpr());
-                } else {
-                    jit.move(params.reps[1].gpr(), params.reps[0].gpr());
-                    jit.add32(params.reps[2].gpr(), params.reps[0].gpr());
-                }
+                
+                add32(jit, params.reps[1].gpr(), params.reps[2].gpr(), params.reps[0].gpr());
             });
 
         root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
@@ -3624,30 +3627,22 @@ void testPatchpointFixedRegister()
             CHECK(params.reps[0].isGPR());
             CHECK(params.reps[1] == ValueRep(GPRInfo::regT0));
             CHECK(params.reps[2] == ValueRep(GPRInfo::regT1));
-            GPRReg result = params.reps[0].gpr();
-
-            if (result == GPRInfo::regT1) {
-                jit.move(GPRInfo::regT1, result);
-                jit.add32(GPRInfo::regT0, result);
-            } else {
-                jit.move(GPRInfo::regT0, result);
-                jit.add32(GPRInfo::regT1, result);
-            }
+            add32(jit, GPRInfo::regT0, GPRInfo::regT1, params.reps[0].gpr());
         });
     root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
 
     CHECK(compileAndRun<int>(proc, 1, 2) == 3);
 }
 
-void testPatchpointAny()
+void testPatchpointAny(ValueRep rep)
 {
     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::Any));
-    patchpoint->append(ConstrainedValue(arg2, ValueRep::Any));
+    patchpoint->append(ConstrainedValue(arg1, rep));
+    patchpoint->append(ConstrainedValue(arg2, rep));
     patchpoint->setGenerator(
         [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
             AllowMacroScratchRegisterUsage allowScratch(jit);
@@ -3656,15 +3651,55 @@ void testPatchpointAny()
             CHECK(params.reps[0].isGPR());
             CHECK(params.reps[1].isGPR());
             CHECK(params.reps[2].isGPR());
-            jit.move(params.reps[1].gpr(), params.reps[0].gpr());
-            jit.add32(params.reps[2].gpr(), params.reps[0].gpr());
+            add32(jit, params.reps[1].gpr(), params.reps[2].gpr(), params.reps[0].gpr());
         });
     root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
 
     CHECK(compileAndRun<int>(proc, 1, 2) == 3);
 }
 
-void testPatchpointAnyImm()
+void testPatchpointLotsOfLateAnys()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Vector<int> things;
+    for (unsigned i = 200; i--;)
+        things.append(i);
+
+    Vector<Value*> values;
+    for (int& thing : things) {
+        Value* value = root->appendNew<MemoryValue>(
+            proc, Load, Int32, Origin(),
+            root->appendNew<ConstPtrValue>(proc, Origin(), &thing));
+        values.append(value);
+    }
+
+    PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+    for (Value* value : values)
+        patchpoint->append(ConstrainedValue(value, ValueRep::LateColdAny));
+    patchpoint->setGenerator(
+        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            // We shouldn't have spilled the inputs, so we assert that they're in registers.
+            CHECK(params.reps.size() == things.size() + 1);
+            CHECK(params.reps[0].isGPR());
+            jit.move(CCallHelpers::TrustedImm32(0), params.reps[0].gpr());
+            for (unsigned i = 1; i < params.reps.size(); ++i) {
+                if (params.reps[i].isGPR()) {
+                    CHECK(params.reps[i] != params.reps[0]);
+                    jit.add32(params.reps[i].gpr(), params.reps[0].gpr());
+                } else {
+                    CHECK(params.reps[i].isStack());
+                    jit.add32(CCallHelpers::Address(GPRInfo::callFrameRegister, params.reps[i].offsetFromFP()), params.reps[0].gpr());
+                }
+            }
+        });
+    root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
+
+    CHECK(compileAndRun<int>(proc) == (things.size() * (things.size() - 1)) / 2);
+}
+
+void testPatchpointAnyImm(ValueRep rep)
 {
     Procedure proc;
     BasicBlock* root = proc.addBlock();
@@ -3673,8 +3708,8 @@ void testPatchpointAnyImm()
         root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
     Value* arg2 = root->appendNew<Const32Value>(proc, Origin(), 42);
     PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
-    patchpoint->append(ConstrainedValue(arg1, ValueRep::Any));
-    patchpoint->append(ConstrainedValue(arg2, ValueRep::Any));
+    patchpoint->append(ConstrainedValue(arg1, rep));
+    patchpoint->append(ConstrainedValue(arg2, rep));
     patchpoint->setGenerator(
         [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
             AllowMacroScratchRegisterUsage allowScratch(jit);
@@ -3701,10 +3736,10 @@ void testPatchpointManyImms()
     Value* arg3 = root->appendNew<Const64Value>(proc, Origin(), 43000000000000ll);
     Value* arg4 = root->appendNew<ConstDoubleValue>(proc, Origin(), 42.5);
     PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Void, Origin());
-    patchpoint->append(ConstrainedValue(arg1, ValueRep::Any));
-    patchpoint->append(ConstrainedValue(arg2, ValueRep::Any));
-    patchpoint->append(ConstrainedValue(arg3, ValueRep::Any));
-    patchpoint->append(ConstrainedValue(arg4, ValueRep::Any));
+    patchpoint->append(ConstrainedValue(arg1, ValueRep::WarmAny));
+    patchpoint->append(ConstrainedValue(arg2, ValueRep::WarmAny));
+    patchpoint->append(ConstrainedValue(arg3, ValueRep::WarmAny));
+    patchpoint->append(ConstrainedValue(arg4, ValueRep::WarmAny));
     patchpoint->setGenerator(
         [&] (CCallHelpers&, const StackmapGenerationParams& params) {
             CHECK(params.reps.size() == 4);
@@ -3737,8 +3772,7 @@ void testPatchpointWithRegisterResult()
             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);
+            add32(jit, params.reps[1].gpr(), params.reps[2].gpr(), GPRInfo::nonArgGPR0);
         });
     root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
 
@@ -3780,7 +3814,7 @@ void testPatchpointWithAnyResult()
     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->resultConstraint = ValueRep::WarmAny;
     patchpoint->clobberLate(RegisterSet::allFPRs());
     patchpoint->clobber(RegisterSet::macroScratchRegisters());
     patchpoint->clobber(RegisterSet(GPRInfo::regT0));
@@ -3791,8 +3825,7 @@ void testPatchpointWithAnyResult()
             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);
+            add32(jit, params.reps[1].gpr(), params.reps[2].gpr(), GPRInfo::regT0);
             jit.convertInt32ToDouble(GPRInfo::regT0, FPRInfo::fpRegT0);
             jit.storeDouble(FPRInfo::fpRegT0, CCallHelpers::Address(GPRInfo::callFrameRegister, params.reps[0].offsetFromFP()));
         });
@@ -6057,8 +6090,12 @@ void run(const char* filter)
     RUN(testPatchpointWithEarlyClobber());
     RUN(testPatchpointCallArg());
     RUN(testPatchpointFixedRegister());
-    RUN(testPatchpointAny());
-    RUN(testPatchpointAnyImm());
+    RUN(testPatchpointAny(ValueRep::WarmAny));
+    RUN(testPatchpointAny(ValueRep::ColdAny));
+    RUN(testPatchpointLotsOfLateAnys());
+    RUN(testPatchpointAnyImm(ValueRep::WarmAny));
+    RUN(testPatchpointAnyImm(ValueRep::ColdAny));
+    RUN(testPatchpointAnyImm(ValueRep::LateColdAny));
     RUN(testPatchpointManyImms());
     RUN(testPatchpointWithRegisterResult());
     RUN(testPatchpointWithStackArgumentResult());
index 77787d1..6a67523 100644 (file)
@@ -9378,7 +9378,7 @@ private:
         OSRExitDescriptorImpl* exitDescriptorImpl = &m_ftlState.osrExitDescriptorImpls.last();
         
         unsigned offset = value->numChildren();
-        value->appendAnys(buildExitArguments(exitDescriptor, m_ftlState.osrExitDescriptorImpls.last(), lowValue));
+        value->appendColdAnys(buildExitArguments(exitDescriptor, m_ftlState.osrExitDescriptorImpls.last(), lowValue));
 
         State* state = &m_ftlState;
         value->setGenerator(