From a7a359647d502e98f4cba90eafecbdde769c54f6 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 9 Aug 2023 07:22:23 +0000 Subject: [PATCH] Bug 1847714 part 3 - Tidy up match result code more. r=iain * Use `initSlot` instead of `setSlot` because it's a little faster. * Use default `undefined` value for template object slots. * Remove some unnecessary stores in match stub code. * Use named constants instead of 0 and 1 in `BuildFlatMatchArray`. * The template array object can be an empty array now. Differential Revision: https://phabricator.services.mozilla.com/D185652 --- js/src/builtin/RegExp.cpp | 26 +++++++++++++------------- js/src/builtin/String.cpp | 8 ++++---- js/src/jit/CodeGenerator.cpp | 25 +++++-------------------- js/src/vm/RegExpObject.cpp | 23 ++++++++--------------- 4 files changed, 30 insertions(+), 52 deletions(-) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index 04690d714453..4526d47c9cda 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -154,10 +154,8 @@ bool js::CreateRegExpMatchResult(JSContext* cx, HandleRegExpShared re, if (!indicesGroups) { return false; } - indices->setSlot(RegExpRealm::IndicesGroupsSlot, - ObjectValue(*indicesGroups)); - } else { - indices->setSlot(RegExpRealm::IndicesGroupsSlot, UndefinedValue()); + indices->initSlot(RegExpRealm::IndicesGroupsSlot, + ObjectValue(*indicesGroups)); } // MakeIndicesArray: step 13 a-d. (Step 13.e is implemented below.) @@ -231,34 +229,36 @@ bool js::CreateRegExpMatchResult(JSContext* cx, HandleRegExpShared re, } else { for (uint32_t i = 0; i < re->numNamedCaptures(); i++) { uint32_t idx = re->getNamedCaptureIndex(i); - groups->setSlot(i, arr->getDenseElement(idx)); + groups->initSlot(i, arr->getDenseElement(idx)); // MakeIndicesArray: Step 13.e (reordered) if (hasIndices) { - indicesGroups->setSlot(i, indices->getDenseElement(idx)); + indicesGroups->initSlot(i, indices->getDenseElement(idx)); } } } // Step 22 (reordered). // Set the |index| property. - arr->setSlot(RegExpRealm::MatchResultObjectIndexSlot, - Int32Value(matches[0].start)); + arr->initSlot(RegExpRealm::MatchResultObjectIndexSlot, + Int32Value(matches[0].start)); // Step 23 (reordered). // Set the |input| property. - arr->setSlot(RegExpRealm::MatchResultObjectInputSlot, StringValue(input)); + arr->initSlot(RegExpRealm::MatchResultObjectInputSlot, StringValue(input)); // Step 32 (reordered) // Set the |groups| property. - arr->setSlot(RegExpRealm::MatchResultObjectGroupsSlot, - groups ? ObjectValue(*groups) : UndefinedValue()); + if (groups) { + arr->initSlot(RegExpRealm::MatchResultObjectGroupsSlot, + ObjectValue(*groups)); + } // Step 34b // Set the |indices| property. if (re->hasIndices()) { - arr->setSlot(RegExpRealm::MatchResultObjectIndicesSlot, - ObjectValue(*indices)); + arr->initSlot(RegExpRealm::MatchResultObjectIndicesSlot, + ObjectValue(*indices)); } #ifdef DEBUG diff --git a/js/src/builtin/String.cpp b/js/src/builtin/String.cpp index bb8d09994454..a1e09bd85c63 100644 --- a/js/src/builtin/String.cpp +++ b/js/src/builtin/String.cpp @@ -4538,11 +4538,11 @@ static bool BuildFlatMatchArray(JSContext* cx, HandleString str, arr->setDenseInitializedLength(1); arr->initDenseElement(0, StringValue(pattern)); - // Set the |index| property. (TemplateObject positions it in slot 0). - arr->setSlot(0, Int32Value(match)); + // Set the |index| property. + arr->initSlot(RegExpRealm::MatchResultObjectIndexSlot, Int32Value(match)); - // Set the |input| property. (TemplateObject positions it in slot 1). - arr->setSlot(1, StringValue(str)); + // Set the |input| property. + arr->initSlot(RegExpRealm::MatchResultObjectInputSlot, StringValue(str)); #ifdef DEBUG RootedValue test(cx); diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index acddff0e6518..e2b7b9f701fc 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -2552,26 +2552,6 @@ static JitCode* GenerateRegExpMatchStubShared(JSContext* cx, masm.bind(&allocated); } - static_assert(RegExpRealm::MatchResultObjectIndexSlot == 0, - "First slot holds the 'index' property"); - static_assert(RegExpRealm::MatchResultObjectInputSlot == 1, - "Second slot holds the 'input' property"); - static_assert(RegExpRealm::MatchResultObjectGroupsSlot == 2, - "Third slot holds the 'groups' property"); - - // Initialize the slots of the result object with the dummy values - // defined in createMatchResultTemplateObject. - masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), temp2); - masm.storeValue( - nativeTemplateObj.getSlot(RegExpRealm::MatchResultObjectIndexSlot), - Address(temp2, RegExpRealm::offsetOfMatchResultObjectIndexSlot())); - masm.storeValue( - nativeTemplateObj.getSlot(RegExpRealm::MatchResultObjectInputSlot), - Address(temp2, RegExpRealm::offsetOfMatchResultObjectInputSlot())); - masm.storeValue( - nativeTemplateObj.getSlot(RegExpRealm::MatchResultObjectGroupsSlot), - Address(temp2, RegExpRealm::offsetOfMatchResultObjectGroupsSlot())); - // clang-format off /* * [SMDOC] Stack layout for the RegExpMatcher stub @@ -2731,6 +2711,11 @@ static JitCode* GenerateRegExpMatchStubShared(JSContext* cx, Address firstMatchPairLimitAddress( FramePointer, pairsVectorStartOffset + MatchPair::offsetOfLimit()); + static_assert(RegExpRealm::MatchResultObjectIndexSlot == 0, + "First slot holds the 'index' property"); + static_assert(RegExpRealm::MatchResultObjectInputSlot == 1, + "Second slot holds the 'input' property"); + masm.loadPtr(Address(object, NativeObject::offsetOfSlots()), temp2); masm.load32(firstMatchPairStartAddress, temp3); diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index ca3caf659359..5c7c7cd156e3 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -863,18 +863,15 @@ ArrayObject* RegExpRealm::createMatchResultTemplateObject( MOZ_ASSERT(!matchResultTemplateObjects_[kind]); /* Create template array object */ - Rooted templateObject( - cx, - NewDenseUnallocatedArray(cx, RegExpObject::MaxPairCount, TenuredObject)); + Rooted templateObject(cx, NewTenuredDenseEmptyArray(cx)); if (!templateObject) { return nullptr; } if (kind == ResultTemplateKind::Indices) { /* The |indices| array only has a |groups| property. */ - RootedValue groupsVal(cx, UndefinedValue()); if (!NativeDefineDataProperty(cx, templateObject, cx->names().groups, - groupsVal, JSPROP_ENUMERATE)) { + UndefinedHandleValue, JSPROP_ENUMERATE)) { return nullptr; } MOZ_ASSERT(templateObject->getLastProperty().slot() == IndicesGroupsSlot); @@ -884,27 +881,24 @@ ArrayObject* RegExpRealm::createMatchResultTemplateObject( } /* Set dummy index property */ - RootedValue index(cx, Int32Value(0)); - if (!NativeDefineDataProperty(cx, templateObject, cx->names().index, index, - JSPROP_ENUMERATE)) { + if (!NativeDefineDataProperty(cx, templateObject, cx->names().index, + UndefinedHandleValue, JSPROP_ENUMERATE)) { return nullptr; } MOZ_ASSERT(templateObject->getLastProperty().slot() == MatchResultObjectIndexSlot); /* Set dummy input property */ - RootedValue inputVal(cx, StringValue(cx->runtime()->emptyString)); - if (!NativeDefineDataProperty(cx, templateObject, cx->names().input, inputVal, - JSPROP_ENUMERATE)) { + if (!NativeDefineDataProperty(cx, templateObject, cx->names().input, + UndefinedHandleValue, JSPROP_ENUMERATE)) { return nullptr; } MOZ_ASSERT(templateObject->getLastProperty().slot() == MatchResultObjectInputSlot); /* Set dummy groups property */ - RootedValue groupsVal(cx, UndefinedValue()); if (!NativeDefineDataProperty(cx, templateObject, cx->names().groups, - groupsVal, JSPROP_ENUMERATE)) { + UndefinedHandleValue, JSPROP_ENUMERATE)) { return nullptr; } MOZ_ASSERT(templateObject->getLastProperty().slot() == @@ -912,9 +906,8 @@ ArrayObject* RegExpRealm::createMatchResultTemplateObject( if (kind == ResultTemplateKind::WithIndices) { /* Set dummy indices property */ - RootedValue indicesVal(cx, UndefinedValue()); if (!NativeDefineDataProperty(cx, templateObject, cx->names().indices, - indicesVal, JSPROP_ENUMERATE)) { + UndefinedHandleValue, JSPROP_ENUMERATE)) { return nullptr; } MOZ_ASSERT(templateObject->getLastProperty().slot() ==