Bug 1303122: Don't re-add an already seen bounds check if it's not redundant; r=luke
authorBenjamin Bouvier <benj@benj.me>
Thu, 15 Sep 2016 21:35:13 +0200
changeset 314403 945544b11e381c00753eb64e6c79196d7609033e
parent 314402 5841b0c33b7ef92545c920d6da48a1f8cb3543db
child 314404 d6809e466fe6c6cdeebeebe8878ee0d03e59d4f6
push id20571
push userkwierso@gmail.com
push dateMon, 19 Sep 2016 22:56:59 +0000
treeherderfx-team@671c2af548b2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1303122
milestone51.0a1
Bug 1303122: Don't re-add an already seen bounds check if it's not redundant; r=luke MozReview-Commit-ID: 4pthpbRTmpD
js/src/jit-test/tests/wasm/regress/teavm-bugs.js
js/src/jit/WasmBCE.cpp
--- a/js/src/jit-test/tests/wasm/regress/teavm-bugs.js
+++ b/js/src/jit-test/tests/wasm/regress/teavm-bugs.js
@@ -23,8 +23,34 @@ for (let i = 15; i --> 0;) {
 let code = `(module
    (func $i64
      ${params} ${locals}
      ${tests}
    )
 )`
 
 evalText(code);
+
+// Bounds check elimination.
+assertEq(evalText(`(module
+    (memory 1)
+    (func (param $p i32) (local $l i32) (result i32)
+        (set_local $l (i32.const 0))
+        (if
+            (get_local $p)
+            (set_local $l
+               (i32.add
+                  (get_local $l)
+                  (i32.load8_s (get_local $p))
+               )
+            )
+        )
+        (set_local $l
+           (i32.add
+              (get_local $l)
+              (i32.load8_s (get_local $p))
+           )
+        )
+        (get_local $l)
+    )
+    (data 0 "\\00\\01\\02\\03\\04\\05\\06\\07\\08\\09\\0a\\0b\\0c\\0d\\0e\\0f")
+    (export "test" 0)
+)`).exports["test"](3), 6);
--- a/js/src/jit/WasmBCE.cpp
+++ b/js/src/jit/WasmBCE.cpp
@@ -39,21 +39,22 @@ jit::EliminateBoundsChecks(MIRGenerator*
         for (MDefinitionIterator dIter(block); dIter;) {
             MDefinition* def = *dIter++;
 
             switch (def->op()) {
               case MDefinition::Op_WasmBoundsCheck: {
                 MWasmBoundsCheck* bc = def->toWasmBoundsCheck();
                 MDefinition* addr = def->getOperand(0);
 
-                LastSeenMap::AddPtr checkPtr = lastSeen.lookupForAdd(addr->id());
-                if (checkPtr && checkPtr->value()->block()->dominates(block)) {
-                    bc->setRedundant(true);
+                LastSeenMap::AddPtr ptr = lastSeen.lookupForAdd(addr->id());
+                if (ptr) {
+                    if (ptr->value()->block()->dominates(block))
+                        bc->setRedundant(true);
                 } else {
-                    if (!lastSeen.add(checkPtr, addr->id(), def))
+                    if (!lastSeen.add(ptr, addr->id(), def))
                         return false;
                 }
                 break;
               }
               case MDefinition::Op_Phi: {
                 MPhi* phi = def->toPhi();
                 bool phiChecked = true;