Bug 1488775 - Add String x Number concatenation IC r=tcampbell

Also addresses a pre-existing bug in emitCallStringConcatResult.

Differential Revision: https://phabricator.services.mozilla.com/D5084

--HG--
extra : rebase_source : f08fdbac7a595a16a1a1f52f06a729da293c799e

js/src/jit-test/tests/cacheir/binaryarith.js

@@ -45,6 +45,11 @@ var funAdd5 = (a, b) => { return a + b; }
 warmup(funAdd5, [[true, 10, 11], [false, 1, 1], [10, true, 11], [1, false, 1],
                  [2147483647, true, 2147483648],[true, 2147483647, 2147483648]]);
 
+// Add: String Number Concat
+var funAdd6 = (a, b) => { return a + b; }
+warmup(funAdd6, [["x", 10, "x10"], [10, "bba", "10bba"], ["x", 1.2, "x1.2"],
+                 [1.2, "bba", "1.2bba"]]);
+
 // Sub Int32
 var funSub1 = (a, b) => { return a - b; }
 warmup(funSub1, [[7, 0, 7], [7, 8, -1], [4294967295, 2, 4294967293], [0,0,0]]);
@@ -210,4 +215,35 @@ for (var k=0; k < 30; k++) {
         res+=v;
     }
     assertEq(res, "76543210");
-}
+}
+
+// Begin OOM testing:
+if (!('oomTest' in this))
+    quit();
+
+// Add: String Number Concat OOM test
+var addOom = (a, b) => { return a + b; }
+
+function generate_digits(prefix, start) {
+    digits = []
+    number = ""+start+".25";
+    for (var i = 1; i < 7; i++) {
+        number = i + number;
+        digits.push([prefix, Number(number), prefix + number]);
+    }
+    return digits;
+}
+
+// Trying to defeat dtoacache: Warm the IC with one set of digits, then actually oomTest
+// using another set.
+var warmup_digits = generate_digits("x", 1);
+var test_digits = generate_digits("x", 2);
+
+function ot(digits) {
+    warmup(addOom, digits);
+}
+
+// Ensure ICs are warmed
+ot(warmup_digits);
+
+oomTest(() => { ot(test_digits); });

js/src/jit/BaselineCacheIRCompiler.cpp

@@ -2375,6 +2375,8 @@ BaselineCacheIRCompiler::emitCallStringConcatResult()
     Register rhs = allocator.useRegister(masm, reader.stringOperandId());
     AutoScratchRegisterMaybeOutput scratch(allocator, masm, output);
 
+    allocator.discardStack(masm);
+
     AutoStubFrame stubFrame(*this);
     stubFrame.enter(masm, scratch);
 

js/src/jit/BaselineInspector.cpp

@@ -392,6 +392,8 @@ ParseCacheIRStub(ICStub* stub)
       case CacheOp::LoadStringResult:
       case CacheOp::CallStringConcatResult:
       case CacheOp::CallStringObjectConcatResult:
+      case CacheOp::CallInt32ToString:
+      case CacheOp::CallNumberToString:
         return MIRType::String;
       case CacheOp::DoubleAddResult:
       case CacheOp::DoubleSubResult:

js/src/jit/CacheIR.cpp

@@ -5825,6 +5825,9 @@ BinaryArithIRGenerator::tryAttachStub()
         return true;
     }
 
+    if (tryAttachStringNumberConcat())
+        return true;
+
 
     trackAttached(IRGenerator::NotAttached);
     return false;
@@ -6021,6 +6024,48 @@ BinaryArithIRGenerator::tryAttachInt32()
     return true;
 }
 
