Bug 1105187 - Do not sink effectful instructions. r=sunfish
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Fri, 28 Nov 2014 14:03:18 +0100
changeset 217958 f41f1f33b1b3
parent 217957 3f0f35b7204b
child 217995 f3600a81b069
push id52416
push usernpierron@mozilla.com
push date2014-11-28 13:03 +0000
treeherdermozilla-inbound@f41f1f33b1b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs1105187
milestone36.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 1105187 - Do not sink effectful instructions. r=sunfish
js/src/jit-test/tests/ion/bug1105187-sink.js
js/src/jit/MIR.cpp
js/src/jit/MIR.h
js/src/jit/Sink.cpp
js/src/tests/lib/jittests.py
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1105187-sink.js
@@ -0,0 +1,9 @@
+// |jit-test| --ion-gvn=off; error:ReferenceError
+
+(function(x) {
+    x = +x
+    switch (y) {
+        case -1:
+            x = 0
+    }
+})()
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -288,16 +288,24 @@ MInstruction::moveResumePointAsEntry()
 {
     MOZ_ASSERT(isNop());
     block()->clearEntryResumePoint();
     block()->setEntryResumePoint(resumePoint_);
     resumePoint_->resetInstruction();
     resumePoint_ = nullptr;
 }
 
+void
+MInstruction::clearResumePoint()
+{
+    resumePoint_->resetInstruction();
+    block()->discardPreAllocatedResumePoint(resumePoint_);
+    resumePoint_ = nullptr;
+}
+
 static bool
 MaybeEmulatesUndefined(MDefinition *op)
 {
     if (!op->mightBeType(MIRType_Object))
         return false;
 
     types::TemporaryTypeSet *types = op->resultTypeSet();
     if (!types)
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -868,16 +868,17 @@ class MInstruction
     // not use any MIRType_Value.
     MDefinition *foldsToStoredValue(TempAllocator &alloc, MDefinition *loaded);
 
     void setResumePoint(MResumePoint *resumePoint);
 
     // Used to transfer the resume point to the rewritten instruction.
     void stealResumePoint(MInstruction *ins);
     void moveResumePointAsEntry();
+    void clearResumePoint();
     MResumePoint *resumePoint() const {
         return resumePoint_;
     }
 
     // For instructions which can be cloned with new inputs, with all other
     // information being the same. clone() implementations do not need to worry
     // about cloning generic MInstruction/MDefinition state like flags and
     // resume points.
--- a/js/src/jit/Sink.cpp
+++ b/js/src/jit/Sink.cpp
@@ -54,16 +54,22 @@ Sink(MIRGenerator *mir, MIRGraph &graph)
         for (MInstructionReverseIterator iter = block->rbegin(); iter != block->rend(); ) {
             MInstruction *ins = *iter++;
 
             // Only instructions which can be recovered on bailout can be moved
             // into the bailout paths.
             if (ins->isGuard() || ins->isRecoveredOnBailout() || !ins->canRecoverOnBailout())
                 continue;
 
+            // To move effectful instruction, we would have to verify that the
+            // side-effect is not observed. In the mean time, we just inhibit
+            // this optimization on effectful instructions.
+            if (ins->isEffectful())
+                continue;
+
             // Compute a common dominator for all uses of the current
             // instruction.
             bool hasLiveUses = false;
             bool hasUses = false;
             MBasicBlock *usesDominator = nullptr;
             for (MUseIterator i(ins->usesBegin()), e(ins->usesEnd()); i != e; i++) {
                 hasUses = true;
                 MNode *consumerNode = (*i)->consumer();
@@ -182,16 +188,22 @@ Sink(MIRGenerator *mir, MIRGraph &graph)
                     (!consumer->isResumePoint() || consumer->toResumePoint() != entry))
                 {
                     continue;
                 }
 
                 use->replaceProducer(clone);
             }
 
+            // As we move this instruction in a different block, we should
+            // verify that we do not carry over a resume point which would refer
+            // to an outdated state of the control flow.
+            if (ins->resumePoint())
+                ins->clearResumePoint();
+
             // Now, that all uses which are not dominated by usesDominator are
             // using the cloned instruction, we can safely move the instruction
             // into the usesDominator block.
             MInstruction *at = usesDominator->safeInsertTop(nullptr, MBasicBlock::IgnoreRecover);
             block->moveBefore(at, ins);
         }
     }
 
--- a/js/src/tests/lib/jittests.py
+++ b/js/src/tests/lib/jittests.py
@@ -172,20 +172,20 @@ class Test:
                     elif name == 'allow-overrecursed':
                         test.allow_overrecursed = True
                     elif name == 'valgrind':
                         test.valgrind = options.valgrind
                     elif name == 'tz-pacific':
                         test.tz_pacific = True
                     elif name == 'ion-eager':
                         test.jitflags.append('--ion-eager')
-                    elif name == 'no-ion':
-                        test.jitflags.append('--no-ion')
                     elif name == 'dump-bytecode':
                         test.jitflags.append('--dump-bytecode')
+                    elif name.startswith('--'): # // |jit-test| --ion-gvn=off; --no-sse4
+                        test.jitflags.append(name)
                     else:
                         print('%s: warning: unrecognized |jit-test| attribute %s' % (path, part))
 
         if options.valgrind_all:
             test.valgrind = True
 
         return test