Bug 1317319: Factor out Global section decoding; r=luke
authorBenjamin Bouvier <benj@benj.me>
Mon, 14 Nov 2016 17:40:56 +0100
changeset 323050 5b9544a7f1d8598f37cbe9f8b7302b84f775c82d
parent 323049 75b417fb952fb20d3dec91949e653d015600015e
child 323051 5d3cab095f63084a4b86082bf2bbe55ce7283749
push id30967
push userphilringnalda@gmail.com
push dateFri, 18 Nov 2016 03:21:38 +0000
treeherdermozilla-central@8e476f8bd52d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1317319
milestone53.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 1317319: Factor out Global section decoding; r=luke MozReview-Commit-ID: 2kIKnu4BAQN
js/src/wasm/WasmBinaryFormat.cpp
js/src/wasm/WasmBinaryFormat.h
js/src/wasm/WasmBinaryToAST.cpp
js/src/wasm/WasmCompile.cpp
--- a/js/src/wasm/WasmBinaryFormat.cpp
+++ b/js/src/wasm/WasmBinaryFormat.cpp
@@ -244,16 +244,33 @@ wasm::GlobalIsJSCompatible(Decoder& d, V
 
     if (isMutable)
         return d.fail("can't import/export mutable globals in the MVP");
 
     return true;
 }
 
 static bool
