Bug 513865 - nanojit: make LirReader::read() clearer and faster. r=gal.

--HG--
extra : convert_revision : d78bd673c8652d17489559744f4a221c78811286

js/src/nanojit/LIR.cpp

@@ -392,51 +392,37 @@ namespace nanojit
     LInsp LirReader::read()
     {
         NanoAssert(_i);
-        LInsp cur = _i;
+        LInsp ret = _i;
-        uintptr_t i = uintptr_t(cur);
-        LOpcode iop = ((LInsp)i)->opcode();
 
-        // We pass over skip instructions below.  Also, the last instruction
+        // Check the invariant: _i never points to a skip.
-        // for a fragment shouldn't be a skip(*).  Therefore we shouldn't see
+        LOpcode iop = _i->opcode();
-        // a skip here.
-        //
-        // (*) Actually, if the last *inserted* instruction exactly fills up a
-        // page, a new page will be created, and thus the last *written*
-        // instruction will be a skip -- the one needed for the cross-page
-        // link.  But the last *inserted* instruction is what is recorded and
-        // used to initialise each LirReader, and that is what is seen here,
-        // and therefore this assertion holds.
         NanoAssert(iop != LIR_skip);
 
-        do
+#ifdef DEBUG
-        {
+        if (iop == LIR_start) {
-            // Nb: this switch is table-driven (because sizeof_LInsXYZ() is
+            // Once we hit here, this method shouldn't be called again.
-            // table-driven) in most cases to avoid branch mispredictions --
+            // The assertion at the top of this method checks this.
-            // if we do a vanilla switch on the iop or LInsRepKind the extra
+            // (In the non-debug case, _i ends up pointing to junk just
-            // branch mispredictions cause a small but noticeable slowdown.
+            // prior to the LIR_start, but it should be ok because,
-            switch (iop)
+            // again, this method shouldn't be called again.)
-            {
+            _i = 0;
-                default:
+            return ret;
-                    i -= insSizes[((LInsp)i)->opcode()];
-                    break;
-
-                case LIR_skip:
-                    // Ignore the skip, move onto its predecessor.
-                    NanoAssert(((LInsp)i)->prevLIns() != (LInsp)i);
-                    i = uintptr_t(((LInsp)i)->prevLIns());
-                    break;
-
-                case LIR_start:
-                    // Once we hit here, this method shouldn't be called again.
-                    // The assertion at the top of this method checks this.
-                    _i = 0;
-                    return cur;
-            }
-            iop = ((LInsp)i)->opcode();
         }
-        while (LIR_skip == iop);
+        else 
-        _i = (LInsp)i;
+#endif
-        return cur;
+        {
+            // Step back one instruction.  Use a table lookup rather than a
+            // switch to avoid branch mispredictions.
+            _i = (LInsp)(uintptr_t(_i) - insSizes[iop]);
+        }
+
+        // Ensure _i doesn't end up pointing to a skip.
+        while (LIR_skip == _i->opcode()) {
+            NanoAssert(_i->prevLIns() != _i);
+            _i = _i->prevLIns();
+        }
+
+        return ret;
     }
 
     // This is never called, but that's ok because it contains only static

js/src/nanojit/LIR.h

@@ -1477,16 +1477,27 @@ namespace nanojit
     // concrete
     class LirReader : public LirFilter
     {
-        LInsp _i; // current instruction that this decoder is operating on.
+        LInsp _i; // next instruction to be read;  invariant: is never a skip
 
     public:
-        LirReader(LInsp i) : LirFilter(0), _i(i) {
+        LirReader(LInsp i) : LirFilter(0), _i(i)
-            NanoAssert(_i);
+        {
+            // The last instruction for a fragment shouldn't be a skip.
+            // (Actually, if the last *inserted* instruction exactly fills up
+            // a chunk, a new chunk will be created, and thus the last *written*
+            // instruction will be a skip -- the one needed for the
+            // cross-chunk link.  But the last *inserted* instruction is what
+            // is recorded and used to initialise each LirReader, and that is
+            // what is seen here, and therefore this assertion holds.)
+            NanoAssert(i && !i->isop(LIR_skip));
         }
         virtual ~LirReader() {}
 
-        // LirReader i/f
+        // Returns next instruction and advances to the prior instruction.
-        LInsp read(); // advance to the prior instruction
+        // Invariant: never returns a skip.
+        LInsp read(); 
+
+        // Returns next instruction.  Invariant: never returns a skip.
         LInsp pos() {
             return _i;
         }