Better fix for bug 452884 (changeset badf4c84665f regressed) - don't emit guards rather than bailing out of comparisons (r=gal).
authorDavid Anderson <danderson@mozilla.com>
Thu, 04 Sep 2008 19:43:07 -0700
changeset 19050 577ccecbf4ad595b5c72cb8a8ac30d3e91a72cdd
parent 19049 f9e142284a5be1ceadc4c50238278fe8df989d43
child 19051 1bd566ad367ad5572d1e9fb32fb7eede00428aa4
push id1930
push usermrbkap@mozilla.com
push dateWed, 10 Sep 2008 06:40:47 +0000
treeherderautoland@ee61af1469cd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal
bugs452884
milestone1.9.1b1pre
Better fix for bug 452884 (changeset badf4c84665f regressed) - don't emit guards rather than bailing out of comparisons (r=gal).
js/src/jstracer.cpp
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -2933,20 +2933,16 @@ TraceRecorder::cmp(LOpcode op, int flags
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
     LIns* x;
     bool negate = !!(flags & CMP_NEGATE);
     bool cond;
     LIns* l_ins = get(&l);
     LIns* r_ins = get(&r);
 
-    /* Don't guard if the same path is always taken. */
-    if (isAnyConst(r_ins) && isAnyConst(l_ins))
-        return true;
-
     if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) {
         JS_ASSERT(!negate);
         LIns* args[] = { r_ins, l_ins };
         x = lir->ins1(LIR_i2f, lir->insCall(F_CompareStrings, args));
         x = lir->ins2i(op, x, 0);
         jsint result = js_CompareStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r));
         switch (op) {
           case LIR_flt:
@@ -3048,26 +3044,29 @@ TraceRecorder::cmp(LOpcode op, int flags
             JS_ASSERT(op == LIR_feq);
             cond = (l == r) ^ negate;
             break;
         }
     } else {
         ABORT_TRACE("unsupported operand types for cmp");
     }
 
-    if (flags & CMP_CASE) {
-        guard(cond, x, BRANCH_EXIT);
-        return true;
+    /* Don't guard if the same path is always taken. */
+    if (!(isAnyConst(r_ins) && isAnyConst(l_ins))) {
+        if (flags & CMP_CASE) {
+            guard(cond, x, BRANCH_EXIT);
+            return true;
+        }
+
+        /* The interpreter fuses comparisons and the following branch,
+           so we have to do that here as well. */
+        if (flags & CMP_TRY_BRANCH_AFTER_COND) 
+            fuseIf(cx->fp->regs->pc + 1, cond, x);
     }
 
-    /* The interpreter fuses comparisons and the following branch,
-       so we have to do that here as well. */
-    if (flags & CMP_TRY_BRANCH_AFTER_COND) 
-        fuseIf(cx->fp->regs->pc + 1, cond, x);
-
     /* We update the stack after the guard. This is safe since
        the guard bails out at the comparison and the interpreter
        will therefore re-execute the comparison. This way the
        value of the condition doesn't have to be calculated and
        saved on the stack in most cases. */
     set(&l, x);
     return true;
 }
@@ -3080,61 +3079,63 @@ TraceRecorder::equal(int flags)
 {
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
     bool negate = !!(flags & CMP_NEGATE);
 
     LIns* r_ins = get(&r);
     LIns* l_ins = get(&l);
 
-    /* Don't guard if the same path is always taken. */
-    if (isAnyConst(r_ins) && isAnyConst(l_ins))
-        return true;
-
     if (JSVAL_IS_STRING(l) && JSVAL_IS_STRING(r)) {
         LIns* args[] = { r_ins, l_ins };
         bool cond = js_EqualStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r)) ^ negate;
         LIns* x = lir->ins_eq0(lir->insCall(F_EqualStrings, args));
         if (!negate)
             x = lir->ins_eq0(x);
 
-        if (flags & CMP_CASE) {
-            guard(cond, x, BRANCH_EXIT);
-            return true;
+        /* Don't guard if the same path is always taken. */
+        if (!(isAnyConst(r_ins) && isAnyConst(l_ins))) {
+            if (flags & CMP_CASE) {
+                guard(cond, x, BRANCH_EXIT);
+                return true;
+            }
+
+            /* The interpreter fuses comparisons and the following branch,
+               so we have to do that here as well. */
+            if (flags & CMP_TRY_BRANCH_AFTER_COND)
+                fuseIf(cx->fp->regs->pc + 1, cond, x);
         }
 
-        /* The interpreter fuses comparisons and the following branch,
-           so we have to do that here as well. */
-        if (flags & CMP_TRY_BRANCH_AFTER_COND)
-            fuseIf(cx->fp->regs->pc + 1, cond, x);
-
         /* We update the stack after the guard. This is safe since
            the guard bails out at the comparison and the interpreter
            will therefore re-execute the comparison. This way the
            value of the condition doesn't have to be calculated and
            saved on the stack in most cases. */
         set(&l, x);
         return true;
     }
     if (JSVAL_IS_OBJECT(l) && JSVAL_IS_OBJECT(r)) {
         bool cond = (l == r) ^ negate;
         LIns* x = lir->ins2(LIR_eq, l_ins, r_ins);
         if (negate)
             x = lir->ins_eq0(x);
 
-        if (flags & CMP_CASE) {
-            guard(cond, x, BRANCH_EXIT);
-            return true;
+        /* Don't guard if the same path is always taken. */
+        if (!(isAnyConst(r_ins) && isAnyConst(l_ins))) {
+            if (flags & CMP_CASE) {
+                guard(cond, x, BRANCH_EXIT);
+                return true;
+            }
+
+            /* The interpreter fuses comparisons and the following branch,
+               so we have to do that here as well. */
+            if (flags & CMP_TRY_BRANCH_AFTER_COND)
+                fuseIf(cx->fp->regs->pc + 1, cond, x);
         }
 
-        /* The interpreter fuses comparisons and the following branch,
-           so we have to do that here as well. */
-        if (flags & CMP_TRY_BRANCH_AFTER_COND)
-            fuseIf(cx->fp->regs->pc + 1, cond, x);
-
         /* We update the stack after the guard. This is safe since
            the guard bails out at the comparison and the interpreter
            will therefore re-execute the comparison. This way the
            value of the condition doesn't have to be calculated and
            saved on the stack in most cases. */
         set(&l, x);
         return true;
     }