[WebAssembly] Handle block and polymorphic stack in AsmTypeCheck #110770
+343
−79
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes the type checker handle blocks with input parameters and return types, branches, and polymorphic stacks correctly. We maintain the stack of "block info", which contains its input parameter type, return type, and whether it is a loop or not. And this is used when checking the validity of the value stack at the `end` marker and all branches targeting the block. `StackType` now supports a new variant `Polymorphic`, which indicates the stack is in the polymorphic state. `Polymorphic`s are not popped even when `popType` is executed; they are only popped when the current block ends. When popping from the value stack, we ensure we don't pop more than we are allowed to at the given block level and print appropriate error messages instead. Also after a block ends, the value stack is guaranteed to have the right types based on the block return type. For example, ```wast block i32 unreachable end_block ;; You can expect to have an i32 on the stack here ``` This also adds handling for `br_if`. Previously only `br`s were checked. `checkEnd` and `checkBr` were removed and their contents have been inlined to the main `typeCheck` function, because they are called only from a single callsite. This also fixes two existing bugs in AsmParser, which were required to make the tests passing. This modifies several existing invalid tests, those that passed (incorrectly) before but do not pass with the new type checker anymore. Fixes llvm#107524.
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-webassembly Author: Heejin Ahn (aheejin) ChangesThis makes the type checker handle blocks with input parameters and return types, branches, and polymorphic stacks correctly. We maintain the stack of "block info", which contains its input parameter type, return type, and whether it is a loop or not. And this is used when checking the validity of the value stack at the
When popping from the value stack, we ensure we don't pop more than we are allowed to at the given block level and print appropriate error messages instead. Also after a block ends, the value stack is guaranteed to have the right types based on the block return type. For example, block i32
unreachable
end_block
;; You can expect to have an i32 on the stack hereThis also adds handling for
This also fixes two existing bugs in AsmParser, which were required to make the tests passing. This modifies several existing invalid tests, those that passed (incorrectly) before but do not pass with the new type checker anymore. Fixes #107524. Patch is 22.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110770.diff 7 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 95db5500b0e1b1..5ea6c6bc0758b9 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -498,7 +498,9 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
void addBlockTypeOperand(OperandVector &Operands, SMLoc NameLoc,
WebAssembly::BlockType BT) {
- if (BT != WebAssembly::BlockType::Void) {
+ if (BT == WebAssembly::BlockType::Void) {
+ TC.setLastSig(wasm::WasmSignature{});
+ } else {
wasm::WasmSignature Sig({static_cast<wasm::ValType>(BT)}, {});
TC.setLastSig(Sig);
NestingStack.back().Sig = Sig;
@@ -1002,7 +1004,8 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
auto *Signature = Ctx.createWasmSignature();
if (parseSignature(Signature))
return ParseStatus::Failure;
- TC.funcDecl(*Signature);
+ if (CurrentState == FunctionStart)
+ TC.funcDecl(*Signature);
WasmSym->setSignature(Signature);
WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
TOut.emitFunctionType(WasmSym);
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
index 6c71460201537c..0ac67619691a31 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
@@ -50,8 +50,7 @@ WebAssemblyAsmTypeCheck::WebAssemblyAsmTypeCheck(MCAsmParser &Parser,
void WebAssemblyAsmTypeCheck::funcDecl(const wasm::WasmSignature &Sig) {
LocalTypes.assign(Sig.Params.begin(), Sig.Params.end());
- ReturnTypes.assign(Sig.Returns.begin(), Sig.Returns.end());
- BrStack.emplace_back(Sig.Returns.begin(), Sig.Returns.end());
+ BlockInfoStack.push_back({Sig, 0, false});
}
void WebAssemblyAsmTypeCheck::localDecl(
@@ -64,14 +63,15 @@ void WebAssemblyAsmTypeCheck::dumpTypeStack(Twine Msg) {
}
bool WebAssemblyAsmTypeCheck::typeError(SMLoc ErrorLoc, const Twine &Msg) {
- // If we're currently in unreachable code, we suppress errors completely.
- if (Unreachable)
- return false;
dumpTypeStack("current stack: ");
return Parser.Error(ErrorLoc, Msg);
}
bool WebAssemblyAsmTypeCheck::match(StackType TypeA, StackType TypeB) {
+ // These should have been filtered out in checkTypes()
+ assert(!std::get_if<Polymorphic>(&TypeA) &&
+ !std::get_if<Polymorphic>(&TypeB));
+
if (TypeA == TypeB)
return false;
if (std::get_if<Any>(&TypeA) || std::get_if<Any>(&TypeB))
@@ -90,6 +90,10 @@ std::string WebAssemblyAsmTypeCheck::getTypesString(ArrayRef<StackType> Types,
size_t StartPos) {
SmallVector<std::string, 4> TypeStrs;
for (auto I = Types.size(); I > StartPos; I--) {
+ if (std::get_if<Polymorphic>(&Types[I - 1])) {
+ TypeStrs.push_back("...");
+ break;
+ }
if (std::get_if<Any>(&Types[I - 1]))
TypeStrs.push_back("any");
else if (std::get_if<Ref>(&Types[I - 1]))
@@ -131,29 +135,48 @@ bool WebAssemblyAsmTypeCheck::checkTypes(SMLoc ErrorLoc,
bool ExactMatch) {
auto StackI = Stack.size();
auto TypeI = Types.size();
+ assert(!BlockInfoStack.empty());
+ auto BlockStackStartPos = BlockInfoStack.back().StackStartPos;
bool Error = false;
+ bool PolymorphicStack = false;
// Compare elements one by one from the stack top
- for (; StackI > 0 && TypeI > 0; StackI--, TypeI--) {
+ for (;StackI > BlockStackStartPos && TypeI > 0; StackI--, TypeI--) {
+ // If the stack is polymorphic, we assume all types in 'Types' have been
+ // compared and matched
+ if (std::get_if<Polymorphic>(&Stack[StackI - 1])) {
+ TypeI = 0;
+ break;
+ }
if (match(Stack[StackI - 1], Types[TypeI - 1])) {
Error = true;
break;
}
}
+
+ // If the stack top is polymorphic, the stack is in the polymorphic state.
+ if (StackI > BlockStackStartPos &&
+ std::get_if<Polymorphic>(&Stack[StackI - 1]))
+ PolymorphicStack = true;
+
// Even if no match failure has happened in the loop above, if not all
// elements of Types has been matched, that means we don't have enough
// elements on the stack.
//
// Also, if not all elements of the Stack has been matched and when
- // 'ExactMatch' is true, that means we have superfluous elements remaining on
- // the stack (e.g. at the end of a function).
- if (TypeI > 0 || (ExactMatch && StackI > 0))
+ // 'ExactMatch' is true and the current stack is not polymorphic, that means
+ // we have superfluous elements remaining on the stack (e.g. at the end of a
+ // function).
+ if (TypeI > 0 ||
+ (ExactMatch && !PolymorphicStack && StackI > BlockStackStartPos))
Error = true;
if (!Error)
return false;
- auto StackStartPos =
- ExactMatch ? 0 : std::max(0, (int)Stack.size() - (int)Types.size());
+ auto StackStartPos = ExactMatch
+ ? BlockStackStartPos
+ : std::max((int)BlockStackStartPos,
+ (int)Stack.size() - (int)Types.size());
return typeError(ErrorLoc, "type mismatch, expected " +
getTypesString(Types, 0) + " but got " +
getTypesString(Stack, StackStartPos));
@@ -169,9 +192,13 @@ bool WebAssemblyAsmTypeCheck::popTypes(SMLoc ErrorLoc,
ArrayRef<StackType> Types,
bool ExactMatch) {
bool Error = checkTypes(ErrorLoc, Types, ExactMatch);
- auto NumPops = std::min(Stack.size(), Types.size());
- for (size_t I = 0, E = NumPops; I != E; I++)
+ auto NumPops = std::min(Stack.size() - BlockInfoStack.back().StackStartPos,
+ Types.size());
+ for (size_t I = 0, E = NumPops; I != E; I++) {
+ if (std::get_if<Polymorphic>(&Stack.back()))
+ break;
Stack.pop_back();
+ }
return Error;
}
@@ -201,25 +228,6 @@ bool WebAssemblyAsmTypeCheck::getLocal(SMLoc ErrorLoc, const MCOperand &LocalOp,
return false;
}
-bool WebAssemblyAsmTypeCheck::checkBr(SMLoc ErrorLoc, size_t Level) {
- if (Level >= BrStack.size())
- return typeError(ErrorLoc,
- StringRef("br: invalid depth ") + std::to_string(Level));
- const SmallVector<wasm::ValType, 4> &Expected =
- BrStack[BrStack.size() - Level - 1];
- return checkTypes(ErrorLoc, Expected);
- return false;
-}
-
-bool WebAssemblyAsmTypeCheck::checkEnd(SMLoc ErrorLoc, bool PopVals) {
- if (!PopVals)
- BrStack.pop_back();
-
- if (PopVals)
- return popTypes(ErrorLoc, LastSig.Returns);
- return checkTypes(ErrorLoc, LastSig.Returns);
-}
-
bool WebAssemblyAsmTypeCheck::checkSig(SMLoc ErrorLoc,
const wasm::WasmSignature &Sig) {
bool Error = popTypes(ErrorLoc, Sig.Params);
@@ -308,10 +316,11 @@ bool WebAssemblyAsmTypeCheck::getSignature(SMLoc ErrorLoc,
return false;
}
-bool WebAssemblyAsmTypeCheck::endOfFunction(SMLoc ErrorLoc, bool ExactMatch) {
- bool Error = popTypes(ErrorLoc, ReturnTypes, ExactMatch);
- Unreachable = true;
- return Error;
+bool WebAssemblyAsmTypeCheck::endOfFunction(SMLoc ErrorLoc,
+ bool ExactMatch) {
+ assert(!BlockInfoStack.empty());
+ const auto &FuncInfo = BlockInfoStack[0];
+ return checkTypes(ErrorLoc, FuncInfo.Sig.Returns, ExactMatch);
}
bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
@@ -453,51 +462,90 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
}
if (Name == "try" || Name == "block" || Name == "loop" || Name == "if") {
- if (Name == "loop")
- BrStack.emplace_back(LastSig.Params.begin(), LastSig.Params.end());
- else
- BrStack.emplace_back(LastSig.Returns.begin(), LastSig.Returns.end());
- if (Name == "if" && popType(ErrorLoc, wasm::ValType::I32))
- return true;
- return false;
+ bool Error = Name == "if" && popType(ErrorLoc, wasm::ValType::I32);
+ // Pop block input parameters and check their types are correct
+ Error |= popTypes(ErrorLoc, LastSig.Params);
+ // Push a new block info
+ BlockInfoStack.push_back({LastSig, Stack.size(), Name == "loop"});
+ // Push back block input parameters
+ pushTypes(LastSig.Params);
+ return Error;
}
if (Name == "end_block" || Name == "end_loop" || Name == "end_if" ||
Name == "else" || Name == "end_try" || Name == "catch" ||
Name == "catch_all" || Name == "delegate") {
- bool Error = checkEnd(ErrorLoc, Name == "else" || Name == "catch" ||
- Name == "catch_all");
- Unreachable = false;
- if (Name == "catch") {
+ assert(!BlockInfoStack.empty());
+ // Check if the types on the stack match with the block return type
+ const auto &LastBlockInfo = BlockInfoStack.back();
+ bool Error = checkTypes(ErrorLoc, LastBlockInfo.Sig.Returns, true);
+ // Pop all types added to the stack for the current block level
+ Stack.truncate(LastBlockInfo.StackStartPos);
+ if (Name == "else") {
+ // 'else' expects the block input parameters to be on the stack, in the
+ // same way we entered 'if'
+ pushTypes(LastBlockInfo.Sig.Params);
+ } else if (Name == "catch") {
+ // 'catch' instruction pushes values whose types are specified in the
+ // tag's 'params' part
const wasm::WasmSignature *Sig = nullptr;
if (!getSignature(Operands[1]->getStartLoc(), Inst.getOperand(0),
wasm::WASM_SYMBOL_TYPE_TAG, Sig))
- // catch instruction pushes values whose types are specified in the
- // tag's "params" part
pushTypes(Sig->Params);
else
Error = true;
+ } else if (Name == "catch_all") {
+ // 'catch_all' does not push anything onto the stack
+ } else {
+ // For normal end markers, push block return value types onto the stack
+ // and pop the block info
+ pushTypes(LastBlockInfo.Sig.Returns);
+ BlockInfoStack.pop_back();
}
return Error;
}
- if (Name == "br") {
+ if (Name == "br" || Name == "br_if") {
+ bool Error = false;
+ if (Name == "br_if")
+ Error |= popType(ErrorLoc, wasm::ValType::I32); // cond
const MCOperand &Operand = Inst.getOperand(0);
- if (!Operand.isImm())
- return true;
- return checkBr(ErrorLoc, static_cast<size_t>(Operand.getImm()));
+ if (Operand.isImm()) {
+ unsigned Level = Operand.getImm();
+ if (Level < BlockInfoStack.size()) {
+ const auto &DestBlockInfo =
+ BlockInfoStack[BlockInfoStack.size() - Level - 1];
+ if (DestBlockInfo.IsLoop)
+ Error |= checkTypes(ErrorLoc, DestBlockInfo.Sig.Params, false);
+ else
+ Error |= checkTypes(ErrorLoc, DestBlockInfo.Sig.Returns, false);
+ } else {
+ Error = typeError(ErrorLoc, StringRef("br: invalid depth ") +
+ std::to_string(Level));
+ }
+ } else {
+ Error =
+ typeError(Operands[1]->getStartLoc(), "depth should be an integer");
+ }
+ if (Name == "br")
+ pushType(Polymorphic{});
+ return Error;
}
if (Name == "return") {
- return endOfFunction(ErrorLoc, false);
+ bool Error = endOfFunction(ErrorLoc, false);
+ pushType(Polymorphic{});
+ return Error;
}
if (Name == "call_indirect" || Name == "return_call_indirect") {
// Function value.
bool Error = popType(ErrorLoc, wasm::ValType::I32);
Error |= checkSig(ErrorLoc, LastSig);
- if (Name == "return_call_indirect" && endOfFunction(ErrorLoc, false))
- return true;
+ if (Name == "return_call_indirect") {
+ Error |= endOfFunction(ErrorLoc, false);
+ pushType(Polymorphic{});
+ }
return Error;
}
@@ -509,13 +557,15 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
Error |= checkSig(ErrorLoc, *Sig);
else
Error = true;
- if (Name == "return_call" && endOfFunction(ErrorLoc, false))
- return true;
+ if (Name == "return_call") {
+ Error |= endOfFunction(ErrorLoc, false);
+ pushType(Polymorphic{});
+ }
return Error;
}
if (Name == "unreachable") {
- Unreachable = true;
+ pushType(Polymorphic{});
return false;
}
@@ -526,11 +576,15 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
}
if (Name == "throw") {
+ bool Error = false;
const wasm::WasmSignature *Sig = nullptr;
if (!getSignature(Operands[1]->getStartLoc(), Inst.getOperand(0),
wasm::WASM_SYMBOL_TYPE_TAG, Sig))
- return checkSig(ErrorLoc, *Sig);
- return true;
+ Error |= checkSig(ErrorLoc, *Sig);
+ else
+ Error = true;
+ pushType(Polymorphic{});
+ return Error;
}
// The current instruction is a stack instruction which doesn't have
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h
index df063d749e3b40..596fb27bce94e6 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h
@@ -31,13 +31,17 @@ class WebAssemblyAsmTypeCheck final {
struct Ref : public std::monostate {};
struct Any : public std::monostate {};
- using StackType = std::variant<wasm::ValType, Ref, Any>;
+ struct Polymorphic : public std::monostate {};
+ using StackType = std::variant<wasm::ValType, Ref, Any, Polymorphic>;
SmallVector<StackType, 16> Stack;
- SmallVector<SmallVector<wasm::ValType, 4>, 8> BrStack;
+ struct BlockInfo {
+ wasm::WasmSignature Sig;
+ size_t StackStartPos;
+ bool IsLoop;
+ };
+ SmallVector<BlockInfo, 8> BlockInfoStack;
SmallVector<wasm::ValType, 16> LocalTypes;
- SmallVector<wasm::ValType, 4> ReturnTypes;
wasm::WasmSignature LastSig;
- bool Unreachable = false;
bool Is64;
// checkTypes checks 'Types' against the value stack. popTypes checks 'Types'
@@ -68,8 +72,6 @@ class WebAssemblyAsmTypeCheck final {
void dumpTypeStack(Twine Msg);
bool typeError(SMLoc ErrorLoc, const Twine &Msg);
bool getLocal(SMLoc ErrorLoc, const MCOperand &LocalOp, wasm::ValType &Type);
- bool checkEnd(SMLoc ErrorLoc, bool PopVals = false);
- bool checkBr(SMLoc ErrorLoc, size_t Level);
bool checkSig(SMLoc ErrorLoc, const wasm::WasmSignature &Sig);
bool getSymRef(SMLoc ErrorLoc, const MCOperand &SymOp,
const MCSymbolRefExpr *&SymRef);
@@ -91,10 +93,8 @@ class WebAssemblyAsmTypeCheck final {
void clear() {
Stack.clear();
- BrStack.clear();
+ BlockInfoStack.clear();
LocalTypes.clear();
- ReturnTypes.clear();
- Unreachable = false;
}
};
diff --git a/llvm/test/MC/WebAssembly/basic-assembly.s b/llvm/test/MC/WebAssembly/basic-assembly.s
index db7ccc9759beca..2028fcea6aeae4 100644
--- a/llvm/test/MC/WebAssembly/basic-assembly.s
+++ b/llvm/test/MC/WebAssembly/basic-assembly.s
@@ -82,11 +82,13 @@ test0:
i32.const 3
end_block # "switch" exit.
if # void
+ i32.const 0
if i32
+ i32.const 0
end_if
+ drop
else
end_if
- drop
block void
i32.const 2
return
@@ -222,11 +224,13 @@ empty_exnref_table:
# CHECK-NEXT: i32.const 3
# CHECK-NEXT: end_block # label2:
# CHECK-NEXT: if
+# CHECK-NEXT: i32.const 0
# CHECK-NEXT: if i32
+# CHECK-NEXT: i32.const 0
# CHECK-NEXT: end_if
+# CHECK-NEXT: drop
# CHECK-NEXT: else
# CHECK-NEXT: end_if
-# CHECK-NEXT: drop
# CHECK-NEXT: block
# CHECK-NEXT: i32.const 2
# CHECK-NEXT: return
diff --git a/llvm/test/MC/WebAssembly/block-assembly.s b/llvm/test/MC/WebAssembly/block-assembly.s
new file mode 100644
index 00000000000000..f97a77b99e1f80
--- /dev/null
+++ b/llvm/test/MC/WebAssembly/block-assembly.s
@@ -0,0 +1,182 @@
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+exception-handling < %s | FileCheck %s
+# Check that it converts to .o without errors, but don't check any output:
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+exception-handling -filetype=obj -o %t.o < %s
+
+ .tagtype __cpp_exception i32
+
+block_branch_test:
+ .functype block_branch_test () -> ()
+
+ # Block input paramter / return tests
+
+ i32.const 0
+ block (i32) -> (i32)
+ end_block
+ drop
+
+ i32.const 0
+ i64.const 0
+ block (i32, i64) -> (i32, f32)
+ drop
+ f32.const 0.0
+ end_block
+ drop
+ drop
+
+ i32.const 0
+ loop (i32) -> (f32)
+ drop
+ f32.const 0.0
+ end_loop
+ drop
+
+ i32.const 0
+ i32.const 0
+ if (i32) -> (i32)
+ else
+ i32.popcnt
+ end_if
+ drop
+
+ try i32
+ i32.const 0
+ catch __cpp_exception
+ i32.clz
+ catch_all
+ i32.const 5
+ end_try
+ drop
+
+ i32.const 0
+ block (i32) -> (i32)
+ block (i32) -> (f32)
+ drop
+ f32.const 0.0
+ end_block
+ drop
+ i32.const 0
+ end_block
+ drop
+
+ # Branch tests
+
+ block f32
+ f32.const 0.0
+ i32.const 0
+ br_if 0
+ f32.const 1.0
+ br 0
+ # After 'br', we can pop any values from the polymorphic stack
+ i32.add
+ i32.sub
+ i32.mul
+ drop
+ end_block
+ drop
+
+ block () -> (f32, f64)
+ f32.const 0.0
+ f64.const 0.0
+ i32.const 0
+ br_if 0
+ block (f32, f64) -> (f32, f64)
+ i32.const 1
+ br_if 0
+ end_block
+ end_block
+ drop
+ drop
+
+ # Withina loop, branches target the start of the loop
+ i32.const 0
+ loop (i32) -> ()
+ i32.const 1
+ br 0
+ end_loop
+
+ end_function
+
+# CHECK-LABEL: block_branch_test
+
+# CHECK: i32.const 0
+# CHECK-NEXT: block (i32) -> (i32)
+# CHECK-NEXT: end_block # label0:
+# CHECK-NEXT: drop
+
+# CHECK: i32.const 0
+# CHECK-NEXT: i64.const 0
+# CHECK-NEXT: block (i32, i64) -> (i32, f32)
+# CHECK-NEXT: drop
+# CHECK-NEXT: f32.const 0x0p0
+# CHECK-NEXT: end_block # label1:
+# CHECK-NEXT: drop
+# CHECK-NEXT: drop
+
+# CHECK: i32.const 0
+# CHECK-NEXT: loop (i32) -> (f32) # label2:
+# CHECK-NEXT: drop
+# CHECK-NEXT: f32.const 0x0p0
+# CHECK-NEXT: end_loop
+# CHECK-NEXT: drop
+
+# CHECK: i32.const 0
+# CHECK-NEXT: i32.const 0
+# CHECK-NEXT: if (i32) -> (i32)
+# CHECK-NEXT: else
+# CHECK-NEXT: i32.popcnt
+# CHECK-NEXT: end_if
+# CHECK-NEXT: drop
+
+# CHECK: try i32
+# CHECK-NEXT: i32.const 0
+# CHECK-NEXT: catch __cpp_exception # catch3:
+# CHECK-NEXT: i32.clz
+# CHECK-NEXT: catch_all
+# CHECK-NEXT: i32.const 5
+# CHECK-NEXT: end_try # label3:
+# CHECK-NEXT: drop
+
+# CHECK: i32.const 0
+# CHECK-NEXT: block (i32) -> (i32)
+# CHECK-NEXT: block (i32) -> (f32)
+# CHECK-NEXT: drop
+# CHECK-NEXT: f32.const 0x0p0
+# CHECK-NEXT: end_block # label5:
+# CHECK-NEXT: drop
+# CHECK-NEXT: i32.const 0
+# CHECK-NEXT: end_block # label4:
+# CHECK-NEXT: drop
+
+# CHECK: block f32
+# CHECK-NEXT: f32.const 0x0p0
+# CHECK-NEXT: i32.const 0
+# CHECK-NEXT: br_if 0 # 0: down to label6
+# CHECK-NEXT: f32.const 0x1p0
+# CHECK-NEXT: br 0 # 0: down to label6
+# CHECK-NEXT: i32.add
+# CHECK-NEXT: i32.sub
+# CHECK-NEXT: i32.mul
+# CHECK-NEXT: drop
+# CHECK-NEXT: end_block # label6:
+# CHECK-NEXT: drop
+
+# CHECK: block () -> (f32, f64)
+# CHECK-NEXT: f32.const 0x0p0
+# CHECK-NEXT: f64.const 0x0p0
+# CHECK-NEXT: i32.const 0
+# CHECK-NEXT: br_if 0 # 0: down to label7
+# CHECK-NEXT: block (f32, f64) -> (f32, f64)
+# CHECK-NEXT: i32.const 1
+# CHECK-NEXT: br_if ...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
aheejin
commented
Oct 2, 2024
Comment on lines
+501
to
+503
| if (BT == WebAssembly::BlockType::Void) { | ||
| TC.setLastSig(wasm::WasmSignature{}); | ||
| } else { |
There was a problem hiding this comment.
Before, when we have a non-void block we set the signature in the type checker, but not when we have a void block. So if we have a non-void block and then a void block, the type checker incorrectly thought void block's signature was the same as the previous (non-void ) one.
Comment on lines
+1007
to
+1008
| if (CurrentState == FunctionStart) | ||
| TC.funcDecl(*Signature); |
There was a problem hiding this comment.
.functype directive does not only happen at the start of a function but also just to declare a function type that does not have a definition associated with it. But we used to set the current function's signature whenever we parsed a .functype. This resulted (incorrectly) in calling funcDecl twice, and pushing to BlockInfoStack twice.
dschuff
approved these changes
Oct 2, 2024
Co-authored-by: Derek Schuff <dschuff@chromium.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes the type checker handle blocks with input parameters and return types, branches, and polymorphic stacks correctly.
We maintain the stack of "block info", which contains its input parameter type, return type, and whether it is a loop or not. And this is used when checking the validity of the value stack at the
endmarker and all branches targeting the block.StackTypenow supports a new variantPolymorphic, which indicates the stack is in the polymorphic state.Polymorphics are not popped even whenpopTypeis executed; they are only popped when the current block ends.When popping from the value stack, we ensure we don't pop more than we are allowed to at the given block level and print appropriate error messages instead. Also after a block ends, the value stack is guaranteed to have the right types based on the block return type. For example,
This also adds handling for
br_if. Previously onlybrs were checked.checkEndandcheckBrwere removed and their contents have been inlined to the maintypeCheckfunction, because they are called only from a single callsite.This also fixes two existing bugs in AsmParser, which were required to make the tests passing. I added Github comments about them inline.
This modifies several existing invalid tests, those that passed (incorrectly) before but do not pass with the new type checker anymore.
Fixes #107524.