Skip to content

Commit

Permalink
Fast validation (#1204)
Browse files Browse the repository at this point in the history 
This makes wasm validation parallel (the function part). This makes loading+validating tanks (a 12MB wasm file) 2.3x faster on a 4-core machine (from 3.5 to 1.5 seconds). It's a big speedup because most of loading+validating was actually validating.

It's also noticeable during compilation, since we validate by default at the end. 8% faster on -O2 and 23% on -O0. So actually fairly significant on -O0 builds.

As a bonus, this PR also moves the code from being 99% in the header to be 1% in the header.
  • Loading branch information
@kripken
kripken authored Oct 2, 2017
1 parent a9f91b9 commit 1f8d8a5
Show file tree
Hide file tree
Showing 4 changed files with 482 additions and 378 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ SET(wasm_as_SOURCES
)
ADD_EXECUTABLE(wasm-as
${wasm_as_SOURCES})
TARGET_LINK_LIBRARIES(wasm-as passes wasm asmjs ast cfg support)
TARGET_LINK_LIBRARIES(wasm-as wasm asmjs passes ast cfg support)
SET_PROPERTY(TARGET wasm-as PROPERTY CXX_STANDARD 11)
SET_PROPERTY(TARGET wasm-as PROPERTY CXX_STANDARD_REQUIRED ON)
INSTALL(TARGETS wasm-as DESTINATION ${CMAKE_INSTALL_BINDIR})
Expand Down
4 changes: 1 addition & 3 deletions src/tools/wasm-reduce.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,7 @@ struct Reducer : public WalkerPass<PostWalker<Reducer, UnifiedExpressionVisitor<
}
for (auto& func : functions) {
curr->removeFunction(func.name);
WasmValidator validator;
validator.quiet = true;
if (validator.validate(*curr) && writeAndTestReduction()) {
if (WasmValidator().validate(*curr, false, true, true /* override quiet */) && writeAndTestReduction()) {
std::cerr << "| removed function " << func.name << '\n';
noteReduction();
} else {
Expand Down
186 changes: 4 additions & 182 deletions src/wasm-validator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
// about function B not existing yet, but we would care
// if e.g. inside function A an i32.add receives an i64).
//
// * quiet: Whether to log errors verbosely.
//

#ifndef wasm_wasm_validator_h
#define wasm_wasm_validator_h
Expand All @@ -46,188 +48,8 @@

namespace wasm {

// Print anything that can be streamed to an ostream
template <typename T,
typename std::enable_if<
!std::is_base_of<Expression, typename std::remove_pointer<T>::type>::value
>::type* = nullptr>
inline std::ostream& printModuleComponent(T curr, std::ostream& stream) {
stream << curr << std::endl;
return stream;
}
// Extra overload for Expressions, to print type info too
inline std::ostream& printModuleComponent(Expression* curr, std::ostream& stream) {
WasmPrinter::printExpression(curr, stream, false, true) << std::endl;
return stream;
}

struct WasmValidator : public PostWalker<WasmValidator> {
bool valid = true;

// what to validate, see comment up top
bool validateWeb = false;
bool validateGlobally = true;

bool quiet = false; // whether to log errors verbosely

struct BreakInfo {
WasmType type;
Index arity;
BreakInfo() {}
BreakInfo(WasmType type, Index arity) : type(type), arity(arity) {}
};

std::map<Name, Expression*> breakTargets;
std::map<Expression*, BreakInfo> breakInfos;

WasmType returnType = unreachable; // type used in returns

std::set<Name> labelNames; // Binaryen IR requires that label names must be unique - IR generators must ensure that

std::unordered_set<Expression*> seenExpressions; // expressions must not appear twice

void noteLabelName(Name name);

public:
// TODO: If we want the validator to be part of libwasm rather than libpasses, then
// Using PassRunner::getPassDebug causes a circular dependence. We should fix that,
// perhaps by moving some of the pass infrastructure into libsupport.
bool validate(Module& module, bool validateWeb_ = false, bool validateGlobally_ = true) {
validateWeb = validateWeb_;
validateGlobally = validateGlobally_;
// wasm logic validation
walkModule(&module);
// validate additional internal IR details when in pass-debug mode
if (PassRunner::getPassDebug()) {
validateBinaryenIR(module);
}
// print if an error occurred
if (!valid && !quiet) {
WasmPrinter::printModule(&module, std::cerr);
}
return valid;
}

// visitors

static void visitPreBlock(WasmValidator* self, Expression** currp) {
auto* curr = (*currp)->cast<Block>();
if (curr->name.is()) self->breakTargets[curr->name] = curr;
}

void visitBlock(Block *curr);

static void visitPreLoop(WasmValidator* self, Expression** currp) {
auto* curr = (*currp)->cast<Loop>();
if (curr->name.is()) self->breakTargets[curr->name] = curr;
}

void visitLoop(Loop *curr);
void visitIf(If *curr);

// override scan to add a pre and a post check task to all nodes
static void scan(WasmValidator* self, Expression** currp) {
PostWalker<WasmValidator>::scan(self, currp);

auto* curr = *currp;
if (curr->is<Block>()) self->pushTask(visitPreBlock, currp);
if (curr->is<Loop>()) self->pushTask(visitPreLoop, currp);
}

void noteBreak(Name name, Expression* value, Expression* curr);
void visitBreak(Break *curr);
void visitSwitch(Switch *curr);
void visitCall(Call *curr);
void visitCallImport(CallImport *curr);
void visitCallIndirect(CallIndirect *curr);
void visitGetLocal(GetLocal* curr);
void visitSetLocal(SetLocal *curr);
void visitLoad(Load *curr);
void visitStore(Store *curr);
void visitAtomicRMW(AtomicRMW *curr);
void visitAtomicCmpxchg(AtomicCmpxchg *curr);
void visitAtomicWait(AtomicWait *curr);
void visitAtomicWake(AtomicWake *curr);
void visitBinary(Binary *curr);
void visitUnary(Unary *curr);
void visitSelect(Select* curr);
void visitDrop(Drop* curr);
void visitReturn(Return* curr);
void visitHost(Host* curr);
void visitImport(Import* curr);
void visitExport(Export* curr);
void visitGlobal(Global* curr);
void visitFunction(Function *curr);

void visitMemory(Memory *curr);
void visitTable(Table* curr);
void visitModule(Module *curr);

void doWalkFunction(Function* func) {
PostWalker<WasmValidator>::doWalkFunction(func);
}

// helpers
private:
template <typename T, typename S>
std::ostream& fail(S text, T curr);
std::ostream& printFailureHeader();

template<typename T>
bool shouldBeTrue(bool result, T curr, const char* text) {
if (!result) {
fail("unexpected false: " + std::string(text), curr);
return false;
}
return result;
}
template<typename T>
bool shouldBeFalse(bool result, T curr, const char* text) {
if (result) {
fail("unexpected true: " + std::string(text), curr);
return false;
}
return result;
}

template<typename T, typename S>
bool shouldBeEqual(S left, S right, T curr, const char* text) {
if (left != right) {
std::ostringstream ss;
ss << left << " != " << right << ": " << text;
fail(ss.str(), curr);
return false;
}
return true;
}

template<typename T, typename S>
bool shouldBeEqualOrFirstIsUnreachable(S left, S right, T curr, const char* text) {
if (left != unreachable && left != right) {
std::ostringstream ss;
ss << left << " != " << right << ": " << text;
fail(ss.str(), curr);
return false;
}
return true;
}

template<typename T, typename S>
bool shouldBeUnequal(S left, S right, T curr, const char* text) {
if (left == right) {
std::ostringstream ss;
ss << left << " == " << right << ": " << text;
fail(ss.str(), curr);
return false;
}
return true;
}

void shouldBeIntOrUnreachable(WasmType ty, Expression* curr, const char* text);
void validateAlignment(size_t align, WasmType type, Index bytes, bool isAtomic,
Expression* curr);
void validateMemBytes(uint8_t bytes, WasmType type, Expression* curr);
void validateBinaryenIR(Module& wasm);
struct WasmValidator {
bool validate(Module& module, bool validateWeb = false, bool validateGlobally = true, bool quiet = false);
};

} // namespace wasm
Expand Down
Loading

0 comments on commit 1f8d8a5

Please sign in to comment.