Bug 945512: IonMonkey: Make regexp only hoistable after analysis it is safe, r=sstangl
authorHannes Verschore <hv1989@gmail.com>
Wed, 09 Apr 2014 13:16:20 +0200
changeset 197389 ae1ae5ef4cc1ac7351140b7275e4b29247778da7
parent 197388 22feaebc0b2c5b355405d0692ab83921a9e0a156
child 197390 5e5711c0cc1d0d32381db8680e9b473b859c6e3f
push id486
push userasasaki@mozilla.com
push dateMon, 14 Jul 2014 18:39:42 +0000
treeherdermozilla-release@d33428174ff1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstangl
bugs945512
milestone31.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 945512: IonMonkey: Make regexp only hoistable after analysis it is safe, r=sstangl
js/src/jit-test/tests/ion/bug945512.js
js/src/jit/Ion.cpp
js/src/jit/IonAnalysis.cpp
js/src/jit/IonAnalysis.h
js/src/jit/IonBuilder.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug945512.js
@@ -0,0 +1,12 @@
+
+var handler = {
+    has: function (name) {
+        assertEq(1, 2);
+    }
+};
+
+for (var i=0; i<10; i++) {
+    var regex = /undefined/;
+    regex.__proto__ = Proxy.createFunction(handler, function(){})
+}
+
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1251,16 +1251,21 @@ jit::ToggleBarriers(JS::Zone *zone, bool
 namespace js {
 namespace jit {
 
 bool
 OptimizeMIR(MIRGenerator *mir)
 {
     MIRGraph &graph = mir->graph();
 
+    if (!mir->compilingAsmJS()) {
+        if (!MakeMRegExpHoistable(graph))
+            return false;
+    }
+
     IonSpewPass("BuildSSA");
     AssertBasicGraphCoherency(graph);
 
     if (mir->shouldCancel("Start"))
         return false;
 
     if (!SplitCriticalEdges(graph))
         return false;
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -1005,16 +1005,73 @@ jit::ApplyTypeInformation(MIRGenerator *
 
     if (!analyzer.analyze())
         return false;
 
     return true;
 }
 
 bool
+jit::MakeMRegExpHoistable(MIRGraph &graph)
+{
+    for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++) {
+        for (MDefinitionIterator iter(*block); iter; iter++) {
+            if (!iter->isRegExp())
+                continue;
+
+            MRegExp *regexp = iter->toRegExp();
+
+            // Test if MRegExp is hoistable by looking at all uses.
+            bool hoistable = true;
+            for (MUseIterator i = regexp->usesBegin(); i != regexp->usesEnd(); i++) {
+                // Ignore resume points. At this point all uses are listed.
+                // No DCE or GVN or something has happened.
+                if (i->consumer()->isResumePoint())
+                    continue;
+
+                JS_ASSERT(i->consumer()->isDefinition());
+
+                // All MRegExp* MIR's don't adjust the regexp.
+                MDefinition *use = i->consumer()->toDefinition();
+                if (use->isRegExpReplace())
+                    continue;
+                if (use->isRegExpExec())
+                    continue;
+                if (use->isRegExpTest())
+                    continue;
+
+                hoistable = false;
+                break;
+            }
+
+            if (!hoistable)
+                continue;
+
+            // Make MRegExp hoistable
+            regexp->setMovable();
+
+            // 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()) {
+                JS_ASSERT(regexp->mustClone());
+                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;
+}
+
+bool
 jit::RenumberBlocks(MIRGraph &graph)
 {
     size_t id = 0;
     for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++)
         block->setId(id++);
 
     return true;
 }
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -34,16 +34,19 @@ EliminateDeadResumePointOperands(MIRGene
 
 bool
 EliminateDeadCode(MIRGenerator *mir, MIRGraph &graph);
 
 bool
 ApplyTypeInformation(MIRGenerator *mir, MIRGraph &graph);
 
 bool
+MakeMRegExpHoistable(MIRGraph &graph);
+
+bool
 RenumberBlocks(MIRGraph &graph);
 
 bool
 BuildDominatorTree(MIRGraph &graph);
 
 bool
 BuildPhiReverseMapping(MIRGraph &graph);
 
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -9394,31 +9394,16 @@ IonBuilder::jsop_regexp(RegExpObject *re
         if (!reobj->global() && !reobj->sticky())
             mustClone = false;
     }
 
     MRegExp *regexp = MRegExp::New(alloc(), constraints(), reobj, mustClone);
     current->add(regexp);
     current->push(regexp);
 
-    regexp->setMovable();
-
-    // The MRegExp is set to be movable.
-    // That would be incorrect for global/sticky, because lastIndex could be wrong.
-    // Therefore setting the lastIndex to 0. That is faster than removing the movable flag.
-    if (reobj->sticky() || reobj->global()) {
-        JS_ASSERT(mustClone);
-        MConstant *zero = MConstant::New(alloc(), Int32Value(0));
-        current->add(zero);
-
-        MStoreFixedSlot *lastIndex =
-            MStoreFixedSlot::New(alloc(), regexp, RegExpObject::lastIndexSlot(), zero);
-        current->add(lastIndex);
-    }
-
     return true;
 }
 
 bool
 IonBuilder::jsop_object(JSObject *obj)
 {
     if (options.cloneSingletons()) {
         MCloneLiteral *clone = MCloneLiteral::New(alloc(), constant(ObjectValue(*obj)));