Bug 1063488 - Treat MaybeCopyElementsForWrite as possibly overwriting an object's elements pointer. r=jandem, a=lmandel
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 11 Sep 2014 09:07:20 -0600
changeset 224875 5d0669f6183263830ca56c07b99c502e8e61e6b2
parent 224874 6b686aa0a606be572e341853032a0b1e401909dc
child 224876 91d7c9c5b349f0f1924f23d4c8cd30926503c976
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lmandel
bugs1063488
milestone34.0a2
Bug 1063488 - Treat MaybeCopyElementsForWrite as possibly overwriting an object's elements pointer. r=jandem, a=lmandel
js/src/jit-test/tests/ion/bug1063488.js
js/src/jit/MIR.h
js/src/jit/shared/CodeGenerator-shared.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1063488.js
@@ -0,0 +1,16 @@
+
+function foo(a, b) {
+    var x = b[0];
+    for (var i = 0; i < 5; i++) {
+	a[i + 1] = 0;
+	x += b[i];
+    }
+    assertEq(x, 2);
+}
+function bar() {
+    for (var i = 0; i < 5; i++) {
+	var arr = [1,2,3,4,5,6];
+	foo(arr, arr);
+    }
+}
+bar();
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -740,16 +740,22 @@ class MDefinition : public MNode
     }
     virtual AliasSet getAliasSet() const {
         // Instructions are effectful by default.
         return AliasSet::Store(AliasSet::Any);
     }
     bool isEffectful() const {
         return getAliasSet().isStore();
     }
+#ifdef DEBUG
+    virtual bool needsResumePoint() const {
+        // Return whether this instruction should have its own resume point.
+        return isEffectful();
+    }
+#endif
     virtual bool mightAlias(const MDefinition *store) const {
         // Return whether this load may depend on the specified store, given
         // that the alias sets intersect. This may be refined to exclude
         // possible aliasing in cases where alias set flags are too imprecise.
         JS_ASSERT(!isEffectful() && store->isEffectful());
         JS_ASSERT(getAliasSet().flags() & store->getAliasSet().flags());
         return true;
     }
@@ -6763,25 +6769,25 @@ class MMaybeCopyElementsForWrite
 
     MDefinition *object() const {
         return getOperand(0);
     }
     bool congruentTo(const MDefinition *ins) const {
         return congruentIfOperandsEqual(ins);
     }
     AliasSet getAliasSet() const {
-        // This instruction can read and write to the elements' contents,
-        // in the same manner as MConvertElementsToDoubles. As with that
-        // instruction, this is safe to consolidate and freely reorder this
-        // instruction, though this must precede any loads of the object's
-        // elements pointer or writes to the object's elements. The latter
-        // property is ensured by chaining this with the object definition
-        // itself, in the same manner as MBoundsCheck.
-        return AliasSet::None();
-    }
+        return AliasSet::Store(AliasSet::ObjectFields);
+    }
+#ifdef DEBUG
+    bool needsResumePoint() const {
+        // This instruction is idempotent and does not change observable
+        // behavior, so does not need its own resume point.
+        return false;
+    }
+#endif
 
     TypePolicy *typePolicy() {
         return this;
     }
 };
 
 // Load the initialized length from an elements header.
 class MInitializedLength
--- a/js/src/jit/shared/CodeGenerator-shared.cpp
+++ b/js/src/jit/shared/CodeGenerator-shared.cpp
@@ -990,17 +990,17 @@ CodeGeneratorShared::callVM(const VMFunc
     // If we're calling a function with an out parameter type of double, make
     // sure we have an FPU.
     JS_ASSERT_IF(fun.outParam == Type_Double, GetIonContext()->runtime->jitSupportsFloatingPoint());
 
 #ifdef DEBUG
     if (ins->mirRaw()) {
         JS_ASSERT(ins->mirRaw()->isInstruction());
         MInstruction *mir = ins->mirRaw()->toInstruction();
-        JS_ASSERT_IF(mir->isEffectful(), mir->resumePoint());
+        JS_ASSERT_IF(mir->needsResumePoint(), mir->resumePoint());
     }
 #endif
 
 #ifdef JS_TRACE_LOGGING
     if (!emitTracelogStartEvent(TraceLogger::VM))
         return false;
 #endif