416 lines
14 KiB
Diff
416 lines
14 KiB
Diff
From 187b93c6acb2340749b1586bd12afac1d619b136 Mon Sep 17 00:00:00 2001
|
|
From: Jan de Mooij <jdemooij@mozilla.com>
|
|
Date: Tue, 4 Feb 2020 14:18:11 +0000
|
|
Subject: [PATCH] Bug 1608256 - Remove MakeMRegExpHoistable optimization
|
|
.
|
|
r=tcampbell
|
|
|
|
It's a lot of code and doesn't seem to affect Octane-regexp.
|
|
|
|
Differential Revision: https://phabricator.services.mozilla.com/D61427
|
|
|
|
--HG--
|
|
extra : moz-landing-system : lando
|
|
---
|
|
js/src/jit/Ion.cpp | 8 -
|
|
js/src/jit/IonAnalysis.cpp | 267 --------------------------------
|
|
js/src/jit/IonAnalysis.h | 3 -
|
|
js/src/jit/Lowering.cpp | 11 +-
|
|
js/src/jit/MIR.h | 8 -
|
|
js/src/vm/CommonPropertyNames.h | 6 -
|
|
6 files changed, 3 insertions(+), 300 deletions(-)
|
|
|
|
diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp
|
|
index fd31413..08d49bd 100644
|
|
--- a/js/src/jit/Ion.cpp
|
|
+++ b/js/src/jit/Ion.cpp
|
|
@@ -1361,14 +1361,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 fd89546..217a4c5 100644
|
|
--- a/js/src/jit/IonAnalysis.cpp
|
|
+++ b/js/src/jit/IonAnalysis.cpp
|
|
@@ -2033,273 +2033,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(CompileRuntime* runtime, 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.
|
|
- 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 = mir->runtime;
|
|
- if (propIdVal.toString() == runtime->names().lastIndex)
|
|
- continue;
|
|
- }
|
|
- }
|
|
- }
|
|
- }
|
|
- // MCall is safe only for some known safe functions.
|
|
- else if (useDef->isCall()) {
|
|
- if (IsRegExpHoistableCall(mir->runtime, 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 512c6bd..d49783c 100644
|
|
--- a/js/src/jit/IonAnalysis.h
|
|
+++ b/js/src/jit/IonAnalysis.h
|
|
@@ -59,9 +59,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 3b52ec5..2ba15a6 100644
|
|
--- a/js/src/jit/Lowering.cpp
|
|
+++ b/js/src/jit/Lowering.cpp
|
|
@@ -2413,14 +2413,9 @@ LIRGenerator::visitToObjectOrNull(MToObjectOrNull* ins)
|
|
void
|
|
LIRGenerator::visitRegExp(MRegExp* ins)
|
|
{
|
|
- if (ins->mustClone()) {
|
|
- LRegExp* lir = new(alloc()) LRegExp(temp());
|
|
- define(lir, ins);
|
|
- assignSafepoint(lir, ins);
|
|
- } else {
|
|
- RegExpObject* source = ins->source();
|
|
- define(new(alloc()) LPointer(source), ins);
|
|
- }
|
|
+ LRegExp* lir = new(alloc()) LRegExp(temp());
|
|
+ define(lir, ins);
|
|
+ assignSafepoint(lir, ins);
|
|
}
|
|
|
|
void
|
|
diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h
|
|
index e90b500..4a97c41 100644
|
|
--- a/js/src/jit/MIR.h
|
|
+++ b/js/src/jit/MIR.h
|
|
@@ -8605,14 +8605,12 @@ class MDefFun
|
|
class MRegExp : public MNullaryInstruction
|
|
{
|
|
CompilerGCPointer<RegExpObject*> source_;
|
|
- bool mustClone_;
|
|
bool hasShared_;
|
|
|
|
MRegExp(TempAllocator& alloc, CompilerConstraintList* constraints, RegExpObject* source,
|
|
bool hasShared)
|
|
: MNullaryInstruction(classOpcode),
|
|
source_(source),
|
|
- mustClone_(true),
|
|
hasShared_(hasShared)
|
|
{
|
|
setResultType(MIRType::Object);
|
|
@@ -8623,12 +8621,6 @@ class MRegExp : public MNullaryInstruction
|
|
INSTRUCTION_HEADER(RegExp)
|
|
TRIVIAL_NEW_WRAPPERS_WITH_ALLOC
|
|
|
|
- void setDoNotClone() {
|
|
- mustClone_ = false;
|
|
- }
|
|
- bool mustClone() const {
|
|
- return mustClone_;
|
|
- }
|
|
bool hasShared() const {
|
|
return hasShared_;
|
|
}
|
|
diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h
|
|
index 0d537fb..3d2eeb5 100644
|
|
--- a/js/src/vm/CommonPropertyNames.h
|
|
+++ b/js/src/vm/CommonPropertyNames.h
|
|
@@ -346,12 +346,7 @@
|
|
"ReadableStreamDefaultReader_releaseLock") \
|
|
macro(ReadableStreamTee, ReadableStreamTee, "ReadableStreamTee") \
|
|
macro(reason, reason, "reason") \
|
|
- macro(RegExpBuiltinExec, RegExpBuiltinExec, "RegExpBuiltinExec") \
|
|
macro(RegExpFlagsGetter, RegExpFlagsGetter, "RegExpFlagsGetter") \
|
|
- macro(RegExpMatcher, RegExpMatcher, "RegExpMatcher") \
|
|
- macro(RegExpSearcher, RegExpSearcher, "RegExpSearcher") \
|
|
- 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") \
|
|
@@ -429,7 +424,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") \
|
|
--
|
|
2.23.0
|
|
|