+
+bool
+BinaryArithIRGenerator::tryAttachStringNumberConcat()
+{
+    // Only Addition
+    if (op_ != JSOP_ADD)
+        return false;
+
+    if (!(lhs_.isString() && rhs_.isNumber()) &&
+        !(lhs_.isNumber() && rhs_.isString()))
+    {
+        return false;
+    }
+
+    ValOperandId lhsId(writer.setInputOperandId(0));
+    ValOperandId rhsId(writer.setInputOperandId(1));
+
+    auto guardToString = [&](ValOperandId id, HandleValue v) {
+        if (v.isString()) {
+            return writer.guardIsString(id);
+        }
+        if (v.isInt32()) {
+            Int32OperandId intId = writer.guardIsInt32(id);
+            return writer.callInt32ToString(intId);
+        }
+        // At this point we are creating an IC that will handle
+        // both Int32 and Double cases.
+        MOZ_ASSERT(v.isNumber());
+        writer.guardIsNumber(id);
+        return writer.callNumberToString(id);
+    };
+
+    StringOperandId lhsStrId = guardToString(lhsId, lhs_);
+    StringOperandId rhsStrId = guardToString(rhsId, rhs_);
+
+    writer.callStringConcatResult(lhsStrId, rhsStrId);
+
+    writer.returnFromIC();
+    trackAttached("BinaryArith.StringNumberConcat");
+    return true;
+}
+
 bool
 BinaryArithIRGenerator::tryAttachStringConcat()
 {

js/src/jit/CacheIR.h

@@ -266,6 +266,8 @@ extern const char* const CacheKindNames[];
     _(CallSetArrayLength)                 \
     _(CallProxySet)                       \
     _(CallProxySetByValue)                \
+    _(CallInt32ToString)                  \
+    _(CallNumberToString)                 \
                                           \
     /* The *Result ops load a value into the cache's result register. */ \
     _(LoadFixedSlotResult)                \
@@ -1033,6 +1035,18 @@ class MOZ_RAII CacheIRWriter : public JS::CustomAutoRooter
         writeOperandId(rhs);
         buffer_.writeByte(uint32_t(strict));
     }
