Bug 1253137 - Baldr: switch from expression-count to function body byte size (r=sunfish)
authorLuke Wagner <luke@mozilla.com>
Fri, 04 Mar 2016 18:43:00 -0600
changeset 323206 d0c4157a7fc6d74975b44864e489afb292845751
parent 323205 46b17b64ec464a71981e7f157cbe0e9f597fa8a5
child 323207 d2e86ab855bd0b14f381e4505627310c915dd79c
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs1253137
milestone47.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 1253137 - Baldr: switch from expression-count to function body byte size (r=sunfish) MozReview-Commit-ID: KcbWjViZAM6
js/src/asmjs/AsmJS.cpp
js/src/asmjs/Wasm.cpp
js/src/asmjs/WasmBinary.h
js/src/asmjs/WasmIonCompile.cpp
js/src/asmjs/WasmText.cpp
js/src/jit-test/tests/wasm/basic-control-flow.js
js/src/jit-test/tests/wasm/basic.js
js/src/jsapi-tests/testWasmLEB128.cpp
--- a/js/src/asmjs/AsmJS.cpp
+++ b/js/src/asmjs/AsmJS.cpp
@@ -3489,17 +3489,17 @@ static bool
 SetLocal(FunctionValidator& f, NumLit lit)
 {
     return f.encoder().writeExpr(Expr::SetLocal) &&
            f.encoder().writeVarU32(f.numLocals()) &&
            f.writeLit(lit);
 }
 
 static bool
