From e7514afc7c13516cdd56e8ffba4399c7c1c974ba Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 18 Mar 2020 13:31:19 +0100 Subject: [js] Remove pointless MakeMRegExpHoistable optimization. It's a lot of code with no measurable effect. --- js/src/jit/Ion.cpp | 8 -- js/src/jit/IonAnalysis.cpp | 268 ---------------------------------------- js/src/jit/IonAnalysis.h | 3 - js/src/jit/Lowering.cpp | 11 +- js/src/jit/MIR.h | 10 +- js/src/vm/CommonPropertyNames.h | 6 - 6 files changed, 4 insertions(+), 302 deletions(-) (limited to 'js') diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 9337f6150..aa0ba8e3d 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -1497,14 +1497,6 @@ OptimizeMIR(MIRGenerator* mir) if (mir->shouldCancel("Start")) return false; - if (!mir->compilingWasm()) { - if (!MakeMRegExpHoistable(mir, graph)) - return false; - - if (mir->shouldCancel("Make MRegExp Hoistable")) - return false; - } - gs.spewPass("BuildSSA"); AssertBasicGraphCoherency(graph); diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp index 8bad8ba9c..a78ef232c 100644 --- a/js/src/jit/IonAnalysis.cpp +++ b/js/src/jit/IonAnalysis.cpp @@ -1975,274 +1975,6 @@ jit::ApplyTypeInformation(MIRGenerator* mir, MIRGraph& graph) return true; } -// Check if `def` is only the N-th operand of `useDef`. -static inline size_t -IsExclusiveNthOperand(MDefinition* useDef, size_t n, MDefinition* def) -{ - uint32_t num = useDef->numOperands(); - if (n >= num || useDef->getOperand(n) != def) - return false; - - for (uint32_t i = 0; i < num; i++) { - if (i == n) - continue; - if (useDef->getOperand(i) == def) - return false; - } - - return true; -} - -static size_t -IsExclusiveThisArg(MCall* call, MDefinition* def) -{ - return IsExclusiveNthOperand(call, MCall::IndexOfThis(), def); -} - -static size_t -IsExclusiveFirstArg(MCall* call, MDefinition* def) -{ - return IsExclusiveNthOperand(call, MCall::IndexOfArgument(0), def); -} - -static bool -IsRegExpHoistableCall(MCall* call, MDefinition* def) -{ - if (call->isConstructing()) - return false; - - JSAtom* name; - if (WrappedFunction* fun = call->getSingleTarget()) { - if (!fun->isSelfHostedBuiltin()) - return false; - name = GetSelfHostedFunctionName(fun->rawJSFunction()); - } else { - MDefinition* funDef = call->getFunction(); - if (funDef->isDebugCheckSelfHosted()) - funDef = funDef->toDebugCheckSelfHosted()->input(); - if (funDef->isTypeBarrier()) - funDef = funDef->toTypeBarrier()->input(); - - if (!funDef->isCallGetIntrinsicValue()) - return false; - name = funDef->toCallGetIntrinsicValue()->name(); - } - - // Hoistable only if the RegExp is the first argument of RegExpBuiltinExec. - CompileRuntime* runtime = GetJitContext()->runtime; - if (name == runtime->names().RegExpBuiltinExec || - name == runtime->names().UnwrapAndCallRegExpBuiltinExec || - name == runtime->names().RegExpMatcher || - name == runtime->names().RegExpTester || - name == runtime->names().RegExpSearcher) - { - return IsExclusiveFirstArg(call, def); - } - - if (name == runtime->names().RegExp_prototype_Exec) - return IsExclusiveThisArg(call, def); - - return false; -} - -static bool -CanCompareRegExp(MCompare* compare, MDefinition* def) -{ - MDefinition* value; - if (compare->lhs() == def) { - value = compare->rhs(); - } else { - MOZ_ASSERT(compare->rhs() == def); - value = compare->lhs(); - } - - // Comparing two regexp that weren't cloned will give different result - // than if they were cloned. - if (value->mightBeType(MIRType::Object)) - return false; - - // Make sure @@toPrimitive is not called which could notice - // the difference between a not cloned/cloned regexp. - - JSOp op = compare->jsop(); - // Strict equality comparison won't invoke @@toPrimitive. - if (op == JSOP_STRICTEQ || op == JSOP_STRICTNE) - return true; - - if (op != JSOP_EQ && op != JSOP_NE) { - // Relational comparison always invoke @@toPrimitive. - MOZ_ASSERT(op == JSOP_GT || op == JSOP_GE || op == JSOP_LT || op == JSOP_LE); - return false; - } - - // Loose equality comparison can invoke @@toPrimitive. - if (value->mightBeType(MIRType::Boolean) || value->mightBeType(MIRType::String) || - value->mightBeType(MIRType::Int32) || - value->mightBeType(MIRType::Double) || value->mightBeType(MIRType::Float32) || - value->mightBeType(MIRType::Symbol)) - { - return false; - } - - return true; -} - -static inline void -SetNotInWorklist(MDefinitionVector& worklist) -{ - for (size_t i = 0; i < worklist.length(); i++) - worklist[i]->setNotInWorklist(); -} - -static bool -IsRegExpHoistable(MIRGenerator* mir, MDefinition* regexp, MDefinitionVector& worklist, - bool* hoistable) -{ - MOZ_ASSERT(worklist.length() == 0); - - if (!worklist.append(regexp)) - return false; - regexp->setInWorklist(); - - for (size_t i = 0; i < worklist.length(); i++) { - MDefinition* def = worklist[i]; - if (mir->shouldCancel("IsRegExpHoistable outer loop")) - return false; - - for (MUseIterator use = def->usesBegin(); use != def->usesEnd(); use++) { - if (mir->shouldCancel("IsRegExpHoistable inner loop")) - return false; - - // Ignore resume points. At this point all uses are listed. - // No DCE or GVN or something has happened. - if (use->consumer()->isResumePoint()) - continue; - - MDefinition* useDef = use->consumer()->toDefinition(); - - // Step through a few white-listed ops. - if (useDef->isPhi() || useDef->isFilterTypeSet() || useDef->isGuardShape()) { - if (useDef->isInWorklist()) - continue; - - if (!worklist.append(useDef)) - return false; - useDef->setInWorklist(); - continue; - } - - // Instructions that doesn't invoke unknown code that may modify - // RegExp instance or pass it to elsewhere. - if (useDef->isRegExpMatcher() || useDef->isRegExpTester() || - useDef->isRegExpSearcher()) - { - if (IsExclusiveNthOperand(useDef, 0, def)) - continue; - } else if (useDef->isLoadFixedSlot() || useDef->isTypeOf()) { - continue; - } else if (useDef->isCompare()) { - if (CanCompareRegExp(useDef->toCompare(), def)) - continue; - } - // Instructions that modifies `lastIndex` property. - else if (useDef->isStoreFixedSlot()) { - if (IsExclusiveNthOperand(useDef, 0, def)) { - MStoreFixedSlot* store = useDef->toStoreFixedSlot(); - if (store->slot() == RegExpObject::lastIndexSlot()) - continue; - } - } else if (useDef->isSetPropertyCache()) { - if (IsExclusiveNthOperand(useDef, 0, def)) { - MSetPropertyCache* setProp = useDef->toSetPropertyCache(); - if (setProp->idval()->isConstant()) { - Value propIdVal = setProp->idval()->toConstant()->toJSValue(); - if (propIdVal.isString()) { - CompileRuntime* runtime = GetJitContext()->runtime; - if (propIdVal.toString() == runtime->names().lastIndex) - continue; - } - } - } - } - // MCall is safe only for some known safe functions. - else if (useDef->isCall()) { - if (IsRegExpHoistableCall(useDef->toCall(), def)) - continue; - } - - // Everything else is unsafe. - SetNotInWorklist(worklist); - worklist.clear(); - *hoistable = false; - - return true; - } - } - - SetNotInWorklist(worklist); - worklist.clear(); - *hoistable = true; - return true; -} - -bool -jit::MakeMRegExpHoistable(MIRGenerator* mir, MIRGraph& graph) -{ - // If we are compiling try blocks, regular expressions may be observable - // from catch blocks (which Ion does not compile). For now just disable the - // pass in this case. - if (graph.hasTryBlock()) - return true; - - MDefinitionVector worklist(graph.alloc()); - - for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++) { - if (mir->shouldCancel("MakeMRegExpHoistable outer loop")) - return false; - - for (MDefinitionIterator iter(*block); iter; iter++) { - if (!*iter) - MOZ_CRASH("confirm bug 1263794."); - - if (mir->shouldCancel("MakeMRegExpHoistable inner loop")) - return false; - - if (!iter->isRegExp()) - continue; - - MRegExp* regexp = iter->toRegExp(); - - bool hoistable = false; - if (!IsRegExpHoistable(mir, regexp, worklist, &hoistable)) - return false; - - if (!hoistable) - continue; - - // Make MRegExp hoistable - regexp->setMovable(); - regexp->setDoNotClone(); - - // That would be incorrect for global/sticky, because lastIndex - // could be wrong. Therefore setting the lastIndex to 0. That is - // faster than a not movable regexp. - RegExpObject* source = regexp->source(); - if (source->sticky() || source->global()) { - if (!graph.alloc().ensureBallast()) - return false; - MConstant* zero = MConstant::New(graph.alloc(), Int32Value(0)); - regexp->block()->insertAfter(regexp, zero); - - MStoreFixedSlot* lastIndex = - MStoreFixedSlot::New(graph.alloc(), regexp, RegExpObject::lastIndexSlot(), zero); - regexp->block()->insertAfter(zero, lastIndex); - } - } - } - - return true; -} - void jit::RenumberBlocks(MIRGraph& graph) { diff --git a/js/src/jit/IonAnalysis.h b/js/src/jit/IonAnalysis.h index 49bc0b591..673c797bd 100644 --- a/js/src/jit/IonAnalysis.h +++ b/js/src/jit/IonAnalysis.h @@ -56,9 +56,6 @@ EliminateDeadCode(MIRGenerator* mir, MIRGraph& graph); MOZ_MUST_USE bool ApplyTypeInformation(MIRGenerator* mir, MIRGraph& graph); -MOZ_MUST_USE bool -MakeMRegExpHoistable(MIRGenerator* mir, MIRGraph& graph); - void RenumberBlocks(MIRGraph& graph); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 19266bae8..f9b0b2157 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -2297,14 +2297,9 @@ LIRGenerator::visitToObjectOrNull(MToObjectOrNull* ins) void LIRGenerator::visitRegExp(MRegExp* ins) { - if (ins->mustClone()) { - LRegExp* lir = new(alloc()) LRegExp(); - defineReturn(lir, ins); - assignSafepoint(lir, ins); - } else { - RegExpObject* source = ins->source(); - define(new(alloc()) LPointer(source), ins); - } + LRegExp* lir = new(alloc()) LRegExp(); + defineReturn(lir, ins); + assignSafepoint(lir, ins); } void diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index af0abc695..81662a9be 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -8102,11 +8102,9 @@ class MDefFun class MRegExp : public MNullaryInstruction { CompilerGCPointer source_; - bool mustClone_; MRegExp(CompilerConstraintList* constraints, RegExpObject* source) - : source_(source), - mustClone_(true) + : source_(source) { setResultType(MIRType::Object); setResultTypeSet(MakeSingletonTypeSet(constraints, source)); @@ -8116,12 +8114,6 @@ class MRegExp : public MNullaryInstruction INSTRUCTION_HEADER(RegExp) TRIVIAL_NEW_WRAPPERS - void setDoNotClone() { - mustClone_ = false; - } - bool mustClone() const { - return mustClone_; - } RegExpObject* source() const { return source_; } diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index 6a8afb56b..420ee7535 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -281,13 +281,8 @@ macro(proxy, proxy, "proxy") \ macro(raw, raw, "raw") \ macro(reason, reason, "reason") \ - macro(RegExpBuiltinExec, RegExpBuiltinExec, "RegExpBuiltinExec") \ macro(RegExpFlagsGetter, RegExpFlagsGetter, "RegExpFlagsGetter") \ - macro(RegExpMatcher, RegExpMatcher, "RegExpMatcher") \ - macro(RegExpSearcher, RegExpSearcher, "RegExpSearcher") \ macro(RegExpStringIterator, RegExpStringIterator, "RegExp String Iterator") \ - macro(RegExpTester, RegExpTester, "RegExpTester") \ - macro(RegExp_prototype_Exec, RegExp_prototype_Exec, "RegExp_prototype_Exec") \ macro(Reify, Reify, "Reify") \ macro(reject, reject, "reject") \ macro(rejected, rejected, "rejected") \ @@ -360,7 +355,6 @@ macro(uninitialized, uninitialized, "uninitialized") \ macro(unsized, unsized, "unsized") \ macro(unwatch, unwatch, "unwatch") \ - macro(UnwrapAndCallRegExpBuiltinExec, UnwrapAndCallRegExpBuiltinExec, "UnwrapAndCallRegExpBuiltinExec") \ macro(url, url, "url") \ macro(usage, usage, "usage") \ macro(useAsm, useAsm, "use asm") \ -- cgit v1.2.3