[WebAssembly,llvm] Fix buildbot problems with llvm.wasm.ref.test.func#150116
Merged
Conversation
Member
|
@llvm/pr-subscribers-backend-webassembly Author: Hood Chatham (hoodmane) ChangesPR #147486 broke the sanitizer buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. cc @dschuff Full diff: https://github.com/llvm/llvm-project/pull/150116.diff 1 Files Affected:
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
index a7991319be8c7..dfc5a6d78756d 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
@@ -123,7 +123,7 @@ static SDValue getTagSymNode(int Tag, SelectionDAG *DAG) {
static APInt encodeFunctionSignature(SelectionDAG *DAG, SDLoc &DL,
SmallVector<MVT, 4> &Returns,
SmallVector<MVT, 4> &Params) {
- auto toWasmValType = [&DAG, &DL](MVT VT) {
+ auto toWasmValType = [](MVT VT) {
if (VT == MVT::i32) {
return wasm::ValType::I32;
}
|
PR llvm#147486 broke the sanitizer buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot.
0e1a04f to
08aa70a
Compare
dschuff
approved these changes
Jul 22, 2025
Member
|
I'll wait for the tests to run and then merge. |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
…llvm#150116) PR llvm#147486 broke the sanitizer and expensive-checks buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot. I also fixed the codegen in the wasm64 target: table.get requires an i32 but in wasm64 the function pointer is an i64. We need an additional `i32.wrap_i64` to convert it. I also added `-verify-machineinstrs` to the tests so that the test suite validates this fix. Finally, I noticed that llvm#150201 uses a feature of the intrinsic that is not covered by the tests, namely `ptr` arguments. So I added one additional test case to ensure that it works properly. cc @dschuff
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
PR #147486 broke the sanitizer and expensive-checks buildbot.
These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot.
I also fixed the codegen in the wasm64 target: table.get requires an i32 but in wasm64 the function pointer is an i64. We need an additional
i32.wrap_i64to convert it. I also added-verify-machineinstrsto the tests so that the test suite validates this fix.Finally, I noticed that #150201 uses a feature of the intrinsic that is not covered by the tests, namely
ptrarguments. So I added one additional test case to ensure that it works properly.cc @dschuff