Don't guard on constant decisions, which will always have the same path (bug 452884, r=gal).
authorDavid Anderson <danderson@mozilla.com>
Thu, 04 Sep 2008 14:44:59 -0700
changeset 19044 badf4c84665f9e10b7ddb460eb6b6daa29099ec9
parent 19043 50e80ff981f71a98f16db4ac73fd61bbe66e9bd8
child 19045 57825218e42a6c592bce6166b93cff37828f25e6
child 19048 42478867792fc408be952eafdd2bea0c360c2ae5
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
Don't guard on constant decisions, which will always have the same path (bug 452884, r=gal).
js/src/jstracer.cpp
js/src/trace-test.js
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -362,16 +362,21 @@ static bool isPromote(LIns* i)
     return isPromoteInt(i) || isPromoteUint(i);
 }
 
 static bool isconst(LIns* i, int32_t c)
 {
     return i->isconst() && i->constval() == c;
 }
 
+static bool isAnyConst(LIns* i)
+{
+    return i->isconst() || i->isconstq(); 
+}
+
 static bool overflowSafe(LIns* i)
 {
     LIns* c;
     return (i->isop(LIR_and) && ((c = i->oprnd2())->isconst()) &&
             ((c->constval() & 0xc0000000) == 0)) ||
            (i->isop(LIR_rsh) && ((c = i->oprnd2())->isconst()) &&
             ((c->constval() > 0)));
 }
@@ -2785,19 +2790,25 @@ TraceRecorder::ifop()
               lir->ins_eq0(lir->ins2i(LIR_eq, get(&v), 1)),
               BRANCH_EXIT);
     } else if (JSVAL_IS_OBJECT(v)) {
         guard(JSVAL_IS_NULL(v), lir->ins_eq0(get(&v)), BRANCH_EXIT);
     } else if (isNumber(v)) {
         jsdouble d = asNumber(v);
         jsdpun u;
         u.d = 0;
-        guard(d == 0 || JSDOUBLE_IS_NaN(d),
-              lir->ins2(LIR_feq, get(&v), lir->insImmq(u.u64)),
-              BRANCH_EXIT);
+        LIns* v_ins = get(&v);
+        // Only insert the guard if the condition is not constant, since in 
+        // that case at runtime we would always take the same path as the
+        // interpreter is taking right now and hence there is no need for
+        // a guard.        
+        if (!v_ins->isconst() && !v_ins->isconstq())
+            guard(d == 0 || JSDOUBLE_IS_NaN(d),
+                  lir->ins2(LIR_feq, v_ins, lir->insImmq(u.u64)),
+                  BRANCH_EXIT);
     } else if (JSVAL_IS_STRING(v)) {
         guard(JSSTRING_LENGTH(JSVAL_TO_STRING(v)) == 0,
               lir->ins_eq0(lir->ins2(LIR_piand,
                                      lir->insLoad(LIR_ldp, 
                                                   get(&v), 
                                                   (int)offsetof(JSString, length)),
                                      INS_CONSTPTR(JSSTRING_LENGTH_MASK))),
               BRANCH_EXIT);
@@ -2919,19 +2930,26 @@ TraceRecorder::incElem(jsint incr, bool 
 bool
 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[] = { get(&r), get(&l) };
+        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:
             cond = result < 0;
             break;
           case LIR_fgt:
@@ -2947,18 +2965,16 @@ TraceRecorder::cmp(LOpcode op, int flags
             JS_NOT_REACHED("unexpected comparison op for strings");
             return false;
         }
     } else if (isNumber(l) || isNumber(r)) {
         jsval tmp[2] = {l, r};
         JSAutoTempValueRooter tvr(cx, 2, tmp);
 
         // TODO: coerce non-numbers to numbers if it's not string-on-string above
-        LIns* l_ins = get(&l);
-        LIns* r_ins = get(&r);
         jsdouble lnum;
         jsdouble rnum;
         LIns* args[] = { l_ins, cx_ins };
         if (JSVAL_IS_STRING(l)) {
             l_ins = lir->insCall(F_StringToNumber, args);
         } else if (JSVAL_TAG(l) == JSVAL_BOOLEAN) {
             /*
              * What I really want here is for undefined to be type-specialized
@@ -2968,17 +2984,17 @@ TraceRecorder::cmp(LOpcode op, int flags
              * without cmov.  Failing that, eat flaming builtin!
              */
             l_ins = lir->insCall(F_BooleanToNumber, args);
         } else if (!isNumber(l)) {
             ABORT_TRACE("unsupported LHS type for cmp vs number");
         }
         lnum = js_ValueToNumber(cx, &tmp[0]);
 
-        args[0] = get(&r);
+        args[0] = r_ins;
         args[1] = cx_ins;
         if (JSVAL_IS_STRING(r)) {
             r_ins = lir->insCall(F_StringToNumber, args);
         } else if (JSVAL_TAG(r) == JSVAL_BOOLEAN) {
             // See above for the sob story.
             r_ins = lir->insCall(F_BooleanToNumber, args);
         } else if (!isNumber(r)) {
             ABORT_TRACE("unsupported RHS type for cmp vs number");
@@ -3060,18 +3076,26 @@ TraceRecorder::cmp(LOpcode op, int flags
 // JSOP_NE we ever evolve to handle conversions then we must insist on like
 // "types" here (care required for 0 == -1, e.g.).
 bool
 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[] = { get(&r), get(&l) };
+        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;
@@ -3087,17 +3111,17 @@ TraceRecorder::equal(int flags)
            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, get(&l), get(&r));
+        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;
         }
 
--- a/js/src/trace-test.js
+++ b/js/src/trace-test.js
@@ -1180,11 +1180,27 @@ test(testNegZero1);
 
 // No test case, just make sure this doesn't assert. 
 function testNegZero2() {
     var z = 0;
     for (let j = 0; j < 5; ++j) { ({p: (-z)}); }
 }
 testNegZero2();
 
+function testConstSwitch() {
+    var x;
+    for (var j=0;j<5;++j) { switch(1.1) { case NaN: case 2: } x = 2; }
+    return x;
+}
+testConstSwitch.expected = 2;
+test(testConstSwitch);
+
+function testConstIf() {
+    var x;
+    for (var j=0;j<5;++j) { if (1.1 || 5) { } x = 2;}
+    return x;
+}
+testConstIf.expected = 2;
+test(testConstIf);
+
 /* Keep these at the end so that we can see the summary after the trace-debug spew. */
 print("\npassed:", passes.length && passes.join(","));
 print("\nFAILED:", fails.length && fails.join(","));