diff --git a/src/ir/metadata.cpp b/src/ir/metadata.cpp index f69927ecc10..61c5da9bc06 100644 --- a/src/ir/metadata.cpp +++ b/src/ir/metadata.cpp @@ -130,7 +130,7 @@ bool equal(Function* a, Function* b) { bList.list[i], a->codeAnnotations, b->codeAnnotations, - Function::CodeAnnotation())) { + CodeAnnotation())) { return false; } } diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 82083ca82ed..a077b4e2918 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1297,39 +1297,63 @@ struct ParseImplicitTypeDefsCtx : TypeParserCtx { }; struct AnnotationParserCtx { - // Return the inline hint for a call instruction, if there is one. - std::optional - getInlineHint(const std::vector& annotations) { - // Find and apply (the last) inline hint. - const Annotation* hint = nullptr; + // Parse annotations into IR. + CodeAnnotation parseAnnotations(const std::vector& annotations) { + CodeAnnotation ret; + + // Find the hints. For hints with content we must find the last one, which + // overrides the others. + const Annotation* branchHint = nullptr; + const Annotation* inlineHint = nullptr; for (auto& a : annotations) { - if (a.kind == Annotations::InlineHint) { - hint = &a; + if (a.kind == Annotations::BranchHint) { + branchHint = &a; + } else if (a.kind == Annotations::InlineHint) { + inlineHint = &a; } } - if (!hint) { - return std::nullopt; - } - - Lexer lexer(hint->contents); - if (lexer.empty()) { - std::cerr << "warning: empty InlineHint\n"; - return std::nullopt; - } - auto str = lexer.takeString(); - if (!str || str->size() != 1) { - std::cerr << "warning: invalid InlineHint string\n"; - return std::nullopt; + // Apply the last branch hint, if valid. + if (branchHint) { + Lexer lexer(branchHint->contents); + if (lexer.empty()) { + std::cerr << "warning: empty BranchHint\n"; + } else { + auto str = lexer.takeString(); + if (!str || str->size() != 1) { + std::cerr << "warning: invalid BranchHint string\n"; + } else { + auto value = (*str)[0]; + if (value != 0 && value != 1) { + std::cerr << "warning: invalid BranchHint value\n"; + } else { + ret.branchLikely = bool(value); + } + } + } } - uint8_t value = (*str)[0]; - if (value > 127) { - std::cerr << "warning: invalid InlineHint value\n"; - return std::nullopt; + // Apply the last inline hint, if valid. + if (inlineHint) { + Lexer lexer(inlineHint->contents); + if (lexer.empty()) { + std::cerr << "warning: empty InlineHint\n"; + } else { + auto str = lexer.takeString(); + if (!str || str->size() != 1) { + std::cerr << "warning: invalid InlineHint string\n"; + } else { + uint8_t value = (*str)[0]; + if (value > 127) { + std::cerr << "warning: invalid InlineHint value\n"; + } else { + ret.inline_ = value; + } + } + } } - return value; + return ret; } }; @@ -2000,10 +2024,10 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { if (!type.isSignature()) { return in.err(pos, "expected function type"); } - auto likely = getBranchHint(annotations); - return withLoc( - pos, - irBuilder.makeIf(label ? *label : Name{}, type.getSignature(), likely)); + return withLoc(pos, + irBuilder.makeIf(label ? *label : Name{}, + type.getSignature(), + parseAnnotations(annotations))); } Result<> visitElse() { return withLoc(irBuilder.visitElse()); } @@ -2422,8 +2446,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { const std::vector& annotations, Name func, bool isReturn) { - auto inline_ = getInlineHint(annotations); - return withLoc(pos, irBuilder.makeCall(func, isReturn, inline_)); + return withLoc( + pos, irBuilder.makeCall(func, isReturn, parseAnnotations(annotations))); } Result<> makeCallIndirect(Index pos, @@ -2433,52 +2457,18 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { bool isReturn) { auto t = getTable(pos, table); CHECK_ERR(t); - auto inline_ = getInlineHint(annotations); return withLoc(pos, - irBuilder.makeCallIndirect(*t, type, isReturn, inline_)); - } - - // Return the branch hint for a branching instruction, if there is one. - std::optional - getBranchHint(const std::vector& annotations) { - // Find and apply (the last) branch hint. - const Annotation* hint = nullptr; - for (auto& a : annotations) { - if (a.kind == Annotations::BranchHint) { - hint = &a; - } - } - if (!hint) { - return std::nullopt; - } - - Lexer lexer(hint->contents); - if (lexer.empty()) { - std::cerr << "warning: empty BranchHint\n"; - return std::nullopt; - } - - auto str = lexer.takeString(); - if (!str || str->size() != 1) { - std::cerr << "warning: invalid BranchHint string\n"; - return std::nullopt; - } - - auto value = (*str)[0]; - if (value != 0 && value != 1) { - std::cerr << "warning: invalid BranchHint value\n"; - return std::nullopt; - } - - return bool(value); + irBuilder.makeCallIndirect( + *t, type, isReturn, parseAnnotations(annotations))); } Result<> makeBreak(Index pos, const std::vector& annotations, Index label, bool isConditional) { - auto likely = getBranchHint(annotations); - return withLoc(pos, irBuilder.makeBreak(label, isConditional, likely)); + return withLoc( + pos, + irBuilder.makeBreak(label, isConditional, parseAnnotations(annotations))); } Result<> makeSwitch(Index pos, @@ -2617,8 +2607,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { const std::vector& annotations, HeapType type, bool isReturn) { - auto inline_ = getInlineHint(annotations); - return withLoc(pos, irBuilder.makeCallRef(type, isReturn, inline_)); + return withLoc( + pos, + irBuilder.makeCallRef(type, isReturn, parseAnnotations(annotations))); } Result<> makeRefI31(Index pos, @@ -2658,8 +2649,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { BrOnOp op, Type in = Type::none, Type out = Type::none) { - auto likely = getBranchHint(annotations); - return withLoc(pos, irBuilder.makeBrOn(label, op, in, out, likely)); + return withLoc( + pos, + irBuilder.makeBrOn(label, op, in, out, parseAnnotations(annotations))); } Result<> makeStructNew(Index pos, diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 5a1f618da3d..d9175f5d3c3 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1719,17 +1719,19 @@ class WasmBinaryReader { void readDylink(size_t payloadLen); void readDylink0(size_t payloadLen); - // We read branch hints *after* the code section, even though they appear + // We read code annotations *after* the code section, even though they appear // earlier. That is simpler for us as we note expression locations as we scan - // code, and then just need to match them up. To do this, we note the branch - // hint position and size in the first pass, and handle it later. - size_t branchHintsPos = 0; - size_t branchHintsLen = 0; - void readBranchHints(size_t payloadLen); + // code, and then just need to match them up. To do this, we note the + // positions of annotation sections in the first pass, and handle them later. + struct AnnotationSectionInfo { + // The start position of the section. We will rewind to there to read it. + size_t pos; + // A lambda that will read the section, from that position. + std::function read; + }; + std::vector deferredAnnotationSections; - // Like branch hints, we note where the section is to read it later. - size_t inlineHintsPos = 0; - size_t inlineHintsLen = 0; + void readBranchHints(size_t payloadLen); void readInlineHints(size_t payloadLen); std::tuple diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 5f06ddd9b39..11f43b46df6 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -129,20 +129,19 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeNop(); Result<> makeBlock(Name label, Signature sig); Result<> - makeIf(Name label, Signature sig, std::optional likely = std::nullopt); + makeIf(Name label, Signature sig, const CodeAnnotation& annotations = {}); Result<> makeLoop(Name label, Signature sig); Result<> makeBreak(Index label, bool isConditional, - std::optional likely = std::nullopt); + const CodeAnnotation& annotations = {}); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. - Result<> makeCall(Name func, - bool isReturn, - std::optional inline_ = std::nullopt); + Result<> + makeCall(Name func, bool isReturn, const CodeAnnotation& annotations = {}); Result<> makeCallIndirect(Name table, HeapType type, bool isReturn, - std::optional inline_ = std::nullopt); + const CodeAnnotation& annotations = {}); Result<> makeLocalGet(Index local); Result<> makeLocalSet(Index local); Result<> makeLocalTee(Index local); @@ -226,7 +225,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeI31Get(bool signed_); Result<> makeCallRef(HeapType type, bool isReturn, - std::optional inline_ = std::nullopt); + const CodeAnnotation& annotations = {}); Result<> makeRefTest(Type type); Result<> makeRefCast(Type type, bool isDesc); Result<> makeRefGetDesc(HeapType type); @@ -234,7 +233,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { BrOnOp op, Type in = Type::none, Type out = Type::none, - std::optional likely = std::nullopt); + const CodeAnnotation& annotations = {}); Result<> makeStructNew(HeapType type, bool isDesc); Result<> makeStructNewDefault(HeapType type, bool isDesc); Result<> @@ -735,11 +734,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Expression* fixExtraOutput(ScopeCtx& scope, Name label, Expression* expr); void fixLoopWithInput(Loop* loop, Type inputType, Index scratch); - // Add a branch hint, if |likely| is present. - void addBranchHint(Expression* expr, std::optional likely); - - // Add an inlining hint, if |inline_| is present. - void addInlineHint(Expression* expr, std::optional inline_); + void applyAnnotations(Expression* expr, const CodeAnnotation& annotation); void dump(); }; diff --git a/src/wasm.h b/src/wasm.h index 373f935b918..3d016f80385 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2232,6 +2232,21 @@ struct BinaryLocations { // Forward declaration for FuncEffectsMap. class EffectAnalyzer; +// Code annotations for VMs. +struct CodeAnnotation { + // Branch Hinting proposal: Whether the branch is likely, or unlikely. + std::optional branchLikely; + + // Compilation Hints proposal. + static const uint8_t NeverInline = 0; + static const uint8_t AlwaysInline = 127; + std::optional inline_; + + bool operator==(const CodeAnnotation& other) const { + return branchLikely == other.branchLikely && inline_ == other.inline_; + } +}; + class Function : public Importable { public: // A non-nullable reference to a function type. Exact for defined functions. @@ -2285,25 +2300,10 @@ class Function : public Importable { delimiterLocations; BinaryLocations::FunctionLocations funcLocation; - // Code annotations for VMs. As with debug info, we do not store these on + // Function-level annotations are implemented with a key of nullptr, matching + // the 0 byte offset in the spec. As with debug info, we do not store these on // Expressions as we assume most instances are unannotated, and do not want to // add constant memory overhead. - struct CodeAnnotation { - // Branch Hinting proposal: Whether the branch is likely, or unlikely. - std::optional branchLikely; - - // Compilation Hints proposal. - static const uint8_t NeverInline = 0; - static const uint8_t AlwaysInline = 127; - std::optional inline_; - - bool operator==(const CodeAnnotation& other) const { - return branchLikely == other.branchLikely && inline_ == other.inline_; - } - }; - - // Function-level annotations are implemented with a key of nullptr, matching - // the 0 byte offset in the spec. std::unordered_map codeAnnotations; // The effects for this function, if they have been computed. We use a shared diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 1aa002f4e03..230ab8af0c2 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1634,7 +1634,7 @@ std::optional WasmBinaryWriter::writeExpressionHints( Expression* expr; // The offset we will write in the custom section. BinaryLocation offset; - Function::CodeAnnotation* hint; + CodeAnnotation* hint; }; struct FuncHints { @@ -1726,11 +1726,8 @@ std::optional WasmBinaryWriter::writeExpressionHints( std::optional WasmBinaryWriter::getBranchHintsBuffer() { return writeExpressionHints( Annotations::BranchHint, - [](const Function::CodeAnnotation& annotation) { - return annotation.branchLikely; - }, - [](const Function::CodeAnnotation& annotation, - BufferWithRandomAccess& buffer) { + [](const CodeAnnotation& annotation) { return annotation.branchLikely; }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -1745,11 +1742,8 @@ std::optional WasmBinaryWriter::getBranchHintsBuffer() { std::optional WasmBinaryWriter::getInlineHintsBuffer() { return writeExpressionHints( Annotations::InlineHint, - [](const Function::CodeAnnotation& annotation) { - return annotation.inline_; - }, - [](const Function::CodeAnnotation& annotation, - BufferWithRandomAccess& buffer) { + [](const CodeAnnotation& annotation) { return annotation.inline_; }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -2158,14 +2152,11 @@ void WasmBinaryReader::read() { } } - // Go back and parse things we deferred. - if (branchHintsPos) { - pos = branchHintsPos; - readBranchHints(branchHintsLen); - } - if (inlineHintsPos) { - pos = inlineHintsPos; - readInlineHints(inlineHintsLen); + // Go back and parse annotations we deferred. + for (auto& [annotationPos, read] : deferredAnnotationSections) { + // Rewind to the right position, and read. + pos = annotationPos; + read(); } validateBinary(); @@ -2189,12 +2180,12 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName.equals(BinaryConsts::CustomSections::Dylink0)) { readDylink0(payloadLen); } else if (sectionName == Annotations::BranchHint) { - // Only note the position and length, we read this later. - branchHintsPos = pos; - branchHintsLen = payloadLen; + // Deferred. + deferredAnnotationSections.push_back(AnnotationSectionInfo{ + pos, [this, payloadLen]() { this->readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { - inlineHintsPos = pos; - inlineHintsLen = payloadLen; + deferredAnnotationSections.push_back(AnnotationSectionInfo{ + pos, [this, payloadLen]() { this->readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { @@ -5456,39 +5447,37 @@ void WasmBinaryReader::readExpressionHints(Name sectionName, } void WasmBinaryReader::readBranchHints(size_t payloadLen) { - readExpressionHints(Annotations::BranchHint, - payloadLen, - [&](Function::CodeAnnotation& annotation) { - auto size = getU32LEB(); - if (size != 1) { - throwError("bad BranchHint size"); - } + readExpressionHints( + Annotations::BranchHint, payloadLen, [&](CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 1) { + throwError("bad BranchHint size"); + } - auto likely = getU32LEB(); - if (likely != 0 && likely != 1) { - throwError("bad BranchHint value"); - } + auto likely = getU32LEB(); + if (likely != 0 && likely != 1) { + throwError("bad BranchHint value"); + } - annotation.branchLikely = likely; - }); + annotation.branchLikely = likely; + }); } void WasmBinaryReader::readInlineHints(size_t payloadLen) { - readExpressionHints(Annotations::InlineHint, - payloadLen, - [&](Function::CodeAnnotation& annotation) { - auto size = getU32LEB(); - if (size != 1) { - throwError("bad InlineHint size"); - } - - uint8_t inline_ = getInt8(); - if (inline_ > 127) { - throwError("bad InlineHint value"); - } - - annotation.inline_ = inline_; - }); + readExpressionHints( + Annotations::InlineHint, payloadLen, [&](CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 1) { + throwError("bad InlineHint size"); + } + + uint8_t inline_ = getInt8(); + if (inline_ > 127) { + throwError("bad InlineHint value"); + } + + annotation.inline_ = inline_; + }); } std::tuple diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 0144c8d1ea6..96a0f639d51 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1313,11 +1313,12 @@ Result<> IRBuilder::makeBlock(Name label, Signature sig) { return visitBlockStart(block, sig.params); } -Result<> -IRBuilder::makeIf(Name label, Signature sig, std::optional likely) { +Result<> IRBuilder::makeIf(Name label, + Signature sig, + const CodeAnnotation& annotations) { auto* iff = wasm.allocator.alloc(); iff->type = sig.results; - addBranchHint(iff, likely); + applyAnnotations(iff, annotations); return visitIfStart(iff, label, sig.params); } @@ -1330,7 +1331,7 @@ Result<> IRBuilder::makeLoop(Name label, Signature sig) { Result<> IRBuilder::makeBreak(Index label, bool isConditional, - std::optional likely) { + const CodeAnnotation& annotations) { auto name = getLabelName(label); CHECK_ERR(name); auto labelType = getLabelType(label); @@ -1342,7 +1343,7 @@ Result<> IRBuilder::makeBreak(Index label, curr.condition = isConditional ? &curr : nullptr; CHECK_ERR(ChildPopper{*this}.visitBreak(&curr, *labelType)); auto* br = builder.makeBreak(curr.name, curr.value, curr.condition); - addBranchHint(br, likely); + applyAnnotations(br, annotations); push(br); return Ok{}; @@ -1376,7 +1377,7 @@ Result<> IRBuilder::makeSwitch(const std::vector& labels, Result<> IRBuilder::makeCall(Name func, bool isReturn, - std::optional inline_) { + const CodeAnnotation& annotations) { auto sig = wasm.getFunction(func)->getSig(); Call curr(wasm.allocator); curr.target = func; @@ -1385,14 +1386,14 @@ Result<> IRBuilder::makeCall(Name func, auto* call = builder.makeCall(curr.target, curr.operands, sig.results, isReturn); push(call); - addInlineHint(call, inline_); + applyAnnotations(call, annotations); return Ok{}; } Result<> IRBuilder::makeCallIndirect(Name table, HeapType type, bool isReturn, - std::optional inline_) { + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_indirect"}; } @@ -1403,7 +1404,7 @@ Result<> IRBuilder::makeCallIndirect(Name table, auto* call = builder.makeCallIndirect(table, curr.target, curr.operands, type, isReturn); push(call); - addInlineHint(call, inline_); + applyAnnotations(call, annotations); return Ok{}; } @@ -1917,7 +1918,7 @@ Result<> IRBuilder::makeI31Get(bool signed_) { Result<> IRBuilder::makeCallRef(HeapType type, bool isReturn, - std::optional inline_) { + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_ref"}; } @@ -1932,7 +1933,7 @@ Result<> IRBuilder::makeCallRef(HeapType type, auto* call = builder.makeCallRef(curr.target, curr.operands, sig.results, isReturn); push(call); - addInlineHint(call, inline_); + applyAnnotations(call, annotations); return Ok{}; } @@ -1980,8 +1981,11 @@ Result<> IRBuilder::makeRefGetDesc(HeapType type) { return Ok{}; } -Result<> IRBuilder::makeBrOn( - Index label, BrOnOp op, Type in, Type out, std::optional likely) { +Result<> IRBuilder::makeBrOn(Index label, + BrOnOp op, + Type in, + Type out, + const CodeAnnotation& annotations) { std::optional descriptor; if (op == BrOnCastDesc || op == BrOnCastDescFail) { assert(out.isRef()); @@ -2062,7 +2066,7 @@ Result<> IRBuilder::makeBrOn( CHECK_ERR(name); auto* br = builder.makeBrOn(op, *name, curr.ref, out, curr.desc); - addBranchHint(br, likely); + applyAnnotations(br, annotations); push(br); return Ok{}; } @@ -2086,7 +2090,7 @@ Result<> IRBuilder::makeBrOn( // Perform the branch. CHECK_ERR(visitBrOn(&curr)); auto* br = builder.makeBrOn(op, extraLabel, curr.ref, out, curr.desc); - addBranchHint(br, likely); + applyAnnotations(br, annotations); push(br); // If the branch wasn't taken, we need to leave the extra values on the @@ -2642,20 +2646,18 @@ Result<> IRBuilder::makeStackSwitch(HeapType ct, Name tag) { return Ok{}; } -void IRBuilder::addBranchHint(Expression* expr, std::optional likely) { - if (likely) { +void IRBuilder::applyAnnotations(Expression* expr, + const CodeAnnotation& annotation) { + if (annotation.branchLikely) { // Branches are only possible inside functions. assert(func); - func->codeAnnotations[expr].branchLikely = likely; + func->codeAnnotations[expr].branchLikely = annotation.branchLikely; } -} -void IRBuilder::addInlineHint(Expression* expr, - std::optional inline_) { - if (inline_) { - // Branches are only possible inside functions. + if (annotation.inline_) { + // Only possible inside functions. assert(func); - func->codeAnnotations[expr].inline_ = inline_; + func->codeAnnotations[expr].inline_ = annotation.inline_; } }