Bug 1367088 - Part 2: Inline NewStringIterator in Ion. r=evilpie
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 25 May 2017 07:17:57 -0700
changeset 360868 0414ffab16f8a5566dea0b6bf17e5fef401d3ce5
parent 360867 d43cb547757f52677247a62dc353a6e46777cfee
child 360869 4769df59b9c21e7a37ec29b4a81cf4ae1ef6e6bb
push id31903
push userryanvm@gmail.com
push dateFri, 26 May 2017 19:44:10 +0000
treeherdermozilla-central@bce03a8eac30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1367088
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1367088 - Part 2: Inline NewStringIterator in Ion. r=evilpie
js/src/builtin/String.js
js/src/jit-test/tests/ion/recover-newstringiterator.js
js/src/jit/BaselineIC.cpp
js/src/jit/CodeGenerator.cpp
js/src/jit/InlinableNatives.h
js/src/jit/MCallOptimize.cpp
js/src/jit/MIR.h
js/src/jit/Recover.cpp
js/src/jit/VMFunctions.h
js/src/jsiter.cpp
js/src/jsiter.h
js/src/vm/SelfHosting.cpp
js/src/vm/SelfHosting.h
--- a/js/src/builtin/String.js
+++ b/js/src/builtin/String.js
@@ -551,25 +551,30 @@ function StringIteratorNext() {
 
     if (index >= size) {
         result.done = true;
         return result;
     }
 
     var charCount = 1;
     var first = callFunction(std_String_charCodeAt, S, index);
-    if (first >= 0xD800 && first <= 0xDBFF && index + 1 < size) {
-        var second = callFunction(std_String_charCodeAt, S, index + 1);
-        if (second >= 0xDC00 && second <= 0xDFFF) {
-            charCount = 2;
+    if (first <= 0xff) {
+        // String.fromCharCode has an optimized path for Latin-1 strings.
+        result.value = callFunction(std_String_fromCharCode, null, first);
+    } else {
+        if (first >= 0xD800 && first <= 0xDBFF && index + 1 < size) {
+            var second = callFunction(std_String_charCodeAt, S, index + 1);
+            if (second >= 0xDC00 && second <= 0xDFFF) {
+                charCount = 2;
+            }
         }
+        result.value = callFunction(String_substring, S, index, index + charCount);
     }
 
     UnsafeSetReservedSlot(this, ITERATOR_SLOT_NEXT_INDEX, index + charCount);
-    result.value = callFunction(String_substring, S, index, index + charCount);
 
     return result;
 }
 
 /**
  * Compare this String against that String, using the locale and collation
  * options provided.
  *
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/recover-newstringiterator.js
@@ -0,0 +1,63 @@
+var max = 40;
+setJitCompilerOption("ion.warmup.trigger", max - 10);
+
+function selfhosted() {
+    if (typeof getSelfHostedValue === "undefined")
+        return;
+
+    var NewStringIterator = getSelfHostedValue("NewStringIterator");
+    var iter = NewStringIterator();
+    bailout();
+    // assertRecoveredOnBailout(iter, true);
+}
+
+function iterator(i) {
+    var string = String.fromCharCode(0x41, i);
+    var iter = string[Symbol.iterator]();
+    assertEq(iter.next().value, 'A');
+    bailout();
+    // This sometimes fails
+    // assertRecoveredOnBailout(iter, true);
+    var result = iter.next();
+    assertEq(result.value, String.fromCharCode(i));
+    assertEq(result.done, false);
+    assertEq(iter.next().done, true);
+}
+
+function forof(i) {
+    var string = String.fromCharCode(0x41, i);
+    var first = true;
+
+    for (var x of string) {
+        if (first) {
+            assertEq(x, 'A');
+            bailout();
+            first = false;
+        } else {
+            assertEq(x, String.fromCharCode(i));
+        }
+    }
+}
+
+var data = {
+  a: 'foo',
+  b: {c: 'd'},
+  str: 'ABC'
+};
+
+function fn() {
+  var {a, b:{c:b}, str:[, c]} = data;
+  return c;
+}
+
+function destructuring() {
+    for (var i = 0; i < max; i++)
+        assertEq(fn(), 'B');
+}
+
+for (var i = 0; i < max; i++) {
+    selfhosted();
+    iterator(i);
+    forof(i);
+    destructuring();
+}
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -2013,16 +2013,21 @@ GetTemplateObjectForNative(JSContext* cx
         return !!res;
     }
 
     if (native == js::intrinsic_NewArrayIterator) {
         res.set(NewArrayIteratorObject(cx, TenuredObject));
         return !!res;
     }
 
+    if (native == js::intrinsic_NewStringIterator) {
+        res.set(NewStringIteratorObject(cx, TenuredObject));
+        return !!res;
+    }
+
     if (JitSupportsSimd() && GetTemplateObjectForSimd(cx, target, res))
        return !!res;
 
     return true;
 }
 
 static bool
 GetTemplateObjectForClassHook(JSContext* cx, JSNative hook, CallArgs& args,
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -5648,30 +5648,39 @@ CodeGenerator::visitNewArrayDynamicLengt
 
     masm.bind(ool->rejoin());
 }
 
 typedef ArrayIteratorObject* (*NewArrayIteratorObjectFn)(JSContext*, NewObjectKind);
 static const VMFunction NewArrayIteratorObjectInfo =
     FunctionInfo<NewArrayIteratorObjectFn>(NewArrayIteratorObject, "NewArrayIteratorObject");
 
+typedef StringIteratorObject* (*NewStringIteratorObjectFn)(JSContext*, NewObjectKind);
+static const VMFunction NewStringIteratorObjectInfo =
+    FunctionInfo<NewStringIteratorObjectFn>(NewStringIteratorObject, "NewStringIteratorObject");
+
 void
 CodeGenerator::visitNewIterator(LNewIterator* lir)
 {
     Register objReg = ToRegister(lir->output());
     Register tempReg = ToRegister(lir->temp());
     JSObject* templateObject = lir->mir()->templateObject();
 
     OutOfLineCode* ool;
     switch (lir->mir()->type()) {
       case MNewIterator::ArrayIterator:
         ool = oolCallVM(NewArrayIteratorObjectInfo, lir,
                         ArgList(Imm32(GenericObject)),
                         StoreRegisterTo(objReg));
         break;
+      case MNewIterator::StringIterator:
+        ool = oolCallVM(NewStringIteratorObjectInfo, lir,
+                        ArgList(Imm32(GenericObject)),
+                        StoreRegisterTo(objReg));
+        break;
       default:
           MOZ_CRASH("unexpected iterator type");
     }
 
     masm.createGCObject(objReg, tempReg, templateObject, gc::DefaultHeap, ool->entry());
 
     masm.bind(ool->rejoin());
 }
--- a/js/src/jit/InlinableNatives.h
+++ b/js/src/jit/InlinableNatives.h
@@ -125,16 +125,17 @@
     _(IntrinsicIsSetIterator)       \
     _(IntrinsicIsStringIterator)    \
                                     \
     _(IntrinsicGetNextMapEntryForIterator) \
                                     \
     _(IntrinsicGetNextSetEntryForIterator) \
                                     \
     _(IntrinsicNewArrayIterator)    \
+    _(IntrinsicNewStringIterator)   \
                                     \
     _(IntrinsicArrayBufferByteLength) \
     _(IntrinsicPossiblyWrappedArrayBufferByteLength) \
                                     \
     _(TypedArrayConstructor)        \
     _(IntrinsicIsTypedArray)        \
     _(IntrinsicIsPossiblyWrappedTypedArray) \
     _(IntrinsicTypedArrayLength)    \
--- a/js/src/jit/MCallOptimize.cpp
+++ b/js/src/jit/MCallOptimize.cpp
@@ -220,16 +220,18 @@ IonBuilder::inlineNativeCall(CallInfo& c
       case InlinableNative::StringCharAt:
         return inlineStrCharAt(callInfo);
 
       // String intrinsics.
       case InlinableNative::IntrinsicStringReplaceString:
         return inlineStringReplaceString(callInfo);
       case InlinableNative::IntrinsicStringSplitString:
         return inlineStringSplitString(callInfo);
+      case InlinableNative::IntrinsicNewStringIterator:
+        return inlineNewIterator(callInfo, MNewIterator::StringIterator);
 
       // Object natives.
       case InlinableNative::ObjectCreate:
         return inlineObjectCreate(callInfo);
 
       // SIMD natives.
       case InlinableNative::SimdInt32x4:
         return inlineSimd(callInfo, target, SimdType::Int32x4);
@@ -905,16 +907,20 @@ IonBuilder::inlineNewIterator(CallInfo& 
     }
 
     JSObject* templateObject = nullptr;
     switch (type) {
       case MNewIterator::ArrayIterator:
         templateObject = inspector->getTemplateObjectForNative(pc, js::intrinsic_NewArrayIterator);
         MOZ_ASSERT_IF(templateObject, templateObject->is<ArrayIteratorObject>());
         break;
+      case MNewIterator::StringIterator:
+        templateObject = inspector->getTemplateObjectForNative(pc, js::intrinsic_NewStringIterator);
+        MOZ_ASSERT_IF(templateObject, templateObject->is<StringIteratorObject>());
+        break;
     }
 
     if (!templateObject)
         return InliningStatus_NotInlined;
 
     callInfo.setImplicitlyUsedUnchecked();
 
     MConstant* templateConst = MConstant::NewConstraintlessObject(alloc(), templateObject);
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -3537,17 +3537,18 @@ class MNewObject
 
 
 class MNewIterator
   : public MUnaryInstruction,
     public NoTypePolicy::Data
 {
   public:
     enum Type {
-        ArrayIterator
+        ArrayIterator,
+        StringIterator,
     };
 
 private:
     Type type_;
 
     MNewIterator(CompilerConstraintList* constraints, MConstant* templateConst, Type type)
       : MUnaryInstruction(templateConst),
         type_(type)
--- a/js/src/jit/Recover.cpp
+++ b/js/src/jit/Recover.cpp
@@ -1386,16 +1386,19 @@ RNewIterator::recover(JSContext* cx, Sna
     RootedObject templateObject(cx, &iter.read().toObject());
     RootedValue result(cx);
 
     JSObject* resultObject = nullptr;
     switch (MNewIterator::Type(type_)) {
       case MNewIterator::ArrayIterator:
         resultObject = NewArrayIteratorObject(cx);
         break;
+      case MNewIterator::StringIterator:
+        resultObject = NewStringIteratorObject(cx);
+        break;
     }
 
     if (!resultObject)
         return false;
 
     result.setObject(*resultObject);
     iter.storeInstructionResult(result);
     return true;
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -278,16 +278,17 @@ template <> struct TypeToDataType<JSObje
 template <> struct TypeToDataType<NativeObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<PlainObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<InlineTypedObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<NamedLambdaObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<LexicalEnvironmentObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<ArrayObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<TypedArrayObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<ArrayIteratorObject*> { static const DataType result = Type_Object; };
+template <> struct TypeToDataType<StringIteratorObject*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<JSString*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<JSFlatString*> { static const DataType result = Type_Object; };
 template <> struct TypeToDataType<HandleObject> { static const DataType result = Type_Handle; };
 template <> struct TypeToDataType<HandleString> { static const DataType result = Type_Handle; };
 template <> struct TypeToDataType<HandlePropertyName> { static const DataType result = Type_Handle; };
 template <> struct TypeToDataType<HandleFunction> { static const DataType result = Type_Handle; };
 template <> struct TypeToDataType<Handle<NativeObject*> > { static const DataType result = Type_Handle; };
 template <> struct TypeToDataType<Handle<InlineTypedObject*> > { static const DataType result = Type_Handle; };
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -1170,16 +1170,26 @@ const Class StringIteratorObject::class_
     JSCLASS_HAS_RESERVED_SLOTS(StringIteratorSlotCount)
 };
 
 static const JSFunctionSpec string_iterator_methods[] = {
     JS_SELF_HOSTED_FN("next", "StringIteratorNext", 0, 0),
     JS_FS_END
 };
 
+StringIteratorObject*
+js::NewStringIteratorObject(JSContext* cx, NewObjectKind newKind)
+{
+    RootedObject proto(cx, GlobalObject::getOrCreateStringIteratorPrototype(cx, cx->global()));
+    if (!proto)
+        return nullptr;
+
+    return NewObjectWithGivenProto<StringIteratorObject>(cx, proto, newKind);
+}
+
 JSObject*
 js::ValueToIterator(JSContext* cx, unsigned flags, HandleValue vp)
 {
     /* JSITER_KEYVALUE must always come with JSITER_FOREACH */
     MOZ_ASSERT_IF(flags & JSITER_KEYVALUE, flags & JSITER_FOREACH);
 
     RootedObject obj(cx);
     if (vp.isObject()) {
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -149,16 +149,19 @@ ArrayIteratorObject*
 NewArrayIteratorObject(JSContext* cx, NewObjectKind newKind = GenericObject);
 
 class StringIteratorObject : public JSObject
 {
   public:
     static const Class class_;
 };
 
+StringIteratorObject*
+NewStringIteratorObject(JSContext* cx, NewObjectKind newKind = GenericObject);
+
 bool
 GetIterator(JSContext* cx, HandleObject obj, unsigned flags, MutableHandleObject objp);
 
 JSObject*
 GetIteratorObject(JSContext* cx, HandleObject obj, unsigned flags);
 
 /*
  * Creates either a key or value iterator, depending on flags. For a value
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -768,27 +768,23 @@ intrinsic_CreateSetIterationResult(JSCon
     JSObject* result = SetIteratorObject::createResult(cx);
     if (!result)
         return false;
 
     args.rval().setObject(*result);
     return true;
 }
 
-static bool
-intrinsic_NewStringIterator(JSContext* cx, unsigned argc, Value* vp)
+bool
+js::intrinsic_NewStringIterator(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 0);
 
-    RootedObject proto(cx, GlobalObject::getOrCreateStringIteratorPrototype(cx, cx->global()));
-    if (!proto)
-        return false;
-
-    JSObject* obj = NewObjectWithGivenProto(cx, &StringIteratorObject::class_, proto);
+    JSObject* obj = NewStringIteratorObject(cx);
     if (!obj)
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
 
 static bool
@@ -2403,17 +2399,18 @@ static const JSFunctionSpec intrinsic_fu
 
     JS_FN("_CreateSetIterationResult", intrinsic_CreateSetIterationResult, 0, 0),
     JS_INLINABLE_FN("_GetNextSetEntryForIterator", intrinsic_GetNextSetEntryForIterator, 2,0,
                     IntrinsicGetNextSetEntryForIterator),
     JS_FN("CallSetIteratorMethodIfWrapped",
           CallNonGenericSelfhostedMethod<Is<SetIteratorObject>>,        2,0),
 
 
-    JS_FN("NewStringIterator",       intrinsic_NewStringIterator,       0,0),
+    JS_INLINABLE_FN("NewStringIterator", intrinsic_NewStringIterator,   0,0,
+                    IntrinsicNewStringIterator),
     JS_FN("CallStringIteratorMethodIfWrapped",
           CallNonGenericSelfhostedMethod<Is<StringIteratorObject>>,     2,0),
 
     JS_FN("IsStarGeneratorObject",
           intrinsic_IsInstanceOfBuiltin<StarGeneratorObject>,           1,0),
     JS_FN("StarGeneratorObjectIsClosed", intrinsic_StarGeneratorObjectIsClosed, 1,0),
     JS_FN("IsSuspendedStarGenerator",intrinsic_IsSuspendedStarGenerator,1,0),
 
--- a/js/src/vm/SelfHosting.h
+++ b/js/src/vm/SelfHosting.h
@@ -45,11 +45,14 @@ CallSelfHostedFunction(JSContext* cx, Ha
                        const AnyInvokeArgs& args, MutableHandleValue rval);
 
 bool
 intrinsic_StringSplitString(JSContext* cx, unsigned argc, JS::Value* vp);
 
 bool
 intrinsic_NewArrayIterator(JSContext* cx, unsigned argc, JS::Value* vp);
 
+bool
+intrinsic_NewStringIterator(JSContext* cx, unsigned argc, JS::Value* vp);
+
 } /* namespace js */
 
 #endif /* vm_SelfHosting_h_ */