Bug 470133 - TM: fails to trace case with a type mismatch. r=gal
authorJeff Walden <jwalden@mit.edu>
Thu, 18 Dec 2008 10:35:09 -0800
changeset 23093 cc3cbf3ea6761a33ad74feb33c6c8fb6c8d0d62a
parent 23092 e6c176faad6bd208cd006056e7197abb20989679
child 23094 efd7691d238a2e50c7cb17469c9e196992a00504
push id4346
push userrsayre@mozilla.com
push dateFri, 26 Dec 2008 01:26:36 +0000
treeherdermozilla-central@8eb5a5b83a93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal
bugs470133
milestone1.9.2a1pre
Bug 470133 - TM: fails to trace case with a type mismatch. r=gal
js/src/jstracer.cpp
js/src/jstracer.h
js/src/trace-test.js
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -4535,50 +4535,60 @@ static bool
 evalCmp(LOpcode op, JSString* l, JSString* r)
 {
     if (op == LIR_feq)
         return js_EqualStrings(l, r);
     return evalCmp(op, js_CompareStrings(l, r));
 }
 
 JS_REQUIRES_STACK void
-TraceRecorder::strictEquality(bool equal)
+TraceRecorder::strictEquality(bool equal, bool cmpCase)
 {
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
     LIns* l_ins = get(&l);
     LIns* r_ins = get(&r);
 
     uint8 ltag = getPromotedType(l);
     if (ltag != getPromotedType(r)) {
         set(&l, lir->insImm(!equal));
         return;
     }
 
     LIns* x;
+    bool cond;
     if (ltag == JSVAL_STRING) {
         LIns* args[] = { r_ins, l_ins };
         x = lir->ins2i(LIR_eq, lir->insCall(&js_EqualStrings_ci, args), equal);
+        cond = js_EqualStrings(JSVAL_TO_STRING(l), JSVAL_TO_STRING(r));
     } else {
         LOpcode op = (ltag != JSVAL_DOUBLE) ? LIR_eq : LIR_feq;
         x = lir->ins2(op, l_ins, r_ins);
         if (!equal)
             x = lir->ins_eq0(x);
+        cond = (l == r);
+    }
+    cond = (cond == equal);
+
+    if (cmpCase) {
+        /* Only guard if the same path may not always be taken. */
+        if (!x->isconst())
+            guard(cond, x, BRANCH_EXIT);
+        return;
     }
 
     set(&l, x);
 }
 
 JS_REQUIRES_STACK bool
