Bug 1239075 - RangeAnalysis: Assume that all captured results are used in bailing branches. r=h4writer
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Fri, 01 Apr 2016 16:11:52 +0000
changeset 291210 8ce603f20bac14b9bf0a34da4a17c65c83bafb66
parent 291209 31898058e49e7d7d6e19666e4d24d0759f983fc3
child 291211 326505923c0f5d80ededec90e7b7e431cf1ed065
push id74516
push usernpierron@mozilla.com
push dateFri, 01 Apr 2016 16:12:12 +0000
treeherdermozilla-inbound@8ce603f20bac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer
bugs1239075
milestone48.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 1239075 - RangeAnalysis: Assume that all captured results are used in bailing branches. r=h4writer
js/src/jit-test/tests/ion/bug1239075.js
js/src/jit/RangeAnalysis.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1239075.js
@@ -0,0 +1,29 @@
+
+function g0() { with({}){}; }
+function f0(y, x) {
+    var a = y >>> 0;
+    a = a - 1 + 1;
+    g0(); // Capture the truncate result after the call.
+    var b = x / 2; // bailout.
+    return ~(a + b);
+}
+assertEq(f0(-1, 0), 0);
+assertEq(f0(-1, 1), 0);
+
+
+function g1() { with({}){}; }
+function f1(y, x) {
+    var a = y >>> 0;
+    a = a - 1 + 1;
+    g1(); // Capture the truncate result after the call.
+    var b = Math.pow(x / 2, x); // bailout.
+    return ~(a + b);
+}
+assertEq(f1(-1, 0), -1);
+assertEq(f1(-1, 1), 0);
+
+function f2(x) {
+    return ~(((~0 | 0) >>> 0 || 0) + Math.pow(Math.cos(x >>> 0), Math.atan2(0, x)))
+}
+assertEq(f2(0), -1);
+assertEq(f2(-9999), 0);
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -2867,19 +2867,19 @@ CloneForDeadBranches(TempAllocator& allo
     return true;
 }
 
 // Examine all the users of |candidate| and determine the most aggressive
 // truncate kind that satisfies all of them.
 static MDefinition::TruncateKind
 ComputeRequestedTruncateKind(MDefinition* candidate, bool* shouldClone)
 {
-    bool isCapturedResult = false;
-    bool isObservableResult = false;
-    bool isRecoverableResult = true;
+    bool isCapturedResult = false;   // Check if used by a recovered instruction or a resume point.
+    bool isObservableResult = false; // Check if it can be read from another frame.
+    bool isRecoverableResult = true; // Check if it can safely be reconstructed.
     bool hasUseRemoved = candidate->isUseRemoved();
 
     MDefinition::TruncateKind kind = MDefinition::Truncate;
     for (MUseIterator use(candidate->usesBegin()); use != candidate->usesEnd(); use++) {
         if (use->consumer()->isResumePoint()) {
             // Truncation is a destructive optimization, as such, we need to pay
             // attention to removed branches and prevent optimization
             // destructive optimizations if we have no alternative. (see
@@ -2909,40 +2909,42 @@ ComputeRequestedTruncateKind(MDefinition
     if (candidate->isGuard() || candidate->isGuardRangeBailouts())
         kind = Min(kind, MDefinition::TruncateAfterBailouts);
 
     // If the value naturally produces an int32 value (before bailout checks)
     // that needs no conversion, we don't have to worry about resume points
     // seeing truncated values.
     bool needsConversion = !candidate->range() || !candidate->range()->isInt32();
 
+    // If the instruction is explicitly truncated (not indirectly) by all its
+    // uses and if it has no removed uses, then we can safely encode its
+    // truncated result as part of the resume point operands.  This is safe,
+    // because even if we resume with a truncated double, the next baseline
+    // instruction operating on this instruction is going to be a no-op.
+    //
+    // Note, that if the result can be observed from another frame, then this
+    // optimization is not safe.
+    bool safeToConvert = kind == MDefinition::Truncate && !hasUseRemoved && !isObservableResult;
+
     // If the candidate instruction appears as operand of a resume point or a
     // recover instruction, and we have to truncate its result, then we might
     // have to either recover the result during the bailout, or avoid the
     // truncation.
-    if (isCapturedResult && needsConversion) {
-
-        // These optimizations are pointless if there are no removed uses or any
-        // resume point observing the result.  Not having any means that we know
-        // everything about where this results flows into.
-        if ((hasUseRemoved || (isObservableResult && isRecoverableResult)) &&
-            candidate->canRecoverOnBailout())
-        {
-            // The cloned instruction is expected to be used as a recover
-            // instruction.
+    if (isCapturedResult && needsConversion && !safeToConvert) {
+
+        // If the result can be recovered from all the resume points (not needed
+        // for iterating over the inlined frames), and this instruction can be
+        // recovered on bailout, then we can clone it and use the cloned
+        // instruction to encode the recover instruction.  Otherwise, we should
+        // keep the original result and bailout if the value is not in the int32
+        // range.
+        if (isRecoverableResult && candidate->canRecoverOnBailout())
             *shouldClone = true;
-
-        } else if (hasUseRemoved || isObservableResult) {
-            // 1. If uses are removed and we cannot recover the result, then we
-            // need to keep the expected result for dead branches.
-            //
-            // 2. If the result is observable and not recoverable, then the
-            // result might be read while the frame is on the stack.
+        else
             kind = Min(kind, MDefinition::TruncateAfterBailouts);
-        }
     }
 
     return kind;
 }
 
 static MDefinition::TruncateKind
 ComputeTruncateKind(MDefinition* candidate, bool* shouldClone)
 {