Bug 1316385: Add an env option to always enable wasm bounds check generation; r=luke
authorBenjamin Bouvier <benj@benj.me>
Wed, 09 Nov 2016 19:41:50 +0100
changeset 364885 07bbb0e3dde46ed12af7dc881695097cc16af045
parent 364821 6c0d7c338607bf58a09d9a9c9bbece1e8bd5321a
child 364886 a67f9e4b5faf56f66f454d58118cbea584a7f334
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1316385
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1316385: Add an env option to always enable wasm bounds check generation; r=luke MozReview-Commit-ID: H5reiyGOCdC
js/src/jit-test/jit_test.py
js/src/jit-test/tests/wasm/bce.js
js/src/jit-test/tests/wasm/memory.js
js/src/jit/CodeGenerator.cpp
js/src/jit/JitOptions.cpp
js/src/jit/JitOptions.h
js/src/jit/Lowering.cpp
js/src/shell/js.cpp
js/src/tests/lib/jittests.py
--- a/js/src/jit-test/jit_test.py
+++ b/js/src/jit-test/jit_test.py
@@ -195,24 +195,24 @@ def main(argv):
     test_list = []
     read_all = True
 
     # No point in adding in noasmjs and wasm-baseline variants if the
     # jitflags forbid asmjs in the first place. (This is to avoid getting a
     # wasm-baseline run when requesting --jitflags=interp, but the test
     # contains test-also-noasmjs.)
     test_flags = get_jitflags(options.jitflags)
-    options.can_test_also_noasmjs = True
-    options.can_test_also_wasm_baseline = True
+    options.asmjs_enabled = True
+    options.wasm_enabled = True
     if all(['--no-asmjs' in flags for flags in test_flags]):
-        options.can_test_also_noasmjs = False
-        options.can_test_also_wasm_baseline = False
+        options.asmjs_enabled = False
+        options.wasm_enabled = False
     if all(['--no-wasm' in flags for flags in test_flags]):
-        options.can_test_also_noasmjs = False
-        options.can_test_also_wasm_baseline = False
+        options.asmjs_enabled = False
+        options.wasm_enabled = False
 
     if test_args:
         read_all = False
         for arg in test_args:
             test_list += jittests.find_tests(arg)
 
     if options.read_tests:
         read_all = False
--- a/js/src/jit-test/tests/wasm/bce.js
+++ b/js/src/jit-test/tests/wasm/bce.js
@@ -1,33 +1,32 @@
+// |jit-test| test-also-wasm-check-bce
 load(libdir + "wasm.js");
 
 mem='\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f'+
     '\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff'+
     '\x00'.repeat(65488) +
     '\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff'
 