-TraceRecorder::equality(int flags)
+TraceRecorder::equality(bool negate, bool tryBranchAfterCond)
 {
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
     LIns* x = NULL;
-    bool negate = !!(flags & CMP_NEGATE);
     bool cond;
     LIns* l_ins = get(&l);
     LIns* r_ins = get(&r);
     bool fp = false;
 
     if (JSVAL_IS_STRING(l) || JSVAL_IS_STRING(r)) {
         // Comparing equality of a string against null always produces false.
         if ((JSVAL_IS_NULL(l) && l_ins->isconst()) ||
@@ -4651,43 +4661,36 @@ TraceRecorder::equality(int flags)
         LOpcode op = fp ? LIR_feq : LIR_eq;
         x = lir->ins2(op, l_ins, r_ins);
         if (negate) {
             x = lir->ins_eq0(x);
             cond = !cond;
         }
     }
 
-    if (flags & CMP_CASE) {
-        /* Only guard if the same path may not always be taken. */
-        if (!x->isconst())
-            guard(cond, x, BRANCH_EXIT);
-        return true;
-    }
-
     /*
      * Don't guard if the same path is always taken.  If it isn't, we have to
      * fuse comparisons and the following branch, because the interpreter does
      * that.
      */
-    if ((flags & CMP_TRY_BRANCH_AFTER_COND) && !x->isconst())
+    if (tryBranchAfterCond && !x->isconst())
         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;
 }
 
 JS_REQUIRES_STACK bool
-TraceRecorder::relational(LOpcode op, int flags)
+TraceRecorder::relational(LOpcode op, bool tryBranchAfterCond)
 {
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
     LIns* x = NULL;
     bool cond;
     LIns* l_ins = get(&l);
     LIns* r_ins = get(&r);
     bool fp = false;
@@ -4783,17 +4786,17 @@ TraceRecorder::relational(LOpcode op, in
     }
     x = lir->ins2(op, l_ins, r_ins);
 
     /*
      * Don't guard if the same path is always taken.  If it isn't, we have to
      * fuse comparisons and the following branch, because the interpreter does
      * that.
      */
-    if ((flags & CMP_TRY_BRANCH_AFTER_COND) && !x->isconst())
+    if (tryBranchAfterCond && !x->isconst())
         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.
      */
@@ -5605,47 +5608,47 @@ JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_BITAND()
 {
     return binary(LIR_and);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_EQ()
 {
-    return equality(CMP_TRY_BRANCH_AFTER_COND);
+    return equality(false, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_NE()
 {
-    return equality(CMP_NEGATE | CMP_TRY_BRANCH_AFTER_COND);
+    return equality(true, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_LT()
 {
-    return relational(LIR_flt, CMP_TRY_BRANCH_AFTER_COND);
+    return relational(LIR_flt, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_LE()
 {
-    return relational(LIR_fle, CMP_TRY_BRANCH_AFTER_COND);
+    return relational(LIR_fle, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_GT()
 {
-    return relational(LIR_fgt, CMP_TRY_BRANCH_AFTER_COND);
+    return relational(LIR_fgt, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_GE()
 {
-    return relational(LIR_fge, CMP_TRY_BRANCH_AFTER_COND);
+    return relational(LIR_fge, true);
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_LSH()
 {
     return binary(LIR_lsh);
 }
 
@@ -7080,24 +7083,24 @@ JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_LOOKUPSWITCH()
 {
     return switchop();
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_STRICTEQ()
 {
-    strictEquality(true);
+    strictEquality(true, false);
     return true;
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_STRICTNE()
 {
-    strictEquality(false);
+    strictEquality(false, false);
     return true;
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_OBJECT()
 {
     JSStackFrame* fp = cx->fp;
     JSScript* script = fp->script;
@@ -7586,17 +7589,18 @@ JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_CONDSWITCH()
 {
     return true;
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_CASE()
 {
-    return equality(CMP_CASE);
+    strictEquality(true, true);
+    return true;
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_DEFAULT()
 {
     return true;
 }
 
@@ -7789,17 +7793,18 @@ JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_GOSUBX()
 {
     return record_JSOP_GOSUB();
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_CASEX()
 {
-    return equality(CMP_CASE);
+    strictEquality(true, true);
+    return true;
 }
 
 JS_REQUIRES_STACK bool
 TraceRecorder::record_JSOP_DEFAULTX()
 {
     return true;
 }
 
--- a/js/src/jstracer.h
+++ b/js/src/jstracer.h
@@ -350,20 +350,19 @@ class TraceRecorder : public avmplus::GC
     JS_REQUIRES_STACK bool ifop();
     JS_REQUIRES_STACK bool switchop();
     JS_REQUIRES_STACK bool inc(jsval& v, jsint incr, bool pre = true);
     JS_REQUIRES_STACK bool inc(jsval& v, nanojit::LIns*& v_ins, jsint incr, bool pre = true);
     JS_REQUIRES_STACK bool incProp(jsint incr, bool pre = true);
     JS_REQUIRES_STACK bool incElem(jsint incr, bool pre = true);
     JS_REQUIRES_STACK bool incName(jsint incr, bool pre = true);
 
-    enum { CMP_NEGATE = 1, CMP_TRY_BRANCH_AFTER_COND = 2, CMP_CASE = 4 };
-    JS_REQUIRES_STACK void strictEquality(bool equal);
-    JS_REQUIRES_STACK bool equality(int flags);
-    JS_REQUIRES_STACK bool relational(nanojit::LOpcode op, int flags);
+    JS_REQUIRES_STACK void strictEquality(bool equal, bool cmpCase);
+    JS_REQUIRES_STACK bool equality(bool negate, bool tryBranchAfterCond);
+    JS_REQUIRES_STACK bool relational(nanojit::LOpcode op, bool tryBranchAfterCond);
 
     JS_REQUIRES_STACK bool unary(nanojit::LOpcode op);
     JS_REQUIRES_STACK bool binary(nanojit::LOpcode op);
 
     bool ibinary(nanojit::LOpcode op);
     bool iunary(nanojit::LOpcode op);
     bool bbinary(nanojit::LOpcode op);
     void demote(jsval& v, jsdouble result);
--- a/js/src/trace-test.js
+++ b/js/src/trace-test.js
@@ -3472,16 +3472,37 @@ function testComparisons()
   if (failures.length == 0)
     return "no failures reported!";
 
   return "\n" + failures.join(",\n");
 }
 testComparisons.expected = "no failures reported!";
 test(testComparisons);
 
+function testCaseAbort()
+{
+  var four = "4";
+  var r = 0;
+  for (var i = 0; i < 5; i++)
+  {
+    switch (i)
+    {
+      case four: r += 1; break;
+      default: r += 2; break;
+    }
+  }
+
+  return "" + r;
+}
+testCaseAbort.expected = "10";
+testCaseAbort.jitstats = {
+  recorderAborted: 0
+};
+test(testCaseAbort);
+
 load("trace-test-math.js");
 
 // BEGIN MANDELBROT STUFF
 // XXXbz I would dearly like to wrap it up into a function to avoid polluting
 // the global scope, but the function ends up heavyweight, and then we lose on
 // the jit.
 load("mandelbrot-results.js");
 //function testMandelbrotAll() {