-CheckVariable(FunctionValidator& f, ParseNode* var, uint32_t* numStmts)
+CheckVariable(FunctionValidator& f, ParseNode* var)
 {
     if (!IsDefinition(var))
         return f.fail(var, "local variable names must not restate argument names");
 
     PropertyName* name = var->name();
 
     if (!CheckIdentifier(f.m(), var, name))
         return false;
@@ -3511,32 +3511,31 @@ CheckVariable(FunctionValidator& f, Pars
     NumLit lit;
     if (!IsLiteralOrConst(f, initNode, &lit))
         return f.failName(var, "var '%s' initializer must be literal or const literal", name);
 
     if (!lit.valid())
         return f.failName(var, "var '%s' initializer out of range", name);
 
     if (!lit.isZeroBits()) {
-        ++*numStmts;
         if (!SetLocal(f, lit))
             return false;
     }
 
     return f.addLocal(var, name, Type::canonicalize(Type::lit(lit)));
 }
 
 static bool
-CheckVariables(FunctionValidator& f, ParseNode** stmtIter, uint32_t* numStmts)
+CheckVariables(FunctionValidator& f, ParseNode** stmtIter)
 {
     ParseNode* stmt = *stmtIter;
 
     for (; stmt && stmt->isKind(PNK_VAR); stmt = NextNonEmptyStatement(stmt)) {
         for (ParseNode* var = VarListHead(stmt); var; var = NextNode(var)) {
-            if (!CheckVariable(f, var, numStmts))
+            if (!CheckVariable(f, var))
                 return false;
         }
     }
 
     *stmtIter = stmt;
     return true;
 }
 
@@ -6727,38 +6726,29 @@ CheckFunction(ModuleValidator& m)
 
     if (!CheckProcessingDirectives(m, &stmtIter))
         return false;
 
     ValTypeVector args;
     if (!CheckArguments(f, &stmtIter, &args))
         return false;
 
-    uint32_t numStmts = 0;
-
-    size_t numStmtsAt;
-    if (!f.encoder().writePatchableVarU32(&numStmtsAt))
-        return false;
-
-    if (!CheckVariables(f, &stmtIter, &numStmts))
+    if (!CheckVariables(f, &stmtIter))
         return false;
 
     ParseNode* lastNonEmptyStmt = nullptr;
     for (; stmtIter; stmtIter = NextNonEmptyStatement(stmtIter)) {
-        numStmts++;
         lastNonEmptyStmt = stmtIter;
         if (!CheckStatement(f, stmtIter))
             return false;
     }
 
     if (!CheckFinalReturn(f, lastNonEmptyStmt))
         return false;
 
-    f.encoder().patchVarU32(numStmtsAt, numStmts);
-
     ModuleValidator::Func* func = nullptr;
     if (!CheckFunctionSignature(m, fn, Sig(Move(args), f.returnedType()), FunctionName(fn), &func))
         return false;
 
     if (func->defined())
         return m.failName(fn, "function '%s' already defined", FunctionName(fn));
 
     func->define(fn);
--- a/js/src/asmjs/Wasm.cpp
+++ b/js/src/asmjs/Wasm.cpp
@@ -1257,35 +1257,41 @@ DecodeFunctionBody(JSContext* cx, Decode
     for (uint32_t i = 0; i < numVars; i++) {
         ValType type;
         if (!DecodeValType(cx, d, &type))
             return false;
         if (!fg.addLocal(type))
             return false;
     }
 
-    const uint8_t* bodyBegin = d.currentPosition();
-
     FunctionDecoder f(cx, d, mg, fg, funcIndex);
 
-    uint32_t numExprs;
-    if (!d.readVarU32(&numExprs))
-        return Fail(cx, d, "expected number of function body expressions");
+    uint32_t numBytes;
+    if (!d.readVarU32(&numBytes))
+        return Fail(cx, d, "expected number of function body bytes");
+
+    if (d.bytesRemain() < numBytes)
+        return Fail(cx, d, "function body length too big");
+
+    const uint8_t* bodyBegin = d.currentPosition();
+    const uint8_t* bodyEnd = bodyBegin + numBytes;
 
     ExprType type = ExprType::Void;
 
-    for (size_t i = 0; i < numExprs; i++) {
+    while (d.currentPosition() < bodyEnd) {
         if (!DecodeExpr(f, &type))
             return false;
     }
 
+    if (d.currentPosition() != bodyEnd)
+        return Fail(cx, d, "function body length mismatch");
+
     if (!CheckType(f, type, f.ret()))
         return false;
 
-    const uint8_t* bodyEnd = d.currentPosition();
     uintptr_t bodyLength = bodyEnd - bodyBegin;
     if (!fg.bytecode().resize(bodyLength))
         return false;
 
     memcpy(fg.bytecode().begin(), bodyBegin, bodyLength);
 
     int64_t after = PRMJ_Now();
     unsigned generateTime = (after - before) / PRMJ_USEC_PER_MSEC;
--- a/js/src/asmjs/WasmBinary.h
+++ b/js/src/asmjs/WasmBinary.h
@@ -379,18 +379,18 @@ class Encoder
 
   public:
     explicit Encoder(Bytecode& bytecode)
       : bytecode_(bytecode)
     {
         MOZ_ASSERT(empty());
     }
 
-    size_t bytecodeOffset() const { return bytecode_.length(); }
-    bool empty() const { return bytecodeOffset() == 0; }
+    size_t currentOffset() const { return bytecode_.length(); }
+    bool empty() const { return currentOffset() == 0; }
 
     // Fixed-size encoding operations simply copy the literal bytes (without
     // attempting to align).
 
     MOZ_WARN_UNUSED_RESULT bool writeFixedU32(uint32_t i) {
         return write<uint32_t>(i);
     }
     MOZ_WARN_UNUSED_RESULT bool writeFixedF32(float f) {
@@ -419,16 +419,42 @@ class Encoder
     }
     MOZ_WARN_UNUSED_RESULT bool writeValType(ValType type) {
         return writeEnum(type);
     }
     MOZ_WARN_UNUSED_RESULT bool writeExprType(ExprType type) {
         return writeEnum(type);
     }
 
+    // Variable-length encodings that allow back-patching.
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU32(size_t* offset) {
+        *offset = bytecode_.length();
+        return writeVarU32(UINT32_MAX);
+    }
+    void patchVarU32(size_t offset, uint32_t patchBits) {
+        return patchVarU32(offset, patchBits, UINT32_MAX);
+    }
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU8(size_t* offset) {
+        *offset = bytecode_.length();
+        return writeU8(UINT8_MAX);
+    }
+    void patchVarU8(size_t offset, uint8_t patchBits) {
+        MOZ_ASSERT(patchBits < 0x80);
+        return patchU8(offset, patchBits);
+    }
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableExpr(size_t* offset) {
+        return writePatchableEnum<Expr>(offset);
+    }
+    void patchExpr(size_t offset, Expr expr) {
+        patchEnum(offset, expr);
+    }
+
     // C-strings are written in UTF8 and null-terminated while raw data can
     // contain nulls and instead has an explicit byte length.
 
     MOZ_WARN_UNUSED_RESULT bool writeCString(const char* cstr) {
         return bytecode_.append(reinterpret_cast<const uint8_t*>(cstr), strlen(cstr) + 1);
     }
     MOZ_WARN_UNUSED_RESULT bool writeRawData(const uint8_t* bytes, uint32_t numBytes) {
         return bytecode_.append(bytes, numBytes);
@@ -447,44 +473,16 @@ class Encoder
         return writePatchableVarU32(offset) &&
                writeVarU32(IdSize) &&
                writeRawData(reinterpret_cast<const uint8_t*>(id), IdSize);
     }
     void finishSection(size_t offset) {
         return patchVarU32(offset, bytecode_.length() - offset - varU32ByteLength(offset));
     }
 
-    // Patching is necessary due to the combination of a preorder encoding and a
-    // single-pass algorithm that only knows the precise opcode after visiting
-    // children. Switching to a postorder encoding will remove these methods:
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU32(size_t* offset) {
-        *offset = bytecode_.length();
-        return writeVarU32(UINT32_MAX);
-    }
-    void patchVarU32(size_t offset, uint32_t patchBits) {
-        return patchVarU32(offset, patchBits, UINT32_MAX);
-    }
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU8(size_t* offset) {
-        *offset = bytecode_.length();
-        return writeU8(UINT8_MAX);
-    }
-    void patchVarU8(size_t offset, uint8_t patchBits) {
-        MOZ_ASSERT(patchBits < 0x80);
-        return patchU8(offset, patchBits);
-    }
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableExpr(size_t* offset) {
-        return writePatchableEnum<Expr>(offset);
-    }
-    void patchExpr(size_t offset, Expr expr) {
-        patchEnum(offset, expr);
-    }
-
     // Temporary encoding forms which should be removed as part of the
     // conversion to wasm:
 
     MOZ_WARN_UNUSED_RESULT bool writeU8(uint8_t i) {
         return write<uint8_t>(i);
     }
     MOZ_WARN_UNUSED_RESULT bool writePatchableU8(size_t* offset) {
         *offset = bytecode_.length();
@@ -497,21 +495,16 @@ class Encoder
 };
 
 class Decoder
 {
     const uint8_t* const beg_;
     const uint8_t* const end_;
     const uint8_t* cur_;
 
-    uintptr_t bytesRemain() const {
-        MOZ_ASSERT(end_ >= cur_);
-        return uintptr_t(end_ - cur_);
-    }
-
     template <class T>
     MOZ_WARN_UNUSED_RESULT bool read(T* out) {
         if (bytesRemain() < sizeof(T))
             return false;
         if (out)
             memcpy((void*)out, cur_, sizeof(T));
         cur_ += sizeof(T);
         return true;
@@ -583,16 +576,20 @@ class Decoder
         cur_(bytecode.begin())
     {}
 
     bool done() const {
         MOZ_ASSERT(cur_ <= end_);
         return cur_ == end_;
     }
 
+    uintptr_t bytesRemain() const {
+        MOZ_ASSERT(end_ >= cur_);
+        return uintptr_t(end_ - cur_);
+    }
     const uint8_t* currentPosition() const {
         return cur_;
     }
     size_t currentOffset() const {
         return cur_ - beg_;
     }
     void assertCurrentIs(const DebugOnly<size_t> offset) const {
         MOZ_ASSERT(currentOffset() == offset);
--- a/js/src/asmjs/WasmIonCompile.cpp
+++ b/js/src/asmjs/WasmIonCompile.cpp
@@ -3056,30 +3056,22 @@ wasm::IonCompileFunction(IonCompileTask*
     mir.initMinAsmJSHeapLength(task->mg().minHeapLength());
 
     // Build MIR graph
     {
         FunctionCompiler f(task->mg(), func, mir, results);
         if (!f.init())
             return false;
 
-        MDefinition* last = nullptr;
-        if (uint32_t numExprs = f.readVarU32()) {
-            for (uint32_t i = 0; i < numExprs - 1; i++) {
-                if (!EmitExpr(f, &last))
-                    return false;
-            }
-
+        MDefinition* last;
+        while (!f.done()) {
             if (!EmitExpr(f, &last))
                 return false;
         }
 
-        MOZ_ASSERT(f.done());
-        MOZ_ASSERT(IsVoid(f.sig().ret()) || f.inDeadCode() || last);
-
         if (IsVoid(f.sig().ret()))
             f.returnVoid();
         else
             f.returnExpr(last);
 
         f.checkPostconditions();
     }
 
--- a/js/src/asmjs/WasmText.cpp
+++ b/js/src/asmjs/WasmText.cpp
@@ -3922,24 +3922,28 @@ EncodeFunctionBody(Encoder& e, WasmAstFu
     if (!e.writeVarU32(func.vars().length()))
         return false;
 
     for (ValType type : func.vars()) {
         if (!e.writeValType(type))
             return false;
     }
 
-    if (!e.writeVarU32(func.body().length()))
+    size_t bodySizeAt;
+    if (!e.writePatchableVarU32(&bodySizeAt))
         return false;
 
+    size_t beforeBody = e.currentOffset();
+
     for (WasmAstExpr* expr : func.body()) {
         if (!EncodeExpr(e, *expr))
             return false;
     }
 
+    e.patchVarU32(bodySizeAt, e.currentOffset() - beforeBody);
     return true;
 }
 
 static bool
 EncodeFunctionBodies(Encoder& e, WasmAstModule& module)
 {
     if (module.funcs().empty())
         return true;
--- a/js/src/jit-test/tests/wasm/basic-control-flow.js
+++ b/js/src/jit-test/tests/wasm/basic-control-flow.js
@@ -135,17 +135,19 @@ wasmEvalText('(module (func (if_else (i3
 // ----------------------------------------------------------------------------
 // return
 
 assertEq(wasmEvalText('(module (func (return)) (export "" 0))')(), undefined);
 assertEq(wasmEvalText('(module (func (result i32) (return (i32.const 1))) (export "" 0))')(), 1);
 assertEq(wasmEvalText('(module (func (if (return) (i32.const 0))) (export "" 0))')(), undefined);
 assertErrorMessage(() => wasmEvalText('(module (func (result f32) (return (i32.const 1))) (export "" 0))'), TypeError, mismatchError("i32", "f32"));
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (result i32) (return)) (export "" 0))'), TypeError);
-assertThrowsInstanceOf(() => wasmEvalText('(module (func (return (i32.const 1))) (export "" 0))'), TypeError);
+
+// TODO: Reenable when syntactic arities are added for returns
+//assertThrowsInstanceOf(() => wasmEvalText('(module (func (return (i32.const 1))) (export "" 0))'), TypeError);
 
 // TODO: convert these to wasmEval and assert some results once they are implemented
 
 // ----------------------------------------------------------------------------
 // br / br_if
 
 assertErrorMessage(() => wasmEvalText('(module (func (result i32) (block (br 0))) (export "" 0))'), TypeError, mismatchError("void", "i32"));
 assertErrorMessage(() => wasmEvalText('(module (func (result i32) (block (br_if 0 (i32.const 0)))) (export "" 0))'), TypeError, mismatchError("void", "i32"));
--- a/js/src/jit-test/tests/wasm/basic.js
+++ b/js/src/jit-test/tests/wasm/basic.js
@@ -284,29 +284,33 @@ assertErrorMessage(() => wasmEvalText('(
 assertEq(wasmEvalText('(module (func (result i32) (block (i32.const 13) (block (i32.const 42)))) (export "" 0))')(), 42);
 assertErrorMessage(() => wasmEvalText('(module (func (result f32) (param f32) (block (get_local 0) (i32.const 0))))'), TypeError, mismatchError("i32", "f32"));
 
 assertEq(wasmEvalText('(module (func (result i32) (local i32) (set_local 0 (i32.const 42)) (get_local 0)) (export "" 0))')(), 42);
 
 // ----------------------------------------------------------------------------
 // calls
 
-assertThrowsInstanceOf(() => wasmEvalText('(module (func (nop)) (func (call 0 (i32.const 0))))'), TypeError);
+// TODO: Reenable when syntactic arities are added for calls
+//assertThrowsInstanceOf(() => wasmEvalText('(module (func (nop)) (func (call 0 (i32.const 0))))'), TypeError);
+
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (param i32) (nop)) (func (call 0)))'), TypeError);
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (param f32) (nop)) (func (call 0 (i32.const 0))))'), TypeError);
 assertErrorMessage(() => wasmEvalText('(module (func (nop)) (func (call 3)))'), TypeError, /callee index out of range/);
 wasmEvalText('(module (func (nop)) (func (call 0)))');
 wasmEvalText('(module (func (param i32) (nop)) (func (call 0 (i32.const 0))))');
 assertEq(wasmEvalText('(module (func (result i32) (i32.const 42)) (func (result i32) (call 0)) (export "" 1))')(), 42);
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (call 0)) (export "" 0))')(), InternalError);
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (call 1)) (func (call 0)) (export "" 0))')(), InternalError);
 wasmEvalText('(module (func (param i32 f32)) (func (call 0 (i32.const 0) (f32.const nan))))');
 assertErrorMessage(() => wasmEvalText('(module (func (param i32 f32)) (func (call 0 (i32.const 0) (i32.const 0))))'), TypeError, mismatchError("i32", "f32"));
 
-assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "") (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
+// TODO: Reenable when syntactic arities are added for calls
+//assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "") (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
+
 assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "" (param i32)) (func (call_import 0)))', {a:()=>{}}), TypeError);
 assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "" (param f32)) (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
 assertErrorMessage(() => wasmEvalText('(module (import "a" "") (func (call_import 1)))'), TypeError, /import index out of range/);
 wasmEvalText('(module (import "a" "") (func (call_import 0)))', {a:()=>{}});
 wasmEvalText('(module (import "a" "" (param i32)) (func (call_import 0 (i32.const 0))))', {a:()=>{}});
 
 function checkF32CallImport(v) {
     assertEq(wasmEvalText('(module (import "a" "" (result f32)) (func (result f32) (call_import 0)) (export "" 0))', {a:()=>{ return v; }})(), Math.fround(v));
--- a/js/src/jsapi-tests/testWasmLEB128.cpp
+++ b/js/src/jsapi-tests/testWasmLEB128.cpp
@@ -27,17 +27,17 @@ static bool WriteValidBytes(js::wasm::En
         return false;
 
     // 0x03 0x80
     if (!encoder.writeVarU32(0x180))
         return false;
 
     if (encoder.empty())
         return true;
-    if (encoder.bytecodeOffset() != 7)
+    if (encoder.currentOffset() != 7)
         return true;
     *passed = true;
     return true;
 }
 
 BEGIN_TEST(testWasmLEB128_encoding)
 {
     using namespace js;