-print (mem.lengt)
-
 let accessWidth = {
   '8_s':  1,
-  '8_u':    1,
+  '8_u':  1,
   '16_s': 2,
-  '16_u':   2,
-  '': 4,
+  '16_u': 2,
+  '':     4,
   'f32':  4,
   'f64':  8,
 }
 
 let baseOp = {
   '8_s':  'i32',
-  '8_u':    'i32',
+  '8_u':  'i32',
   '16_s': 'i32',
-  '16_u':   'i32',
-  '': 'i32',
+  '16_u': 'i32',
+  '':     'i32',
   'f32':  'f32',
   'f64':  'f64',
 }
 
 function toSigned(width, num) {
   let unsignedMax = Math.pow(2, accessWidth[width] * 8) - 1;
   let signedMax = Math.pow(2, accessWidth[width] * 8 - 1) - 1;
 
@@ -144,22 +143,21 @@ function testOk(mod, args, expected, exp
 
 // TODO: It would be nice to verify how many BCs are eliminated on positive tests.
 
 const align = 0;
 for (let offset of [0, 1, 2, 3, 4, 8, 16, 41, 0xfff8]) {
 
   var widths = ['8_s', '8_u', '16_s', '16_u', '']
 
-  for(let width of widths) {
+  for (let width of widths) {
     // Accesses of 1 byte.
     let lastValidIndex = 0x10000 - offset - accessWidth[width];
     let op = baseOp[width];
 
-    print("Width: " + width + " offset: " + offset + " op: " + op)
     var mod = loadTwiceModule(op, width, offset, align);
 
     // Two consecutive loads from two different bases
     testOk(mod, [lastValidIndex, lastValidIndex], getInt(width, lastValidIndex + offset, mem), op);
     testOOB(mod, [lastValidIndex + 42, lastValidIndex + 42]);
     testOOB(mod, [lastValidIndex, lastValidIndex + 42]);
 
     mod = loadTwiceSameBasePlusConstModule(op, width, offset, align, 1);
--- a/js/src/jit-test/tests/wasm/memory.js
+++ b/js/src/jit-test/tests/wasm/memory.js
@@ -1,8 +1,9 @@
+// |jit-test| test-also-wasm-check-bce
 load(libdir + "wasm.js");
 
 const RuntimeError = WebAssembly.RuntimeError;
 
 function loadModule(type, ext, offset, align) {
     return wasmEvalText(
     `(module
        (memory 1)
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -11743,29 +11743,16 @@ CodeGenerator::visitWasmTrap(LWasmTrap* 
     masm.jump(trap(mir, mir->trap()));
 }
 
 void
 CodeGenerator::visitWasmBoundsCheck(LWasmBoundsCheck* ins)
 {
     const MWasmBoundsCheck* mir = ins->mir();
     Register ptr = ToRegister(ins->ptr());
-
-    if (mir->isRedundant()) {
-        // For better test coverage, inject debug assertions that redundant
-        // bounds checks really are redundant.
-#ifdef DEBUG
-        Label ok;
-        masm.wasmBoundsCheck(Assembler::Below, ptr, &ok);
-        masm.assumeUnreachable("Redundant bounds check failed!");
-        masm.bind(&ok);
-#endif
-        return;
-    }
-
     masm.wasmBoundsCheck(Assembler::AboveOrEqual, ptr, trap(mir, wasm::Trap::OutOfBounds));
 }
 
 typedef bool (*RecompileFn)(JSContext*);
 static const VMFunction RecompileFnInfo = FunctionInfo<RecompileFn>(Recompile, "Recompile");
 
 typedef bool (*ForcedRecompileFn)(JSContext*);
 static const VMFunction ForcedRecompileFnInfo =
--- a/js/src/jit/JitOptions.cpp
+++ b/js/src/jit/JitOptions.cpp
@@ -222,16 +222,19 @@ DefaultJitOptions::DefaultJitOptions()
     SET_DEFAULT(disableUnboxedObjects, false);
 
     // Test whether Atomics are allowed in asm.js code.
     SET_DEFAULT(asmJSAtomicsEnable, false);
 
     // Test whether wasm int64 / double NaN bits testing is enabled.
     SET_DEFAULT(wasmTestMode, false);
 
+    // Test whether wasm bounds check should always be generated.
+    SET_DEFAULT(wasmAlwaysCheckBounds, false);
+
     // Toggles the optimization whereby offsets are folded into loads and not
     // included in the bounds check.
     SET_DEFAULT(wasmFoldOffsets, true);
 
     // Determines whether we suppress using signal handlers
     // for interrupting jit-ed code. This is used only for testing.
     SET_DEFAULT(ionInterruptWithoutSignals, false);
 }
--- a/js/src/jit/JitOptions.h
+++ b/js/src/jit/JitOptions.h
@@ -66,16 +66,17 @@ struct DefaultJitOptions
     bool disableSincos;
     bool disableSink;
     bool eagerCompilation;
     bool forceInlineCaches;
     bool limitScriptSize;
     bool osr;
     bool asmJSAtomicsEnable;
     bool wasmTestMode;
+    bool wasmAlwaysCheckBounds;
     bool wasmFoldOffsets;
     bool ionInterruptWithoutSignals;
     uint32_t baselineWarmUpThreshold;
     uint32_t exceptionBailoutThreshold;
     uint32_t frequentBailoutThreshold;
     uint32_t maxStackArgs;
     uint32_t osrPcMismatchesBeforeRecompile;
     uint32_t smallFunctionMaxBytecodeLength_;
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -4163,20 +4163,20 @@ LIRGenerator::visitWasmAddOffset(MWasmAd
     MOZ_ASSERT(ins->base()->type() == MIRType::Int32);
     MOZ_ASSERT(ins->type() == MIRType::Int32);
     define(new(alloc()) LWasmAddOffset(useRegisterAtStart(ins->base())), ins);
 }
 
 void
 LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins)
 {
-#ifndef DEBUG
-    if (ins->isRedundant())
-        return;
-#endif
+    if (ins->isRedundant()) {
+        if (MOZ_LIKELY(!JitOptions.wasmAlwaysCheckBounds))
+            return;
+    }
 
     MDefinition* input = ins->input();
     MOZ_ASSERT(input->type() == MIRType::Int32);
 
     auto* lir = new(alloc()) LWasmBoundsCheck(useRegisterAtStart(input));
     add(lir, ins);
 }
 
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -7223,16 +7223,19 @@ SetContextOptions(JSContext* cx, const O
     JS::ContextOptionsRef(cx).setBaseline(enableBaseline)
                              .setIon(enableIon)
                              .setAsmJS(enableAsmJS)
                              .setWasm(enableWasm)
                              .setWasmAlwaysBaseline(enableWasmAlwaysBaseline)
                              .setNativeRegExp(enableNativeRegExp)
                              .setUnboxedArrays(enableUnboxedArrays);
 
+    if (op.getBoolOption("wasm-check-bce"))
+        jit::JitOptions.wasmAlwaysCheckBounds = true;
+
     if (op.getBoolOption("no-unboxed-objects"))
         jit::JitOptions.disableUnboxedObjects = true;
 
     if (const char* str = op.getStringOption("cache-ir-stubs")) {
         if (strcmp(str, "on") == 0)
             jit::JitOptions.disableCacheIR = false;
         else if (strcmp(str, "off") == 0)
             jit::JitOptions.disableCacheIR = true;
@@ -7680,17 +7683,18 @@ main(int argc, char** argv, char** envp)
                             "(default: # of cores - 1)", -1)
         || !op.addBoolOption('\0', "ion", "Enable IonMonkey (default)")
         || !op.addBoolOption('\0', "no-ion", "Disable IonMonkey")
         || !op.addBoolOption('\0', "no-asmjs", "Disable asm.js compilation")
         || !op.addBoolOption('\0', "no-wasm", "Disable WebAssembly compilation")
         || !op.addBoolOption('\0', "no-native-regexp", "Disable native regexp compilation")
         || !op.addBoolOption('\0', "no-unboxed-objects", "Disable creating unboxed plain objects")
         || !op.addBoolOption('\0', "unboxed-arrays", "Allow creating unboxed arrays")
-        || !op.addBoolOption('\0', "wasm-always-baseline", "Enable experimental Wasm baseline compiler when possible")
+        || !op.addBoolOption('\0', "wasm-always-baseline", "Enable wasm baseline compiler when possible")
+        || !op.addBoolOption('\0', "wasm-check-bce", "Always generate wasm bounds check, even redundant ones.")
 #ifdef ENABLE_SHARED_ARRAY_BUFFER
         || !op.addStringOption('\0', "shared-memory", "on/off",
                                "SharedArrayBuffer and Atomics "
 #  if SHARED_MEMORY_DEFAULT
                                "(default: on, off to disable)"
 #  else
                                "(default: off, on to enable)"
 #  endif
--- a/js/src/tests/lib/jittests.py
+++ b/js/src/tests/lib/jittests.py
@@ -238,21 +238,24 @@ class JitTest:
                         test.allow_unhandlable_oom = True
                     elif name == 'allow-overrecursed':
                         test.allow_overrecursed = True
                     elif name == 'valgrind':
                         test.valgrind = options.valgrind
                     elif name == 'tz-pacific':
                         test.tz_pacific = True
                     elif name == 'test-also-noasmjs':
-                        if options.can_test_also_noasmjs:
+                        if options.asmjs_enabled:
                             test.test_also.append(['--no-asmjs'])
                     elif name == 'test-also-wasm-baseline':
-                        if options.can_test_also_wasm_baseline:
+                        if options.wasm_enabled:
                             test.test_also.append(['--wasm-always-baseline'])
+                    elif name == 'test-also-wasm-check-bce':
+                        if options.wasm_enabled:
+                            test.test_also.append(['--wasm-check-bce'])
                     elif name.startswith('test-also='):
                         test.test_also.append([name[len('test-also='):]])
                     elif name.startswith('test-join='):
                         test.test_join.append([name[len('test-join='):]])
                     elif name == 'module':
                         test.is_module = True
                     elif name == 'crash':
                         test.expect_crash = True