Bug 1346140 - Flatten external strings when creating dependent strings. r=jwalden, r=h4writer, a=lizzard
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 22 Mar 2017 15:47:21 +0100
changeset 379253 df0808040376f8e12f1d110cf8855c7666476291
parent 379252 f22cedfd2ec3cd89be02876552a6a7ceb3b4fafd
child 379254 fa31c6b8354559a6fad9390a843f0f3bc636f849
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden, h4writer, lizzard
bugs1346140
milestone53.0
Bug 1346140 - Flatten external strings when creating dependent strings. r=jwalden, r=h4writer, a=lizzard
js/src/jit/CodeGenerator.cpp
js/src/jit/MacroAssembler-inl.h
js/src/jit/MacroAssembler.h
js/src/vm/String-inl.h
js/src/vm/String.cpp
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -1176,17 +1176,17 @@ PrepareAndExecuteRegExp(JSContext* cx, M
         // execution finished successfully.
         masm.store32(Imm32(1), pairCountAddress);
         masm.store32(Imm32(-1), pairsVectorAddress);
         masm.computeEffectiveAddress(pairsVectorAddress, temp1);
         masm.storePtr(temp1, pairsPointerAddress);
     }
 
     // Check for a linear input string.
-    masm.branchIfRope(input, failure);
+    masm.branchIfRopeOrExternal(input, temp1, failure);
 
     // Get the RegExpShared for the RegExp.
     masm.loadPtr(Address(regexp, NativeObject::getFixedSlotOffset(RegExpObject::PRIVATE_SLOT)), temp1);
     masm.branchPtr(Assembler::Equal, temp1, ImmWord(0), failure);
 
     // ES6 21.2.2.2 step 2.
     // See RegExp.cpp ExecuteRegExp for more detail.
     {
@@ -7533,20 +7533,17 @@ CodeGenerator::visitSubstr(LSubstr* lir)
     // Zero length, return emptystring.
     masm.branchTest32(Assembler::NonZero, length, length, &nonZero);
     const JSAtomState& names = GetJitContext()->runtime->names();
     masm.movePtr(ImmGCPtr(names.empty), output);
     masm.jump(done);
 
     // Use slow path for ropes.
     masm.bind(&nonZero);
-    static_assert(JSString::ROPE_FLAGS == 0,
-                  "rope flags must be zero for (flags & TYPE_FLAGS_MASK) == 0 "
-                  "to be a valid is-rope check");
-    masm.branchTest32(Assembler::Zero, stringFlags, Imm32(JSString::TYPE_FLAGS_MASK), slowPath);
+    masm.branchIfRopeOrExternal(string, temp, slowPath);
 
     // Handle inlined strings by creating a FatInlineString.
     masm.branchTest32(Assembler::Zero, stringFlags, Imm32(JSString::INLINE_CHARS_BIT), &notInline);
     masm.newGCFatInlineString(output, temp, slowPath);
     masm.store32(length, Address(output, JSString::offsetOfLength()));
     Address stringStorage(string, JSInlineString::offsetOfInlineStorage());
     Address outputStorage(output, JSInlineString::offsetOfInlineStorage());
 
--- a/js/src/jit/MacroAssembler-inl.h
+++ b/js/src/jit/MacroAssembler-inl.h
@@ -391,16 +391,29 @@ void
 MacroAssembler::branchIfRope(Register str, Label* label)
 {
     Address flags(str, JSString::offsetOfFlags());
     static_assert(JSString::ROPE_FLAGS == 0, "Rope type flags must be 0");
     branchTest32(Assembler::Zero, flags, Imm32(JSString::TYPE_FLAGS_MASK), label);
 }
 
 void
+MacroAssembler::branchIfRopeOrExternal(Register str, Register temp, Label* label)
+{
+    Address flags(str, JSString::offsetOfFlags());
+    move32(Imm32(JSString::TYPE_FLAGS_MASK), temp);
+    and32(flags, temp);
+
+    static_assert(JSString::ROPE_FLAGS == 0, "Rope type flags must be 0");
+    branchTest32(Assembler::Zero, temp, temp, label);
+
+    branch32(Assembler::Equal, temp, Imm32(JSString::EXTERNAL_FLAGS), label);
+}
+
+void
 MacroAssembler::branchLatin1String(Register string, Label* label)
 {
     branchTest32(Assembler::NonZero, Address(string, JSString::offsetOfFlags()),
                  Imm32(JSString::LATIN1_CHARS_BIT), label);
 }
 
 void
 MacroAssembler::branchTwoByteString(Register string, Label* label)
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -1108,16 +1108,17 @@ class MacroAssembler : public MacroAssem
     // Branches to |label| if |reg| is false. |reg| should be a C++ bool.
     template <class L>
     inline void branchIfFalseBool(Register reg, L label);
 
     // Branches to |label| if |reg| is true. |reg| should be a C++ bool.
     inline void branchIfTrueBool(Register reg, Label* label);
 
     inline void branchIfRope(Register str, Label* label);
+    inline void branchIfRopeOrExternal(Register str, Register temp, Label* label);
 
     inline void branchLatin1String(Register string, Label* label);
     inline void branchTwoByteString(Register string, Label* label);
 
     inline void branchIfFunctionHasNoScript(Register fun, Label* label);
     inline void branchIfInterpreted(Register fun, Label* label);
 
     inline void branchFunctionKind(Condition cond, JSFunction::FunctionKind kind, Register fun,
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -176,16 +176,19 @@ JSDependentString::new_(js::ExclusiveCon
                      : JSInlineString::lengthFits<JS::Latin1Char>(length);
     if (useInline) {
         js::RootedLinearString base(cx, baseArg);
         return baseArg->hasLatin1Chars()
                ? js::NewInlineString<JS::Latin1Char>(cx, base, start, length)
                : js::NewInlineString<char16_t>(cx, base, start, length);
     }
 
+    if (baseArg->isExternal() && !baseArg->ensureFlat(cx->asJSContext()))
+        return nullptr;
+
     JSDependentString* str = static_cast<JSDependentString*>(js::Allocate<JSString, js::NoGC>(cx));
     if (str) {
         str->init(cx, baseArg, start, length);
         return str;
     }
 
     js::RootedLinearString base(cx, baseArg);
 
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -923,16 +923,19 @@ bool
 AutoStableStringChars::init(JSContext* cx, JSString* s)
 {
     RootedLinearString linearString(cx, s->ensureLinear(cx));
     if (!linearString)
         return false;
 
     MOZ_ASSERT(state_ == Uninitialized);
 
+    if (linearString->isExternal() && !linearString->ensureFlat(cx))
+        return false;
+
     // If the chars are inline then we need to copy them since they may be moved
     // by a compacting GC.
     if (baseIsInline(linearString)) {
         return linearString->hasTwoByteChars() ? copyTwoByteChars(cx, linearString)
                                                : copyLatin1Chars(cx, linearString);
     }
 
     if (linearString->hasLatin1Chars()) {
@@ -954,16 +957,19 @@ AutoStableStringChars::initTwoByte(JSCon
     if (!linearString)
         return false;
 
     MOZ_ASSERT(state_ == Uninitialized);
 
     if (linearString->hasLatin1Chars())
         return copyAndInflateLatin1Chars(cx, linearString);
 
+    if (linearString->isExternal() && !linearString->ensureFlat(cx))
+        return false;
+
     // If the chars are inline then we need to copy them since they may be moved
     // by a compacting GC.
     if (baseIsInline(linearString))
         return copyTwoByteChars(cx, linearString);
 
     state_ = TwoByte;
     twoByteChars_ = linearString->rawTwoByteChars();
     s_ = linearString;