+    StringOperandId callInt32ToString(Int32OperandId id) {
+        StringOperandId res(nextOperandId_++);
+        writeOpWithOperandId(CacheOp::CallInt32ToString, id);
+        writeOperandId(res);
+        return res;
+    }
+    StringOperandId callNumberToString(ValOperandId id) {
+        StringOperandId res(nextOperandId_++);
+        writeOpWithOperandId(CacheOp::CallNumberToString, id);
+        writeOperandId(res);
+        return res;
+    }
 
     void megamorphicLoadSlotResult(ObjOperandId obj, PropertyName* name, bool handleMissing) {
         writeOpWithOperandId(CacheOp::MegamorphicLoadSlotResult, obj);
@@ -1975,6 +1989,7 @@ class MOZ_RAII BinaryArithIRGenerator : public IRGenerator
     bool tryAttachBitwise();
     bool tryAttachStringConcat();
     bool tryAttachStringObjectConcat();
+    bool tryAttachStringNumberConcat();
 
   public:
     BinaryArithIRGenerator(JSContext* cx, HandleScript, jsbytecode* pc, ICState::Mode,

js/src/jit/CacheIRCompiler.cpp

@@ -4041,6 +4041,63 @@ CacheIRCompiler::emitLoadObject()
     return true;
 }
 
+bool
+CacheIRCompiler::emitCallInt32ToString() {
+    Register input = allocator.useRegister(masm, reader.int32OperandId());
+    Register result = allocator.defineRegister(masm, reader.stringOperandId());
+
+    FailurePath* failure;
+    if (!addFailurePath(&failure))
+        return false;
+
+    LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs());
+    volatileRegs.takeUnchecked(result);
+    masm.PushRegsInMask(volatileRegs);
+
+    masm.setupUnalignedABICall(result);
+    masm.loadJSContext(result);
+    masm.passABIArg(result);
+    masm.passABIArg(input);
+    masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, (js::Int32ToStringHelper)));
+
+    masm.mov(ReturnReg, result);
+    masm.PopRegsInMask(volatileRegs);
+
+    masm.branchPtr(Assembler::Equal, result, ImmPtr(0), failure->label());
+    return true;
+}
+
+bool
+CacheIRCompiler::emitCallNumberToString() {
+    // Float register must be preserved. The BinaryArith ICs use
+    // the fact that baseline has them available, as well as fixed temps on
+    // LBinaryCache.
+    allocator.ensureDoubleRegister(masm, reader.valOperandId(), FloatReg0);
+    Register result = allocator.defineRegister(masm, reader.stringOperandId());
+
+    FailurePath* failure;
+    if (!addFailurePath(&failure))
+        return false;
+
+    LiveRegisterSet volatileRegs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs());
+    volatileRegs.takeUnchecked(result);
+    volatileRegs.addUnchecked(FloatReg0);
+    masm.PushRegsInMask(volatileRegs);
+
+    masm.setupUnalignedABICall(result);
+    masm.loadJSContext(result);
+    masm.passABIArg(result);
+    masm.passABIArg(FloatReg0, MoveOp::DOUBLE);
+    masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, (js::NumberToStringHelper)));
+
+    masm.mov(ReturnReg, result);
+    masm.PopRegsInMask(volatileRegs);
+
+    masm.branchPtr(Assembler::Equal, result, ImmPtr(0), failure->label());
+    return true;
+}
+
+
 void
 js::jit::LoadTypedThingData(MacroAssembler& masm, TypedThingLayout layout, Register obj, Register result)
 {

js/src/jit/CacheIRCompiler.h

@@ -112,6 +112,8 @@ namespace jit {
     _(MegamorphicStoreSlot)               \
     _(MegamorphicHasPropResult)           \
     _(CallObjectHasSparseElementResult)   \
+    _(CallInt32ToString)                  \
+    _(CallNumberToString)                 \
     _(WrapResult)
 
 // Represents a Value on the Baseline frame's expression stack. Slot 0 is the

js/src/jsnum.cpp

@@ -719,6 +719,17 @@ js::Int32ToString<CanGC>(JSContext* cx, int32_t si);
 template JSFlatString*
 js::Int32ToString<NoGC>(JSContext* cx, int32_t si);
 
+JSFlatString*
+js::Int32ToStringHelper(JSContext* cx, int32_t si)
+{
+    AutoUnsafeCallWithABI unsafe;
+    JSFlatString* res = Int32ToString<NoGC>(cx, si);
+    if (!res) {
+        cx->recoverFromOutOfMemory();
+    }
+    return res;
+}
+
 JSAtom*
 js::Int32ToAtom(JSContext* cx, int32_t si)
 {
@@ -1518,6 +1529,17 @@ js::NumberToString<CanGC>(JSContext* cx, double d);
 template JSString*
 js::NumberToString<NoGC>(JSContext* cx, double d);
 
+JSString*
+js::NumberToStringHelper(JSContext* cx, double d)
+{
+    AutoUnsafeCallWithABI unsafe;
+    JSString* res = NumberToString<NoGC>(cx, d);
+    if (!res) {
+        cx->recoverFromOutOfMemory();
+    }
+    return res;
+}
+
 JSAtom*
 js::NumberToAtom(JSContext* cx, double d)
 {

js/src/jsnum.h

@@ -60,6 +60,9 @@ template <AllowGC allowGC>
 extern JSString*
 NumberToString(JSContext* cx, double d);
 
+extern JSString*
+NumberToStringHelper(JSContext* cx, double d);
+
 extern JSAtom*
 NumberToAtom(JSContext* cx, double d);
 
@@ -67,6 +70,9 @@ template <AllowGC allowGC>
 extern JSFlatString*
 Int32ToString(JSContext* cx, int32_t i);
 
+extern JSFlatString*
+Int32ToStringHelper(JSContext* cx, int32_t i);
+
 extern JSAtom*
 Int32ToAtom(JSContext* cx, int32_t si);