+DecodeGlobalType(Decoder& d, ValType* type, bool* isMutable)
+{
+    if (!DecodeValType(d, ModuleKind::Wasm, type))
+        return false;
+
+    uint32_t flags;
+    if (!d.readVarU32(&flags))
+        return d.fail("expected global flags");
+
+    if (flags & ~uint32_t(GlobalFlags::AllowedMask))
+        return d.fail("unexpected bits set in global flags");
+
+    *isMutable = flags & uint32_t(GlobalFlags::IsMutable);
+    return true;
+}
+
+static bool
 DecodeMemoryLimits(Decoder& d, bool hasMemory, Limits* memory)
 {
     if (hasMemory)
         return d.fail("already have default memory");
 
     if (!DecodeLimits(d, memory))
         return false;
 
@@ -447,16 +464,52 @@ wasm::DecodeMemorySection(Decoder& d, bo
 
     if (!d.finishSection(sectionStart, sectionSize, "memory"))
         return false;
 
     return true;
 }
 
 bool
+wasm::DecodeGlobalSection(Decoder& d, GlobalDescVector* globals)
+{
+    uint32_t sectionStart, sectionSize;
+    if (!d.startSection(SectionId::Global, &sectionStart, &sectionSize, "global"))
+        return false;
+    if (sectionStart == Decoder::NotStarted)
+        return true;
+
+    uint32_t numGlobals;
+    if (!d.readVarU32(&numGlobals))
+        return d.fail("expected number of globals");
+
+    if (numGlobals > MaxGlobals)
+        return d.fail("too many globals");
+
+    for (uint32_t i = 0; i < numGlobals; i++) {
+        ValType type;
+        bool isMutable;
+        if (!DecodeGlobalType(d, &type, &isMutable))
+            return false;
+
+        InitExpr initializer;
+        if (!DecodeInitializerExpression(d, *globals, type, &initializer))
+            return false;
+
+        if (!globals->append(GlobalDesc(initializer, isMutable)))
+            return false;
+    }
+
+    if (!d.finishSection(sectionStart, sectionSize, "global"))
+        return false;
+
+    return true;
+}
+
+bool
 wasm::EncodeLocalEntries(Encoder& e, const ValTypeVector& locals)
 {
     uint32_t numLocalEntries = 0;
     ValType prev = ValType(TypeCode::Limit);
     for (ValType t : locals) {
         if (t != prev) {
             numLocalEntries++;
             prev = t;
@@ -510,33 +563,16 @@ wasm::DecodeLocalEntries(Decoder& d, Mod
         if (!locals->appendN(type, count))
             return false;
     }
 
     return true;
 }
 
 bool
-wasm::DecodeGlobalType(Decoder& d, ValType* type, bool* isMutable)
-{
-    if (!DecodeValType(d, ModuleKind::Wasm, type))
-        return false;
-
-    uint32_t flags;
-    if (!d.readVarU32(&flags))
-        return d.fail("expected global flags");
-
-    if (flags & ~uint32_t(GlobalFlags::AllowedMask))
-        return d.fail("unexpected bits set in global flags");
-
-    *isMutable = flags & uint32_t(GlobalFlags::IsMutable);
-    return true;
-}
-
-bool
 wasm::DecodeInitializerExpression(Decoder& d, const GlobalDescVector& globals, ValType expected,
                                   InitExpr* init)
 {
     uint16_t op;
     if (!d.readOp(&op))
         return d.fail("failed to read initializer type");
 
     switch (op) {
--- a/js/src/wasm/WasmBinaryFormat.h
+++ b/js/src/wasm/WasmBinaryFormat.h
@@ -632,19 +632,16 @@ GlobalIsJSCompatible(Decoder& d, ValType
 
 MOZ_MUST_USE bool
 EncodeLocalEntries(Encoder& d, const ValTypeVector& locals);
 
 MOZ_MUST_USE bool
 DecodeLocalEntries(Decoder& d, ModuleKind kind, ValTypeVector* locals);
 
 MOZ_MUST_USE bool
-DecodeGlobalType(Decoder& d, ValType* type, bool* isMutable);
-
-MOZ_MUST_USE bool
 DecodeInitializerExpression(Decoder& d, const GlobalDescVector& globals, ValType expected,
                             InitExpr* init);
 
 // Section macros.
 
 MOZ_MUST_USE bool
 DecodePreamble(Decoder& d);
 
@@ -662,16 +659,19 @@ DecodeFunctionSection(Decoder& d, const 
 
 MOZ_MUST_USE bool
 DecodeTableSection(Decoder& d, TableDescVector* tables);
 
 MOZ_MUST_USE bool
 DecodeMemorySection(Decoder& d, bool hasMemory, Limits* memory, bool* present);
 
 MOZ_MUST_USE bool
+DecodeGlobalSection(Decoder& d, GlobalDescVector* globals);
+
+MOZ_MUST_USE bool
 DecodeUnknownSections(Decoder& d);
 
 MOZ_MUST_USE bool
 DecodeDataSection(Decoder& d, bool usesMemory, uint32_t minMemoryByteLength,
                   const GlobalDescVector& globals, DataSegmentVector* segments);
 
 } // namespace wasm
 } // namespace js
--- a/js/src/wasm/WasmBinaryToAST.cpp
+++ b/js/src/wasm/WasmBinaryToAST.cpp
@@ -125,33 +125,17 @@ class AstDecodeContext
     AstDecodeOpIter& iter() { return *iter_; }
     AstDecodeStack& exprs() { return exprs_; }
     DepthStack& depths() { return depths_; }
 
     AstNameVector& blockLabels() { return blockLabels_; }
 
     ExprType retType() const { return retType_; }
     const ValTypeVector& locals() const { return *locals_; }
-
-    bool addGlobalDesc(ValType type, bool isMutable, bool isImport) {
-        if (isImport)
-            return globals_.append(GlobalDesc(type, isMutable, globals_.length()));
-        // No need to have the precise init expr value; we just need the right
-        // type.
-        Val dummy;
-        switch (type) {
-          case ValType::I32: dummy = Val(uint32_t(0)); break;
-          case ValType::I64: dummy = Val(uint64_t(0)); break;
-          case ValType::F32: dummy = Val(RawF32(0.f)); break;
-          case ValType::F64: dummy = Val(RawF64(0.0)); break;
-          default:           return false;
-        }
-        return globals_.append(GlobalDesc(InitExpr(dummy), isMutable));
-    }
-    const GlobalDescVector& globalDescs() const { return globals_; }
+    GlobalDescVector& globalDescs() { return globals_; }
 
     void popBack() { return exprs().popBack(); }
     AstDecodeStackItem popCopy() { return exprs().popCopy(); }
     AstDecodeStackItem& top() { return exprs().back(); }
     MOZ_MUST_USE bool push(AstDecodeStackItem item) { return exprs().append(item); }
 
     bool needFirst() {
         for (size_t i = depths().back(); i < exprs().length(); ++i) {
@@ -1468,20 +1452,20 @@ ToAstName(AstDecodeContext& c, const Uni
 
     return AstName(buffer, len);
 }
 
 static bool
 AstDecodeImportSection(AstDecodeContext& c, const SigWithIdVector& sigs, TableDescVector* tables)
 {
     Uint32Vector funcSigIndices;
-    GlobalDescVector globals;
     Maybe<Limits> memory;
     ImportVector imports;
-    if (!DecodeImportSection(c.d, sigs, &funcSigIndices, &globals, tables, &memory, &imports))
+    if (!DecodeImportSection(c.d, sigs, &funcSigIndices, &c.globalDescs(), tables, &memory,
+                             &imports))
         return false;
 
     size_t lastFunc = 0;
     size_t lastGlobal = 0;
     size_t lastTable = 0;
     size_t lastMemory = 0;
 
     for (size_t importIndex = 0; importIndex < imports.length(); importIndex++) {
@@ -1505,23 +1489,20 @@ AstDecodeImportSection(AstDecodeContext&
             lastFunc++;
             break;
           }
           case DefinitionKind::Global: {
             AstName importName;
             if (!GenerateName(c, AstName(u"global"), lastGlobal, &importName))
                 return false;
 
-            const GlobalDesc& global = globals[lastGlobal];
+            const GlobalDesc& global = c.globalDescs()[lastGlobal];
             ValType type = global.type();
             bool isMutable = global.isMutable();
 
-            if (!c.addGlobalDesc(type, isMutable, /* import */ true))
-                return false;
-
             ast = new(c.lifo) AstImport(importName, moduleName, fieldName,
                                         AstGlobal(importName, type, isMutable));
             lastGlobal++;
             break;
           }
           case DefinitionKind::Table: {
             AstName importName;
             if (!GenerateName(c, AstName(u"table"), lastTable, &importName))
@@ -1543,16 +1524,21 @@ AstDecodeImportSection(AstDecodeContext&
             break;
           }
         }
 
         if (!ast || !c.module().append(ast))
             return false;
     }
 
+    MOZ_ASSERT(lastFunc == funcSigIndices.length());
+    MOZ_ASSERT(lastGlobal == c.globalDescs().length());
+    MOZ_ASSERT(lastTable == tables->length());
+    MOZ_ASSERT(!!lastMemory == !!memory);
+
     return true;
 }
 
 static bool
 AstDecodeFunctionSection(AstDecodeContext& c, const SigWithIdVector& sigs)
 {
     Uint32Vector funcSigIndexes;
     if (!DecodeFunctionSection(c.d, sigs, c.module().numFuncImports(), &funcSigIndexes))
@@ -1645,64 +1631,40 @@ AstDecodeInitializerExpression(AstDecode
     if (!DecodeInitializerExpression(c.d, c.globalDescs(), type, &initExpr))
         return false;
 
     *init = ToAstExpr(c, initExpr);
     return !!*init;
 }
 
 static bool
-AstDecodeGlobal(AstDecodeContext& c, uint32_t i, AstGlobal* global)
+AstDecodeGlobalSection(AstDecodeContext& c)
 {
-    AstName name;
-    if (!GenerateName(c, AstName(u"global"), i, &name))
-        return false;
-
-    ValType type;
-    bool isMutable;
-    if (!DecodeGlobalType(c.d, &type, &isMutable))
-        return false;
-
-    AstExpr* init;
-    if (!AstDecodeInitializerExpression(c, type, &init))
-        return false;
-
-    if (!c.addGlobalDesc(type, isMutable, /* import */ false))
+    size_t numImported = c.globalDescs().length();
+    if (!DecodeGlobalSection(c.d, &c.globalDescs()))
         return false;
 
-    *global = AstGlobal(name, type, isMutable, Some(init));
-    return true;
-}
+    for (uint32_t i = numImported; i < c.globalDescs().length(); i++) {
+        AstName name;
+        if (!GenerateName(c, AstName(u"global"), i, &name))
+            return false;
+
+        const GlobalDesc& global = c.globalDescs()[i];
 
-static bool
-AstDecodeGlobalSection(AstDecodeContext& c)
-{
-    uint32_t sectionStart, sectionSize;
-    if (!c.d.startSection(SectionId::Global, &sectionStart, &sectionSize, "global"))
-        return false;
-    if (sectionStart == Decoder::NotStarted)
-        return true;
+        AstExpr* init = global.isConstant()
+                        ? new(c.lifo) AstConst(global.constantValue())
+                        : ToAstExpr(c, global.initExpr());
+        if (!init)
+            return false;
 
-    uint32_t numGlobals;
-    if (!c.d.readVarU32(&numGlobals))
-        return c.d.fail("expected number of globals");
-
-    uint32_t numImported = c.globalDescs().length();
-
-    for (uint32_t i = 0; i < numGlobals; i++) {
-        auto* global = new(c.lifo) AstGlobal;
-        if (!AstDecodeGlobal(c, i + numImported, global))
-            return false;
-        if (!c.module().append(global))
+        auto* g = new(c.lifo) AstGlobal(name, global.type(), global.isMutable(), Some(init));
+        if (!g || !c.module().append(g))
             return false;
     }
 
-    if (!c.d.finishSection(sectionStart, sectionSize, "global"))
-        return false;
-
     return true;
 }
 
 static bool
 AstDecodeExport(AstDecodeContext& c, AstExport** export_)
 {
     AstName fieldName;
     if (!AstDecodeName(c, &fieldName))
--- a/js/src/wasm/WasmCompile.cpp
+++ b/js/src/wasm/WasmCompile.cpp
@@ -465,52 +465,16 @@ DecodeMemorySection(Decoder& d, ModuleGe
         init->memoryUsage = MemoryUsage::Unshared;
         init->minMemoryLength = memory.initial;
         init->maxMemoryLength = memory.maximum;
     }
 
     return true;
 }
 
-static bool
-DecodeGlobalSection(Decoder& d, ModuleGeneratorData* init)
-{
-    uint32_t sectionStart, sectionSize;
-    if (!d.startSection(SectionId::Global, &sectionStart, &sectionSize, "global"))
-        return false;
-    if (sectionStart == Decoder::NotStarted)
-        return true;
-
-    uint32_t numGlobals;
-    if (!d.readVarU32(&numGlobals))
-        return d.fail("expected number of globals");
-
-    if (numGlobals > MaxGlobals)
-        return d.fail("too many globals");
-
-    for (uint32_t i = 0; i < numGlobals; i++) {
-        ValType type;
-        bool isMutable;
-        if (!DecodeGlobalType(d, &type, &isMutable))
-            return false;
-
-        InitExpr initializer;
-        if (!DecodeInitializerExpression(d, init->globals, type, &initializer))
-            return false;
-
-        if (!init->globals.append(GlobalDesc(initializer, isMutable)))
-            return false;
-    }
-
-    if (!d.finishSection(sectionStart, sectionSize, "global"))
-        return false;
-
-    return true;
-}
-
 typedef HashSet<const char*, CStringHasher, SystemAllocPolicy> CStringSet;
 
 static UniqueChars
 DecodeExportName(Decoder& d, CStringSet* dupSet)
 {
     UniqueChars exportName = DecodeName(d);
     if (!exportName) {
         d.fail("expected valid export name");
@@ -898,17 +862,17 @@ wasm::Compile(const ShareableBytes& byte
         return nullptr;
 
     if (!DecodeTableSection(d, &init->tables))
         return nullptr;
 
     if (!::DecodeMemorySection(d, init.get()))
         return nullptr;
 
-    if (!DecodeGlobalSection(d, init.get()))
+    if (!DecodeGlobalSection(d, &init->globals))
         return nullptr;
 
     ModuleGenerator mg(Move(imports));
     if (!mg.init(Move(init), args))
         return nullptr;
 
     if (!DecodeExportSection(d, mg))
         return nullptr;