From 14b2ae493545d7618149c5dac40d2e55712fcd2e Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 16:41:53 +0200 Subject: [PATCH 1/8] Replace Type::expand() with an interator-based approach --- src/asmjs/asm_v_wasm.cpp | 4 +- src/binaryen-c.cpp | 7 ++-- src/passes/Asyncify.cpp | 15 ++++--- src/passes/DeadArgumentElimination.cpp | 2 +- src/passes/FuncCastEmulation.cpp | 6 +-- src/passes/I64ToI32Lowering.cpp | 2 +- src/passes/LegalizeJSInterface.cpp | 16 ++++---- src/passes/Print.cpp | 32 +++++++-------- src/shell-interface.h | 8 ++-- src/tools/execution-results.h | 2 +- src/tools/fuzzing.h | 23 ++++++----- src/tools/js-wrapper.h | 2 +- src/tools/spec-wrapper.h | 2 +- src/tools/wasm-shell.cpp | 2 +- src/tools/wasm2c-wrapper.h | 11 +++-- src/wasm-builder.h | 2 +- src/wasm-interpreter.h | 8 ++-- src/wasm-type.h | 45 ++++++++++++++++++-- src/wasm/literal.cpp | 2 +- src/wasm/wasm-binary.cpp | 18 ++++---- src/wasm/wasm-emscripten.cpp | 15 ++++--- src/wasm/wasm-s-parser.cpp | 6 +-- src/wasm/wasm-stack.cpp | 9 ++-- src/wasm/wasm-type.cpp | 51 +++++++++-------------- src/wasm/wasm-validator.cpp | 57 ++++++++++++++------------ src/wasm/wasm.cpp | 4 +- 26 files changed, 190 insertions(+), 161 deletions(-) diff --git a/src/asmjs/asm_v_wasm.cpp b/src/asmjs/asm_v_wasm.cpp index 75fc0506bf3..fca6284c64f 100644 --- a/src/asmjs/asm_v_wasm.cpp +++ b/src/asmjs/asm_v_wasm.cpp @@ -104,8 +104,8 @@ std::string getSig(Type results, Type params) { assert(!results.isMulti()); std::string sig; sig += getSig(results); - for (Type t : params.expand()) { - sig += getSig(t); + for (auto& param : params) { + sig += getSig(param); } return sig; } diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 2201f97bd3b..4d057d6144d 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -150,9 +150,10 @@ BinaryenType BinaryenTypeCreate(BinaryenType* types, uint32_t numTypes) { uint32_t BinaryenTypeArity(BinaryenType t) { return Type(t).size(); } void BinaryenTypeExpand(BinaryenType t, BinaryenType* buf) { - const std::vector& types = Type(t).expand(); - for (size_t i = 0; i < types.size(); ++i) { - buf[i] = types[i].getID(); + Type types(t); + size_t i = 0; + for (auto& type : types) { + buf[i++] = type.getID(); } } diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp index 2aca1694260..b94b40cfb6c 100644 --- a/src/passes/Asyncify.cpp +++ b/src/passes/Asyncify.cpp @@ -1304,10 +1304,9 @@ struct AsyncifyLocals : public WalkerPass> { if (!relevantLiveLocals.count(i)) { continue; } - const auto& types = func->getLocalType(i).expand(); + auto localType = func->getLocalType(i); SmallVector loads; - for (Index j = 0; j < types.size(); j++) { - auto type = types[j]; + for (auto type : localType) { auto size = type.getByteSize(); assert(size % STACK_ALIGN == 0); // TODO: higher alignment? @@ -1323,7 +1322,7 @@ struct AsyncifyLocals : public WalkerPass> { Expression* load; if (loads.size() == 1) { load = loads[0]; - } else if (types.size() > 1) { + } else if (localType.size() > 1) { load = builder->makeTupleMake(std::move(loads)); } else { WASM_UNREACHABLE("Unexpected empty type"); @@ -1350,12 +1349,11 @@ struct AsyncifyLocals : public WalkerPass> { continue; } auto localType = func->getLocalType(i); - const auto& types = localType.expand(); - for (Index j = 0; j < types.size(); j++) { - auto type = types[j]; + size_t j = 0; + for (auto type : localType) { auto size = type.getByteSize(); Expression* localGet = builder->makeLocalGet(i, localType); - if (types.size() > 1) { + if (localType.size() > 1) { localGet = builder->makeTupleExtract(localGet, j); } assert(size % STACK_ALIGN == 0); @@ -1368,6 +1366,7 @@ struct AsyncifyLocals : public WalkerPass> { localGet, type)); offset += size; + ++j; } } block->list.push_back(builder->makeIncStackPos(offset)); diff --git a/src/passes/DeadArgumentElimination.cpp b/src/passes/DeadArgumentElimination.cpp index a20ff1fb83e..9fed4f1dc74 100644 --- a/src/passes/DeadArgumentElimination.cpp +++ b/src/passes/DeadArgumentElimination.cpp @@ -409,7 +409,7 @@ struct DAE : public Pass { // Remove the parameter from the function. We must add a new local // for uses of the parameter, but cannot make it use the same index // (in general). - std::vector params = func->sig.params.expand(); + std::vector params(func->sig.params.begin(), func->sig.params.end()); auto type = params[i]; params.erase(params.begin() + i); func->sig.params = Type(params); diff --git a/src/passes/FuncCastEmulation.cpp b/src/passes/FuncCastEmulation.cpp index 9e455b0c8b2..a2f47ca90b5 100644 --- a/src/passes/FuncCastEmulation.cpp +++ b/src/passes/FuncCastEmulation.cpp @@ -193,13 +193,13 @@ struct FuncCastEmulation : public Pass { } // The item in the table may be a function or a function import. auto* func = module->getFunction(name); - const std::vector& params = func->sig.params.expand(); Type type = func->sig.results; Builder builder(*module); std::vector callOperands; - for (Index i = 0; i < params.size(); i++) { + Index i = 0; + for (auto& param : func->sig.params) { callOperands.push_back( - fromABI(builder.makeLocalGet(i, Type::i64), params[i], module)); + fromABI(builder.makeLocalGet(i++, Type::i64), param, module)); } auto* call = builder.makeCall(name, callOperands, type); std::vector thunkParams; diff --git a/src/passes/I64ToI32Lowering.cpp b/src/passes/I64ToI32Lowering.cpp index f66af87cc62..41d0e799a0e 100644 --- a/src/passes/I64ToI32Lowering.cpp +++ b/src/passes/I64ToI32Lowering.cpp @@ -272,7 +272,7 @@ struct I64ToI32Lowering : public WalkerPass> { visitGenericCall( curr, [&](std::vector& args, Type results) { std::vector params; - for (auto param : curr->sig.params.expand()) { + for (auto& param : curr->sig.params) { if (param == Type::i64) { params.push_back(Type::i32); params.push_back(Type::i32); diff --git a/src/passes/LegalizeJSInterface.cpp b/src/passes/LegalizeJSInterface.cpp index 65916530615..9a4dc6b6398 100644 --- a/src/passes/LegalizeJSInterface.cpp +++ b/src/passes/LegalizeJSInterface.cpp @@ -183,7 +183,7 @@ struct LegalizeJSInterface : public Pass { std::map illegalImportsToLegal; template bool isIllegal(T* t) { - for (auto param : t->sig.params.expand()) { + for (auto& param : t->sig.params) { if (param == Type::i64) { return true; } @@ -222,9 +222,8 @@ struct LegalizeJSInterface : public Pass { call->target = func->name; call->type = func->sig.results; - const std::vector& params = func->sig.params.expand(); std::vector legalParams; - for (auto param : params) { + for (auto& param : func->sig.params) { if (param == Type::i64) { call->operands.push_back(I64Utilities::recreateI64( builder, legalParams.size(), legalParams.size() + 1)); @@ -277,18 +276,19 @@ struct LegalizeJSInterface : public Pass { auto* call = module->allocator.alloc(); call->target = legalIm->name; - const std::vector& imParams = im->sig.params.expand(); std::vector params; - for (size_t i = 0; i < imParams.size(); ++i) { - if (imParams[i] == Type::i64) { + Index i = 0; + for (auto& param : im->sig.params) { + if (param == Type::i64) { call->operands.push_back(I64Utilities::getI64Low(builder, i)); call->operands.push_back(I64Utilities::getI64High(builder, i)); params.push_back(Type::i32); params.push_back(Type::i32); } else { - call->operands.push_back(builder.makeLocalGet(i, imParams[i])); - params.push_back(imParams[i]); + call->operands.push_back(builder.makeLocalGet(i, param)); + params.push_back(param); } + ++i; } if (im->sig.results == Type::i64) { diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index d90e7f95835..c0702df55d9 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -67,15 +67,16 @@ struct SExprType { static std::ostream& operator<<(std::ostream& o, const SExprType& localType) { Type type = localType.type; if (type.isMulti()) { - const std::vector& types = type.expand(); - o << '(' << types[0]; - for (size_t i = 1; i < types.size(); ++i) { - o << ' ' << types[i]; + o << '('; + auto ws = ""; + for (auto& t : type) { + o << ws << t; + ws = " "; } o << ')'; - return o; + } else { + o << type; } - o << type; return o; } @@ -90,12 +91,10 @@ std::ostream& operator<<(std::ostream& os, SigName sigName) { if (type == Type::none) { os << "none"; } else { - const std::vector& types = type.expand(); - for (size_t i = 0; i < types.size(); ++i) { - if (i != 0) { - os << '_'; - } - os << types[i]; + auto us = ""; + for (auto& t : type) { + os << us << t; + us = "_"; } } }; @@ -2169,14 +2168,15 @@ struct PrintSExpression : public OverriddenVisitor { if (!printStackIR && curr->stackIR && !minify) { o << " (; has Stack IR ;)"; } - const std::vector& params = curr->sig.params.expand(); - if (params.size() > 0) { - for (size_t i = 0; i < params.size(); i++) { + if (curr->sig.params.size() > 0) { + Index i = 0; + for (auto& param : curr->sig.params) { o << maybeSpace; o << '('; printMinor(o, "param "); printLocal(i, currFunction, o); - o << ' ' << params[i] << ')'; + o << ' ' << param << ')'; + ++i; } } if (curr->sig.results != Type::none) { diff --git a/src/shell-interface.h b/src/shell-interface.h index 963832fed97..0667e43a12d 100644 --- a/src/shell-interface.h +++ b/src/shell-interface.h @@ -165,12 +165,12 @@ struct ShellExternalInterface : ModuleInstance::ExternalInterface { if (sig != func->sig) { trap("callIndirect: function signatures don't match"); } - const std::vector& params = func->sig.params.expand(); - if (params.size() != arguments.size()) { + if (func->sig.params.size() != arguments.size()) { trap("callIndirect: bad # of arguments"); } - for (size_t i = 0; i < params.size(); i++) { - if (!Type::isSubType(arguments[i].type, params[i])) { + size_t i = 0; + for (auto& param : func->sig.params) { + if (!Type::isSubType(arguments[i++].type, param)) { trap("callIndirect: bad argument type"); } } diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index e5d4dab005d..dc62bba386a 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -144,7 +144,7 @@ struct ExecutionResults { instance.callFunction(ex->value, arguments); } // call the method - for (Type param : func->sig.params.expand()) { + for (auto& param : func->sig.params) { // zeros in arguments TODO: more? arguments.push_back(Literal::makeSingleZero(param)); } diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index 7fef9c20888..c76b6e13a94 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -308,7 +308,7 @@ class TranslateToFuzzReader { Type getSubType(Type type) { if (type.isMulti()) { std::vector types; - for (auto t : type.expand()) { + for (auto& t : type) { types.push_back(getSubType(t)); } return Type(types); @@ -770,7 +770,7 @@ class TranslateToFuzzReader { std::vector invocations; while (oneIn(2) && !finishedInput) { std::vector args; - for (auto type : func->sig.params.expand()) { + for (auto& type : func->sig.params) { args.push_back(makeConst(type)); } Expression* invoke = @@ -1166,7 +1166,7 @@ class TranslateToFuzzReader { } // we found one! std::vector args; - for (auto argType : target->sig.params.expand()) { + for (auto& argType : target->sig.params) { args.push_back(make(argType)); } return builder.makeCall(target->name, args, type, isReturn); @@ -1210,7 +1210,7 @@ class TranslateToFuzzReader { target = make(Type::i32); } std::vector args; - for (auto type : targetFn->sig.params.expand()) { + for (auto& type : targetFn->sig.params) { args.push_back(make(type)); } return builder.makeCallIndirect(target, args, targetFn->sig, isReturn); @@ -1267,7 +1267,7 @@ class TranslateToFuzzReader { assert(wasm.features.hasMultivalue()); assert(type.isMulti()); std::vector elements; - for (auto t : type.expand()) { + for (auto& t : type) { elements.push_back(make(t)); } return builder.makeTupleMake(std::move(elements)); @@ -1277,20 +1277,21 @@ class TranslateToFuzzReader { assert(wasm.features.hasMultivalue()); assert(type.isSingle() && type.isConcrete()); Type tupleType = getTupleType(); - auto& elements = tupleType.expand(); // Find indices from which we can extract `type` std::vector extractIndices; - for (size_t i = 0; i < elements.size(); ++i) { - if (elements[i] == type) { + size_t i = 0; + for (auto& t : tupleType) { + if (t == type) { extractIndices.push_back(i); } + ++i; } // If there are none, inject one if (extractIndices.size() == 0) { - auto newElements = elements; - size_t injected = upTo(elements.size()); + std::vector newElements(tupleType.begin(), tupleType.end()); + size_t injected = upTo(newElements.size()); newElements[injected] = type; tupleType = Type(newElements); extractIndices.push_back(injected); @@ -1766,7 +1767,7 @@ class TranslateToFuzzReader { } if (type.isMulti()) { std::vector operands; - for (auto t : type.expand()) { + for (auto& t : type) { operands.push_back(makeConst(t)); } return builder.makeTupleMake(std::move(operands)); diff --git a/src/tools/js-wrapper.h b/src/tools/js-wrapper.h index 0a9b5925ed5..58e52b78216 100644 --- a/src/tools/js-wrapper.h +++ b/src/tools/js-wrapper.h @@ -99,7 +99,7 @@ static std::string generateJSWrapper(Module& wasm) { } ret += std::string("instance.exports.") + exp->name.str + "("; bool first = true; - for (Type param : func->sig.params.expand()) { + for (auto& param : func->sig.params) { // zeros in arguments TODO more? if (first) { first = false; diff --git a/src/tools/spec-wrapper.h b/src/tools/spec-wrapper.h index 3a674e495c1..513cc8bda6a 100644 --- a/src/tools/spec-wrapper.h +++ b/src/tools/spec-wrapper.h @@ -30,7 +30,7 @@ static std::string generateSpecWrapper(Module& wasm) { } ret += std::string("(invoke \"hangLimitInitializer\") (invoke \"") + exp->name.str + "\" "; - for (Type param : func->sig.params.expand()) { + for (auto& param : func->sig.params) { // zeros in arguments TODO more? TODO_SINGLE_COMPOUND(param); switch (param.getBasic()) { diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index 1430c18914f..1093ff85ce7 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -111,7 +111,7 @@ static void run_asserts(Name moduleName, std::cerr << "Unknown entry " << entry << std::endl; } else { LiteralList arguments; - for (Type param : function->sig.params.expand()) { + for (auto& param : function->sig.params) { arguments.push_back(Literal(param)); } try { diff --git a/src/tools/wasm2c-wrapper.h b/src/tools/wasm2c-wrapper.h index eae92ad60c8..d724f2580d9 100644 --- a/src/tools/wasm2c-wrapper.h +++ b/src/tools/wasm2c-wrapper.h @@ -144,7 +144,6 @@ int main(int argc, char** argv) { // Emit the callee's name with wasm2c name mangling. ret += std::string("(*Z_") + exp->name.str + "Z_"; - auto params = func->sig.params.expand(); auto wasm2cSignature = [](Type type) { TODO_SINGLE_COMPOUND(type); @@ -165,18 +164,18 @@ int main(int argc, char** argv) { }; ret += wasm2cSignature(result); - if (params.empty()) { - ret += wasm2cSignature(Type::none); - } else { - for (auto param : params) { + if (func->sig.params.isMulti()) { + for (auto& param : func->sig.params) { ret += wasm2cSignature(param); } + } else { + ret += wasm2cSignature(func->sig.params); } ret += ")("; // Emit the parameters (all 0s, like the other wrappers). bool first = true; - for (auto param : params) { + for (auto param : func->sig.params) { WASM_UNUSED(param); if (!first) { ret += ", "; diff --git a/src/wasm-builder.h b/src/wasm-builder.h index 9ee98949a85..7cf8b40438a 100644 --- a/src/wasm-builder.h +++ b/src/wasm-builder.h @@ -657,7 +657,7 @@ class Builder { // only ok to add a param if no vars, otherwise indices are invalidated assert(func->localIndices.size() == func->sig.params.size()); assert(name.is()); - std::vector params = func->sig.params.expand(); + std::vector params(func->sig.params.begin(), func->sig.params.end()); params.push_back(type); func->sig.params = Type(params); Index index = func->localNames.size(); diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 4f0d22e58aa..baeda061290 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -1890,14 +1890,12 @@ template class ModuleInstanceBase { WASM_UNREACHABLE("invalid param count"); } locals.resize(function->getNumLocals()); - const std::vector& params = function->sig.params.expand(); for (size_t i = 0; i < function->getNumLocals(); i++) { if (i < arguments.size()) { - assert(i < params.size()); - if (!Type::isSubType(arguments[i].type, params[i])) { + if (!Type::isSubType(arguments[i].type, function->sig.params[i])) { std::cerr << "Function `" << function->name << "` expects type " - << params[i] << " for parameter " << i << ", got " - << arguments[i].type << "." << std::endl; + << function->sig.params[i] << " for parameter " << i + << ", got " << arguments[i].type << "." << std::endl; WASM_UNREACHABLE("invalid param count"); } locals[i] = {arguments[i]}; diff --git a/src/wasm-type.h b/src/wasm-type.h index 31c91562008..e44e6a66d52 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -69,7 +69,6 @@ class Type { // Accessors size_t size() const; - const std::vector& expand() const; // Predicates // Compound Concrete @@ -116,8 +115,8 @@ class Type { private: template bool hasPredicate() { - for (auto t : expand()) { - if ((t.*pred)()) { + for (auto& type : *this) { + if ((type.*pred)()) { return true; } } @@ -177,6 +176,46 @@ class Type { } std::string toString() const; + + Type& operator[](size_t i) { + if (isMulti()) { + return (*(std::vector*)getID())[i]; + } + assert(id != Type::none && i == 0 && "Index out of bounds"); + return *this; + } + + struct Iterator + : std::iterator { + const Type* parent; + size_t index; + Iterator(const Type* parent, size_t index) : parent(parent), index(index) {} + bool operator==(const Iterator& other) const { + return index == other.index && parent == other.parent; + } + bool operator!=(const Iterator& other) const { return !(*this == other); } + void operator++() { index++; } + Iterator& operator+=(difference_type off) { + index += off; + return *this; + } + const Iterator operator+(difference_type off) const { + return Iterator(*this) += off; + } + difference_type operator-(const Iterator& other) { + assert(parent == other.parent); + return index - other.index; + } + const value_type& operator*() const { + if (parent->isMulti()) { + return (*(std::vector*)parent->getID())[index]; + } + return *parent; + } + }; + + Iterator begin() const { return Iterator(this, 0); } + Iterator end() const { return Iterator(this, size()); } }; // Wrapper type for formatting types as "(param i32 i64 f32)" diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 699d3ea63d8..11130bc18b1 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -102,7 +102,7 @@ Literal::Literal(const LaneArray<2>& lanes) : type(Type::v128) { Literals Literal::makeZero(Type type) { assert(type.isConcrete()); Literals zeroes; - for (auto t : type.expand()) { + for (auto& t : type) { zeroes.push_back(makeSingleZero(t)); } return zeroes; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index ddc4de36c22..285fd578f7a 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -220,7 +220,7 @@ void WasmBinaryWriter::writeTypes() { o << S32LEB(BinaryConsts::EncodedType::Func); for (auto& sigType : {sig.params, sig.results}) { o << U32LEB(sigType.size()); - for (auto type : sigType.expand()) { + for (auto& type : sigType) { o << binaryType(type); } } @@ -385,16 +385,17 @@ void WasmBinaryWriter::writeGlobals() { o << U32LEB(num); ModuleUtils::iterDefinedGlobals(*wasm, [&](Global* global) { BYN_TRACE("write one\n"); - const auto& types = global->type.expand(); - for (size_t i = 0; i < types.size(); ++i) { - o << binaryType(types[i]); + size_t i = 0; + for (auto& t : global->type) { + o << binaryType(t); o << U32LEB(global->mutable_); - if (types.size() == 1) { + if (global->type.size() == 1) { writeExpression(global->init); } else { writeExpression(global->init->cast()->operands[i]); } o << int8_t(BinaryConsts::End); + ++i; } }); finishSection(start); @@ -1385,7 +1386,9 @@ void WasmBinaryBuilder::readImports() { wasm.addEvent(curr); break; } - default: { throwError("bad import kind"); } + default: { + throwError("bad import kind"); + } } } } @@ -1795,8 +1798,7 @@ void WasmBinaryBuilder::pushExpression(Expression* curr) { Builder builder(wasm); Index tuple = builder.addVar(currFunction, curr->type); expressionStack.push_back(builder.makeLocalSet(tuple, curr)); - const std::vector types = curr->type.expand(); - for (Index i = 0; i < types.size(); ++i) { + for (Index i = 0; i < curr->type.size(); ++i) { expressionStack.push_back( builder.makeTupleExtract(builder.makeLocalGet(tuple, curr->type), i)); } diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index e4664e645c6..0e982acb62a 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -154,15 +154,15 @@ void EmscriptenGlueGenerator::generateDynCallThunk(Signature sig) { std::vector params; params.emplace_back("fptr", Type::i32); // function pointer param int p = 0; - const std::vector& paramTypes = sig.params.expand(); - for (const auto& ty : paramTypes) { - params.emplace_back(std::to_string(p++), ty); + for (auto& param : sig.params) { + params.emplace_back(std::to_string(p++), param); } Function* f = builder.makeFunction(name, std::move(params), sig.results, {}); Expression* fptr = builder.makeLocalGet(0, Type::i32); std::vector args; - for (unsigned i = 0; i < paramTypes.size(); ++i) { - args.push_back(builder.makeLocalGet(i + 1, paramTypes[i])); + Index i = 0; + for (auto& param : sig.params) { + args.push_back(builder.makeLocalGet(++i, param)); } Expression* call = builder.makeCallIndirect(fptr, args, sig); f->body = call; @@ -486,7 +486,7 @@ AsmConstWalker::AsmConst& AsmConstWalker::createAsmConst(uint32_t id, } Signature AsmConstWalker::asmConstSig(Signature baseSig) { - std::vector params = baseSig.params.expand(); + std::vector params(baseSig.params.begin(), baseSig.params.end()); assert(params.size() >= 1); // Omit the signature of the "code" parameter, taken as a string, as the // first argument @@ -649,8 +649,7 @@ struct FixInvokeFunctionNamesWalker return name; } - const std::vector& params = sig.params.expand(); - std::vector newParams(params.begin() + 1, params.end()); + std::vector newParams(sig.params.begin() + 1, sig.params.end()); Signature sigWoOrigFunc = Signature(Type(newParams), sig.results); invokeSigs.insert(sigWoOrigFunc); return Name("invoke_" + diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 5e6008bca36..5e6647a9878 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -640,9 +640,9 @@ SExpressionWasmBuilder::parseTypeUse(Element& s, // If only (type) is specified, populate `namedParams` if (!paramsOrResultsExist) { - const std::vector& funcParams = functionSignature.params.expand(); - for (size_t index = 0, e = funcParams.size(); index < e; index++) { - namedParams.emplace_back(Name::fromInt(index), funcParams[index]); + size_t index = 0; + for (auto param : functionSignature.params) { + namedParams.emplace_back(Name::fromInt(index++), param); } } diff --git a/src/wasm/wasm-stack.cpp b/src/wasm/wasm-stack.cpp index 19a4590e31b..e64e330e8c3 100644 --- a/src/wasm/wasm-stack.cpp +++ b/src/wasm/wasm-stack.cpp @@ -1808,17 +1808,16 @@ void BinaryInstWriter::mapLocalsAndEmitHeader() { return; } for (auto type : func->vars) { - for (auto t : type.expand()) { + for (auto& t : type) { numLocalsByType[t]++; } } countScratchLocals(); std::map currLocalsByType; for (Index i = func->getVarIndexBase(); i < func->getNumLocals(); i++) { - const std::vector types = func->getLocalType(i).expand(); - for (Index j = 0; j < types.size(); j++) { - Type type = types[j]; - auto fullIndex = std::make_pair(i, j); + Index j = 0; + for (auto& type : func->getLocalType(i)) { + auto fullIndex = std::make_pair(i, j++); Index index = func->getVarIndexBase(); for (auto& typeCount : numLocalsByType) { if (type == typeCount.first) { diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 5db04b40d42..19ac6e06bf5 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -123,23 +123,18 @@ Type::Type(std::initializer_list types) { init(types); } Type::Type(const std::vector& types) { init(types); } -size_t Type::size() const { return expand().size(); } - -const std::vector& Type::expand() const { - if (id <= _last_value_type) { - return basicTypes[id]; - } else { - return *(std::vector*)id; +size_t Type::size() const { + if (isMulti()) { + return (*(std::vector*)getID()).size(); } + return size_t(getID() != Type::none); } bool Type::operator<(const Type& other) const { - const std::vector& these = expand(); - const std::vector& others = other.expand(); - return std::lexicographical_compare(these.begin(), - these.end(), - others.begin(), - others.end(), + return std::lexicographical_compare((*this).begin(), + (*this).end(), + other.begin(), + other.end(), [](const Type& a, const Type& b) { TODO_SINGLE_COMPOUND(a); TODO_SINGLE_COMPOUND(b); @@ -172,7 +167,7 @@ unsigned Type::getByteSize() const { if (isMulti()) { unsigned size = 0; - for (auto t : expand()) { + for (auto& t : *this) { size += getSingleByteSize(t); } return size; @@ -181,7 +176,7 @@ unsigned Type::getByteSize() const { } Type Type::reinterpret() const { - Type singleType = *expand().begin(); + auto singleType = *(*this).begin(); switch (singleType.getBasic()) { case Type::i32: return f32; @@ -222,7 +217,7 @@ FeatureSet Type::getFeatures() const { if (isMulti()) { FeatureSet feats = FeatureSet::Multivalue; - for (Type t : expand()) { + for (auto& t : *this) { feats |= getSingleFeatures(t); } return feats; @@ -255,13 +250,11 @@ bool Type::isSubType(Type left, Type right) { return true; } if (left.isMulti() && right.isMulti()) { - const auto& leftElems = left.expand(); - const auto& rightElems = right.expand(); - if (leftElems.size() != rightElems.size()) { + if (left.size() != right.size()) { return false; } - for (size_t i = 0; i < leftElems.size(); ++i) { - if (!isSubType(leftElems[i], rightElems[i])) { + for (size_t i = 0; i < left.size(); ++i) { + if (!isSubType(left[i], right[i])) { return false; } } @@ -286,10 +279,8 @@ Type Type::getLeastUpperBound(Type a, Type b) { if (a.isMulti()) { std::vector types; types.resize(a.size()); - const auto& as = a.expand(); - const auto& bs = b.expand(); for (size_t i = 0; i < types.size(); ++i) { - types[i] = getLeastUpperBound(as[i], bs[i]); + types[i] = getLeastUpperBound(a[i], b[i]); if (types[i] == Type::none) { return Type::none; } @@ -313,7 +304,7 @@ namespace { std::ostream& printPrefixedTypes(std::ostream& os, const char* prefix, Type type) { os << '(' << prefix; - for (auto t : type.expand()) { + for (auto& t : type) { os << " " << t; } os << ')'; @@ -347,12 +338,10 @@ bool Signature::operator<(const Signature& other) const { std::ostream& operator<<(std::ostream& os, Type type) { if (type.isMulti()) { os << '('; - const std::vector& types = type.expand(); - for (size_t i = 0; i < types.size(); ++i) { - os << types[i]; - if (i < types.size() - 1) { - os << ", "; - } + auto sep = ""; + for (auto& t : type) { + os << sep << t; + sep = ", "; } os << ')'; } else { diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 4921dafdd35..1ff04906e24 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -641,20 +641,21 @@ void FunctionValidator::visitCall(Call* curr) { if (!shouldBeTrue(!!target, curr, "call target must exist")) { return; } - const std::vector params = target->sig.params.expand(); - if (!shouldBeTrue(curr->operands.size() == params.size(), + if (!shouldBeTrue(curr->operands.size() == target->sig.params.size(), curr, "call param number must match")) { return; } - for (size_t i = 0; i < curr->operands.size(); i++) { + size_t i = 0; + for (auto& param : target->sig.params) { if (!shouldBeSubTypeOrFirstIsUnreachable(curr->operands[i]->type, - params[i], + param, curr, "call param types must match") && !info.quiet) { getStream() << "(on argument " << i << ")\n"; } + ++i; } if (curr->isReturn) { shouldBeEqual(curr->type, @@ -692,24 +693,25 @@ void FunctionValidator::visitCallIndirect(CallIndirect* curr) { if (!info.validateGlobally) { return; } - const std::vector& params = curr->sig.params.expand(); shouldBeEqualOrFirstIsUnreachable(curr->target->type, Type(Type::i32), curr, "indirect call target must be an i32"); - if (!shouldBeTrue(curr->operands.size() == params.size(), + if (!shouldBeTrue(curr->operands.size() == curr->sig.params.size(), curr, "call param number must match")) { return; } - for (size_t i = 0; i < curr->operands.size(); i++) { + size_t i = 0; + for (auto& param : curr->sig.params) { if (!shouldBeSubTypeOrFirstIsUnreachable(curr->operands[i]->type, - params[i], + param, curr, "call param types must match") && !info.quiet) { getStream() << "(on argument " << i << ")\n"; } + ++i; } if (curr->isReturn) { shouldBeEqual(curr->type, @@ -1871,15 +1873,16 @@ void FunctionValidator::visitThrow(Throw* curr) { "event's param numbers must match")) { return; } - const std::vector& paramTypes = event->sig.params.expand(); - for (size_t i = 0; i < curr->operands.size(); i++) { + size_t i = 0; + for (auto& param : event->sig.params) { if (!shouldBeSubTypeOrFirstIsUnreachable(curr->operands[i]->type, - paramTypes[i], + param, curr->operands[i], "event param types must match") && !info.quiet) { getStream() << "(on argument " << i << ")\n"; } + ++i; } } @@ -1957,7 +1960,7 @@ void FunctionValidator::visitTupleExtract(TupleExtract* curr) { shouldBeTrue(inBounds, curr, "tuple.extract index out of bounds"); if (inBounds) { shouldBeSubType( - curr->tuple->type.expand()[curr->index], + curr->tuple->type[curr->index], curr->type, curr, "tuple.extract type does not match the type of the extracted element"); @@ -1972,17 +1975,17 @@ void FunctionValidator::visitFunction(Function* curr) { "Multivalue function results (multivalue is not enabled)"); } FeatureSet features; - for (auto type : curr->sig.params.expand()) { - features |= type.getFeatures(); - shouldBeTrue(type.isConcrete(), curr, "params must be concretely typed"); + for (auto& param : curr->sig.params) { + features |= param.getFeatures(); + shouldBeTrue(param.isConcrete(), curr, "params must be concretely typed"); } - for (auto type : curr->sig.results.expand()) { - features |= type.getFeatures(); - shouldBeTrue(type.isConcrete(), curr, "results must be concretely typed"); + for (auto& result : curr->sig.results) { + features |= result.getFeatures(); + shouldBeTrue(result.isConcrete(), curr, "results must be concretely typed"); } - for (auto type : curr->vars) { - features |= type.getFeatures(); - shouldBeTrue(type.isConcrete(), curr, "vars must be concretely typed"); + for (auto& var : curr->vars) { + features |= var.getFeatures(); + shouldBeTrue(var.isConcrete(), curr, "vars must be concretely typed"); } shouldBeTrue(features <= getModule()->features, curr, @@ -2141,13 +2144,13 @@ static void validateImports(Module& module, ValidationInfo& info) { "(multivalue is not enabled)"); } if (info.validateWeb) { - for (Type param : curr->sig.params.expand()) { + for (auto& param : curr->sig.params) { info.shouldBeUnequal(param, Type(Type::i64), curr->name, "Imported function must not have i64 parameters"); } - for (Type result : curr->sig.results.expand()) { + for (auto& result : curr->sig.results) { info.shouldBeUnequal(result, Type(Type::i64), curr->name, @@ -2170,14 +2173,14 @@ static void validateExports(Module& module, ValidationInfo& info) { if (curr->kind == ExternalKind::Function) { if (info.validateWeb) { Function* f = module.getFunction(curr->value); - for (auto param : f->sig.params.expand()) { + for (auto& param : f->sig.params) { info.shouldBeUnequal( param, Type(Type::i64), f->name, "Exported function must not have i64 parameters"); } - for (auto result : f->sig.results.expand()) { + for (auto& result : f->sig.results) { info.shouldBeUnequal(result, Type(Type::i64), f->name, @@ -2349,8 +2352,8 @@ static void validateEvents(Module& module, ValidationInfo& info) { curr->name, "Multivalue event type (multivalue is not enabled)"); } - for (auto type : curr->sig.params.expand()) { - info.shouldBeTrue(type.isConcrete(), + for (auto& param : curr->sig.params) { + info.shouldBeTrue(param.isConcrete(), curr->name, "Values in an event should have concrete types"); } diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index be3ab6ccca9..387ea577fea 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -948,7 +948,7 @@ void TupleExtract::finalize() { if (tuple->type == Type::unreachable) { type = Type::unreachable; } else { - type = tuple->type.expand()[index]; + type = tuple->type[index]; } } @@ -1006,7 +1006,7 @@ Index Function::getVarIndexBase() { return sig.params.size(); } Type Function::getLocalType(Index index) { auto numParams = sig.params.size(); if (index < numParams) { - return sig.params.expand()[index]; + return sig.params[index]; } else if (isVar(index)) { return vars[index - numParams]; } else { From 91266978181415dea07fb19c6bdd138fcdd75b22 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 17:47:11 +0200 Subject: [PATCH 2/8] sep --- src/passes/Print.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index c0702df55d9..ba4e022e029 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -68,10 +68,10 @@ static std::ostream& operator<<(std::ostream& o, const SExprType& localType) { Type type = localType.type; if (type.isMulti()) { o << '('; - auto ws = ""; + auto sep = ""; for (auto& t : type) { - o << ws << t; - ws = " "; + o << sep << t; + sep = " "; } o << ')'; } else { @@ -91,10 +91,10 @@ std::ostream& operator<<(std::ostream& os, SigName sigName) { if (type == Type::none) { os << "none"; } else { - auto us = ""; + auto sep = ""; for (auto& t : type) { - os << us << t; - us = "_"; + os << sep << t; + sep = "_"; } } }; From adc06d3cdd19cdd9f9b5c8c60c15312022d59ff8 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 17:48:14 +0200 Subject: [PATCH 3/8] add an assert --- src/wasm-type.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wasm-type.h b/src/wasm-type.h index e44e6a66d52..50f5b534a50 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -210,6 +210,7 @@ class Type { if (parent->isMulti()) { return (*(std::vector*)parent->getID())[index]; } + assert(*parent != Type::none); return *parent; } }; From 59ac428a0df3defd7c3ae9d8bbe01cf40ac08ad6 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 17:51:00 +0200 Subject: [PATCH 4/8] add a better assert --- src/wasm-type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index 50f5b534a50..ea2f4a1328d 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -210,7 +210,7 @@ class Type { if (parent->isMulti()) { return (*(std::vector*)parent->getID())[index]; } - assert(*parent != Type::none); + assert(index == 0 && *parent != Type::none && "Index out of bounds"); return *parent; } }; From 106475c3c71d780ca79f6ffc117704f064827af5 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 19:42:14 +0200 Subject: [PATCH 5/8] more --- src/passes/Asyncify.cpp | 4 ++-- src/tools/wasm2c-wrapper.h | 2 +- src/wasm/wasm-emscripten.cpp | 8 ++++---- src/wasm/wasm-s-parser.cpp | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp index b94b40cfb6c..1bbc89435fa 100644 --- a/src/passes/Asyncify.cpp +++ b/src/passes/Asyncify.cpp @@ -1306,7 +1306,7 @@ struct AsyncifyLocals : public WalkerPass> { } auto localType = func->getLocalType(i); SmallVector loads; - for (auto type : localType) { + for (auto& type : localType) { auto size = type.getByteSize(); assert(size % STACK_ALIGN == 0); // TODO: higher alignment? @@ -1350,7 +1350,7 @@ struct AsyncifyLocals : public WalkerPass> { } auto localType = func->getLocalType(i); size_t j = 0; - for (auto type : localType) { + for (auto& type : localType) { auto size = type.getByteSize(); Expression* localGet = builder->makeLocalGet(i, localType); if (localType.size() > 1) { diff --git a/src/tools/wasm2c-wrapper.h b/src/tools/wasm2c-wrapper.h index d724f2580d9..0dd3c43819a 100644 --- a/src/tools/wasm2c-wrapper.h +++ b/src/tools/wasm2c-wrapper.h @@ -175,7 +175,7 @@ int main(int argc, char** argv) { // Emit the parameters (all 0s, like the other wrappers). bool first = true; - for (auto param : func->sig.params) { + for (auto& param : func->sig.params) { WASM_UNUSED(param); if (!first) { ret += ", "; diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 0e982acb62a..0d43ad7af9c 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -486,12 +486,12 @@ AsmConstWalker::AsmConst& AsmConstWalker::createAsmConst(uint32_t id, } Signature AsmConstWalker::asmConstSig(Signature baseSig) { - std::vector params(baseSig.params.begin(), baseSig.params.end()); - assert(params.size() >= 1); + assert(baseSig.params.size() >= 1); // Omit the signature of the "code" parameter, taken as a string, as the // first argument - params.erase(params.begin()); - return Signature(Type(params), baseSig.results); + return Signature( + Type(std::vector(baseSig.params.begin() + 1, baseSig.params.end())), + baseSig.results); } Name AsmConstWalker::nameForImportWithSig(Signature sig, Proxying proxy) { diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index 5e6647a9878..15ec266b392 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -641,7 +641,7 @@ SExpressionWasmBuilder::parseTypeUse(Element& s, // If only (type) is specified, populate `namedParams` if (!paramsOrResultsExist) { size_t index = 0; - for (auto param : functionSignature.params) { + for (auto& param : functionSignature.params) { namedParams.emplace_back(Name::fromInt(index++), param); } } From 0edaa68f2fb955f9c1d42c644abf394006d319e6 Mon Sep 17 00:00:00 2001 From: Daniel Wirtz Date: Wed, 19 Aug 2020 22:34:58 +0200 Subject: [PATCH 6/8] Update src/wasm-type.h Co-authored-by: Thomas Lively <7121787+tlively@users.noreply.github.com> --- src/wasm-type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index ea2f4a1328d..ee1ac831c26 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -177,7 +177,7 @@ class Type { std::string toString() const; - Type& operator[](size_t i) { + const Type& operator[](size_t i) const { if (isMulti()) { return (*(std::vector*)getID())[i]; } From eb2a485ada2a9e1c933c20992439363459dd7737 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 19 Aug 2020 23:59:36 +0200 Subject: [PATCH 7/8] move size logic to Type::end() and implement Type::size() on top --- src/wasm-type.h | 34 ++++++++++++++++++++-------------- src/wasm/wasm-type.cpp | 20 -------------------- 2 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index ee1ac831c26..256a94b1d53 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -67,9 +67,6 @@ class Type { Type(std::initializer_list types); explicit Type(const std::vector& types); - // Accessors - size_t size() const; - // Predicates // Compound Concrete // Type Basic │ Single│ @@ -177,14 +174,6 @@ class Type { std::string toString() const; - const Type& operator[](size_t i) const { - if (isMulti()) { - return (*(std::vector*)getID())[i]; - } - assert(id != Type::none && i == 0 && "Index out of bounds"); - return *this; - } - struct Iterator : std::iterator { const Type* parent; @@ -209,14 +198,31 @@ class Type { const value_type& operator*() const { if (parent->isMulti()) { return (*(std::vector*)parent->getID())[index]; + } else { + // see TODO in Type::end() + assert(index == 0 && parent->id != Type::none && "Index out of bounds"); + return *parent; } - assert(index == 0 && *parent != Type::none && "Index out of bounds"); - return *parent; } }; Iterator begin() const { return Iterator(this, 0); } - Iterator end() const { return Iterator(this, size()); } + Iterator end() const { + if (isMulti()) { + return Iterator(this, (*(std::vector*)getID()).size()); + } else { + // TODO: unreachable expands to {unreachable} currently. change to {}? + return Iterator(this, size_t(id != Type::none)); + } + } + size_t size() const { return end() - begin(); } + const Type& operator[](size_t i) const { + if (isMulti()) { + return (*(std::vector*)getID())[i]; + } else { + return *begin(); + } + } }; // Wrapper type for formatting types as "(param i32 i64 f32)" diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 19ac6e06bf5..6e887d74251 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -56,19 +56,6 @@ namespace { std::mutex mutex; -std::array, Type::_last_value_type + 1> basicTypes = { - {{}, - {Type::unreachable}, - {Type::i32}, - {Type::i64}, - {Type::f32}, - {Type::f64}, - {Type::v128}, - {Type::funcref}, - {Type::externref}, - {Type::nullref}, - {Type::exnref}}}; - // Track unique_ptrs for constructed types to avoid leaks std::vector>> constructedTypes; @@ -123,13 +110,6 @@ Type::Type(std::initializer_list types) { init(types); } Type::Type(const std::vector& types) { init(types); } -size_t Type::size() const { - if (isMulti()) { - return (*(std::vector*)getID()).size(); - } - return size_t(getID() != Type::none); -} - bool Type::operator<(const Type& other) const { return std::lexicographical_compare((*this).begin(), (*this).end(), From 7cad96c210ab077e9047877b7e028b78c34efa84 Mon Sep 17 00:00:00 2001 From: dcode Date: Thu, 20 Aug 2020 00:43:39 +0200 Subject: [PATCH 8/8] reference #3062 in comment --- src/wasm-type.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index 256a94b1d53..29b251eebe8 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -211,7 +211,8 @@ class Type { if (isMulti()) { return Iterator(this, (*(std::vector*)getID()).size()); } else { - // TODO: unreachable expands to {unreachable} currently. change to {}? + // TODO: unreachable is special and expands to {unreachable} currently. + // see also: https://github.com/WebAssembly/binaryen/issues/3062 return Iterator(this, size_t(id != Type::none)); } }