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 291269 8ce603f20bac14b9bf0a34da4a17c65c83bafb66
parent 291268 31898058e49e7d7d6e19666e4d24d0759f983fc3
child 291270 326505923c0f5d80ededec90e7b7e431cf1ed065
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer
bugs1239075
milestone48.0a1
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)
 {