Repository: bytecodealliance/wasmtime
Pull request #8249 (2024-03-27 15:32:07)

Un-ignore a custom test on riscv64
Use `(either ..)` to assert one of the two possible results.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 9d30010c7..fc4bd14c2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1348,12 +1348,6 @@ dependencies = [
  "stable_deref_trait",
 ]
 
-[[package]]
-name = "glob"
-version = "0.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
-
 [[package]]
 name = "h2"
 version = "0.4.2"
@@ -4134,56 +4128,6 @@ dependencies = [
  "wasmtime-environ",
 ]
 
-[[package]]
-name = "winch-filetests"
-version = "0.0.0"
-dependencies = [
- "anyhow",
- "capstone",
- "cranelift-codegen",
- "serde",
- "serde_derive",
- "similar",
- "target-lexicon",
- "toml",
- "wasmparser 0.202.0",
- "wasmtime-environ",
- "wat",
- "winch-codegen",
- "winch-test-macros",
-]
-
-[[package]]
-name = "winch-test-macros"
-version = "0.0.0"
-dependencies = [
- "glob",
- "proc-macro2",
- "quote",
- "syn 2.0.32",
-]
-
-[[package]]
-name = "winch-tools"
-version = "0.0.0"
-dependencies = [
- "anyhow",
- "capstone",
- "clap",
- "cranelift-codegen",
- "glob",
- "serde",
- "similar",
- "target-lexicon",
- "toml",
- "wasmparser 0.202.0",
- "wasmtime-environ",
- "wat",
- "winch-codegen",
- "winch-filetests",
- "winch-test-macros",
-]
-
 [[package]]
 name = "windows"
 version = "0.52.0"
diff --git a/Cargo.toml b/Cargo.toml
index 5090a7993..e18af8278 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -122,7 +122,6 @@ members = [
   "crates/test-programs",
   "crates/wasi-preview1-component-adapter",
   "crates/wasi-preview1-component-adapter/verify",
-  "crates/winch",
   "examples/fib-debug/wasm",
   "examples/wasi/wasm",
   "examples/tokio/wasm",
@@ -130,8 +129,7 @@ members = [
   "examples/min-platform",
   "examples/min-platform/embedding",
   "fuzz",
-  "winch",
-  "winch/codegen", "crates/slab",
+  "winch/codegen",
 ]
 exclude = [
   'docs/rust_wasi_markdown_parser',
@@ -211,8 +209,6 @@ cranelift-control = { path = "cranelift/control", version = "0.107.0" }
 cranelift = { path = "cranelift/umbrella", version = "0.107.0" }
 
 winch-codegen = { path = "winch/codegen", version = "=0.18.0" }
-winch-filetests = { path = "winch/filetests" }
-winch-test-macros = { path = "winch/test-macros" }
 
 wasi-preview1-component-adapter = { path = "crates/wasi-preview1-component-adapter" }
 byte-array-literals = { path = "crates/wasi-preview1-component-adapter/byte-array-literals" }
diff --git a/build.rs b/build.rs
index d0b42afe2..d116b18d8 100644
--- a/build.rs
+++ b/build.rs
@@ -281,16 +281,6 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
             testsuite == "function_references" || testsuite == "tail_call"
         }
 
-        "riscv64" => {
-            // This test case is disabled because it relies on `fvpromote_low`
-            // not flipping the sign bit of the input when it is a NaN. This
-            // is allowed by the spec. It's worth keeping the testcase as is
-            // since it is stressing a specific codegen bug in another arch.
-            //
-            // See #6961 for more details
-            testname == "issue_3327_bnot_lowering"
-        }
-
         _ => false,
     }
 }
diff --git a/winch/Cargo.toml b/winch/Cargo.toml
deleted file mode 100644
index b50224762..000000000
--- a/winch/Cargo.toml
+++ /dev/null
@@ -1,36 +0,0 @@
-[package]
-name = "winch-tools"
-version = "0.0.0"
-description = "Binaries for testing Winch"
-license = "Apache-2.0 WITH LLVM-exception"
-repository = "https://github.com/bytecodealliance/wasmtime"
-publish = false
-edition.workspace = true
-
-[lints]
-workspace = true
-
-[[bin]]
-name = "winch-tools"
-path = "src/main.rs"
-
-[dependencies]
-winch-codegen = { workspace = true }
-winch-filetests = { workspace = true }
-winch-test-macros = { workspace = true }
-wasmtime-environ = { workspace = true }
-target-lexicon = { workspace = true }
-anyhow = { workspace = true }
-wasmparser = { workspace = true }
-clap = { workspace = true, features = ['default'] }
-wat = { workspace = true }
-cranelift-codegen = { workspace = true }
-capstone = { workspace = true }
-similar = { workspace = true }
-toml = { workspace = true }
-serde = { workspace = true }
-glob = { workspace = true }
-
-[features]
-default = ["all-arch"]
-all-arch = ["winch-codegen/all-arch"]
diff --git a/winch/src/compile.rs b/winch/src/compile.rs
deleted file mode 100644
index 542d7a14c..000000000
--- a/winch/src/compile.rs
+++ /dev/null
@@ -1,87 +0,0 @@
-use anyhow::{Context, Result};
-use clap::Parser;
-use cranelift_codegen::settings;
-use std::{fs, path::PathBuf, str::FromStr};
-use target_lexicon::Triple;
-use wasmtime_environ::{
-    wasmparser::{Parser as WasmParser, Validator},
-    DefinedFuncIndex, FunctionBodyData, ModuleEnvironment, ModuleTranslation, ModuleTypesBuilder,
-    Tunables, TypeConvert, VMOffsets,
-};
-use winch_codegen::{lookup, BuiltinFunctions, TargetIsa};
-use winch_filetests::disasm::{disasm, OffsetStyle};
-
-#[derive(Parser, Debug)]
-pub struct Options {
-    /// The input file.
-    input: PathBuf,
-
-    /// The target architecture.
-    #[arg(long = "target")]
-    target: String,
-}
-
-pub fn run(opt: &Options) -> Result<()> {
-    let bytes = fs::read(&opt.input)
-        .with_context(|| format!("Failed to read input file {}", opt.input.display()))?;
-    let bytes = wat::parse_bytes(&bytes)?;
-    let triple = Triple::from_str(&opt.target)?;
-    let shared_flags = settings::Flags::new(settings::builder());
-    let isa_builder = lookup(triple)?;
-    let isa = isa_builder.finish(shared_flags)?;
-    let mut validator = Validator::new();
-    let parser = WasmParser::new(0);
-    let mut types = Default::default();
-    let tunables = Tunables::default_host();
-    let mut translation = ModuleEnvironment::new(&tunables, &mut validator, &mut types)
-        .translate(parser, &bytes)
-        .context("Failed to translate WebAssembly module")?;
-    let body_inputs = std::mem::take(&mut translation.function_body_inputs);
-
-    body_inputs
-        .into_iter()
-        .try_for_each(|func| compile(&isa, &translation, &types, func))?;
-
-    Ok(())
-}
-
-fn compile(
-    isa: &Box<dyn TargetIsa>,
-    translation: &ModuleTranslation,
-    module_types: &ModuleTypesBuilder,
-    f: (DefinedFuncIndex, FunctionBodyData<'_>),
-) -> Result<()> {
-    let index = translation.module.func_index(f.0);
-    let types = &translation.get_types();
-    let sig = types[types.core_function_at(index.as_u32())].unwrap_func();
-    let sig = DummyConvert.convert_func_type(sig);
-    let FunctionBodyData { body, validator } = f.1;
-    let vmoffsets = VMOffsets::new(isa.pointer_bytes(), &translation.module);
-    let mut builtins = BuiltinFunctions::new(&vmoffsets, isa.wasmtime_call_conv());
-    let mut validator = validator.into_validator(Default::default());
-    let buffer = isa
-        .compile_function(
-            &sig,
-            &body,
-            translation,
-            module_types,
-            &mut builtins,
-            &mut validator,
-        )
-        .expect("Couldn't compile function");
-
-    println!("Disassembly for function: {}", index.as_u32());
-    disasm(buffer.buffer.data(), isa, OffsetStyle::Full)?
-        .iter()
-        .for_each(|s| println!("{}", s));
-
-    Ok(())
-}
-
-struct DummyConvert;
-
-impl TypeConvert for DummyConvert {
-    fn lookup_heap_type(&self, _: wasmparser::UnpackedIndex) -> wasmtime_environ::WasmHeapType {
-        todo!()
-    }
-}
diff --git a/winch/src/main.rs b/winch/src/main.rs
deleted file mode 100644
index 9984b86d3..000000000
--- a/winch/src/main.rs
+++ /dev/null
@@ -1,21 +0,0 @@
-mod compile;
-mod filetests;
-
-use anyhow::Result;
-use clap::Parser;
-
-/// Winch compilation and testing tool.
-#[derive(Parser)]
-enum Commands {
-    /// Compile a Wasm module to the specified target architecture.
-    Compile(compile::Options),
-    /// Run the filetests.
-    Test(filetests::Options),
-}
-
-fn main() -> Result<()> {
-    match Commands::parse() {
-        Commands::Compile(c) => compile::run(&c),
-        Commands::Test(t) => filetests::run(&t),
-    }
-}


Test patch:
diff --git a/tests/misc_testsuite/simd/issue_3327_bnot_lowering.wast b/tests/misc_testsuite/simd/issue_3327_bnot_lowering.wast
index 4563afe89..93faaf6df 100644
--- a/tests/misc_testsuite/simd/issue_3327_bnot_lowering.wast
+++ b/tests/misc_testsuite/simd/issue_3327_bnot_lowering.wast
@@ -26,7 +26,10 @@
     v128.not
     i64x2.bitmask)
   (export "" (func 0)))
-(assert_return (invoke "") (i32.const 0))
+;; the f64x2.promote_low_f32x4 operation may or may not preserve the sign bit
+;; on the NaN in the first operation. This leads to one of two results depending
+;; on how platforms propagate NaN bits.
+(assert_return (invoke "") (either (i32.const 0) (i32.const 1)))
 
 ;; from #3327
 (module
diff --git a/winch/filetests/Cargo.toml b/winch/filetests/Cargo.toml
deleted file mode 100644
index 1de596fea..000000000
--- a/winch/filetests/Cargo.toml
+++ /dev/null
@@ -1,27 +0,0 @@
-[package]
-authors = ["The Winch Project Developers"]
-name = "winch-filetests"
-description = "Tests for the Winch compiler based on a set of known valid files"
-license = "Apache-2.0 WITH LLVM-exception"
-repository = "https://github.com/bytecodealliance/wasmtime"
-version = "0.0.0"
-publish = false
-edition.workspace = true
-
-[lints]
-workspace = true
-
-[dependencies]
-winch-test-macros = {workspace = true}
-target-lexicon = { workspace = true }
-winch-codegen = { workspace = true, features = ['all-arch'] }
-wasmtime-environ = { workspace = true }
-anyhow = { workspace = true }
-wat = { workspace = true }
-similar = { workspace = true }
-toml = { workspace = true }
-serde = { workspace = true }
-serde_derive = { workspace = true }
-cranelift-codegen = { workspace = true }
-capstone = { workspace = true }
-wasmparser = { workspace = true }
diff --git a/winch/filetests/build.rs b/winch/filetests/build.rs
deleted file mode 100644
index bd0f62b29..000000000
--- a/winch/filetests/build.rs
+++ /dev/null
@@ -1,4 +0,0 @@
-fn main() {
-    // Ensure that new files in the filetests directory cause a rebuild.
-    println!("cargo:rerun-if-changed=filetests");
-}
diff --git a/winch/filetests/src/disasm.rs b/winch/filetests/src/disasm.rs
deleted file mode 100644
index 64ddf45b0..000000000
--- a/winch/filetests/src/disasm.rs
+++ /dev/null
@@ -1,112 +0,0 @@
-//! Disassembly utilities.
-
-use anyhow::{bail, Result};
-use capstone::prelude::*;
-use std::fmt::Write;
-use target_lexicon::Architecture;
-use winch_codegen::TargetIsa;
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub enum OffsetStyle {
-    Minimal,
-    Full,
-}
-
-/// Disassemble and print a machine code buffer.
-pub fn disasm(
-    bytes: &[u8],
-    isa: &Box<dyn TargetIsa>,
-    offset_style: OffsetStyle,
-) -> Result<Vec<String>> {
-    let dis = disassembler_for(isa)?;
-    let insts = dis.disasm_all(bytes, 0x0).unwrap();
-
-    let mut prev_jump = false;
-    let mut write_offsets = offset_style == OffsetStyle::Full;
-
-    let disassembled_lines = insts
-        .iter()
-        .map(|i| {
-            use capstone::InsnGroupType::{CS_GRP_JUMP, CS_GRP_RET};
-
-            let detail = dis.insn_detail(&i).unwrap();
-
-            let is_jump = detail
-                .groups()
-                .iter()
-                .find(|g| g.0 as u32 == CS_GRP_JUMP)
-                .is_some();
-
-            let mut line = String::new();
-
-            if write_offsets || (prev_jump && !is_jump) {
-                write!(&mut line, "{:4x}:\t ", i.address()).unwrap();
-            } else {
-                write!(&mut line, "     \t ").unwrap();
-            }
-
-            let mut bytes_str = String::new();
-            let mut len = 0;
-            for b in i.bytes() {
-                write!(&mut bytes_str, "{:02x}", b).unwrap();
-                len += 1;
-            }
-            write!(&mut line, "{:21}\t", bytes_str).unwrap();
-            if len > 8 {
-                write!(&mut line, "\n\t\t\t\t").unwrap();
-            }
-
-            if let Some(s) = i.mnemonic() {
-                write!(&mut line, "{}\t", s).unwrap();
-            }
-
-            if let Some(s) = i.op_str() {
-                write!(&mut line, "{}", s).unwrap();
-            }
-
-            prev_jump = is_jump;
-
-            // Flip write_offsets to true once we've seen a `ret`, as instructions that follow the
-            // return are often related to trap tables.
-            write_offsets = write_offsets
-                || detail
-                    .groups()
-                    .iter()
-                    .find(|g| g.0 as u32 == CS_GRP_RET)
-                    .is_some();
-
-            line
-        })
-        .collect();
-
-    Ok(disassembled_lines)
-}
-
-fn disassembler_for(isa: &Box<dyn TargetIsa>) -> Result<Capstone> {
-    let disasm = match isa.triple().architecture {
-        Architecture::X86_64 => Capstone::new()
-            .x86()
-            .mode(arch::x86::ArchMode::Mode64)
-            .syntax(arch::x86::ArchSyntax::Att)
-            .detail(true)
-            .build()
-            .map_err(|e| anyhow::format_err!("{}", e))?,
-
-        Architecture::Aarch64 { .. } => {
-            let mut cs = Capstone::new()
-                .arm64()
-                .mode(arch::arm64::ArchMode::Arm)
-                .detail(true)
-                .build()
-                .map_err(|e| anyhow::format_err!("{}", e))?;
-
-            cs.set_skipdata(true)
-                .map_err(|e| anyhow::format_err!("{}", e))?;
-            cs
-        }
-
-        _ => bail!("Unsupported ISA"),
-    };
-
-    Ok(disasm)
-}
diff --git a/winch/filetests/src/lib.rs b/winch/filetests/src/lib.rs
deleted file mode 100644
index d2a29dc17..000000000
--- a/winch/filetests/src/lib.rs
+++ /dev/null
@@ -1,1 +0,0 @@
-pub mod disasm;
diff --git a/winch/src/filetests.rs b/winch/src/filetests.rs
deleted file mode 100644
index 423f8d836..000000000
--- a/winch/src/filetests.rs
+++ /dev/null
@@ -1,25 +0,0 @@
-use std::process::Command;
-
-use anyhow::Result;
-use clap::Parser;
-
-#[derive(Parser, Debug)]
-pub struct Options {
-    /// Passes extra arguments to `cargo test --package winch-filetests`. For example, to run a single
-    /// test, use `-- --test-threads 1 --test single_test_name`.
-    #[arg(last = true, value_parser)]
-    cargo_test_args: Vec<String>,
-}
-
-pub fn run(opts: &Options) -> Result<()> {
-    Command::new("cargo")
-        .arg("test")
-        .arg("--package")
-        .arg("winch-filetests")
-        .arg("--")
-        .args(&opts.cargo_test_args)
-        .spawn()?
-        .wait()
-        .map(|_| ())
-        .map_err(|e| anyhow::anyhow!("Failed to run cargo test: {}", e))
-}
diff --git a/winch/test-macros/Cargo.toml b/winch/test-macros/Cargo.toml
deleted file mode 100644
index a1186787f..000000000
--- a/winch/test-macros/Cargo.toml
+++ /dev/null
@@ -1,23 +0,0 @@
-[package]
-authors = ["The Winch Project Developers"]
-name = "winch-test-macros"
-description = "Winch test macros"
-license = "Apache-2.0 WITH LLVM-exception"
-repository = "https://github.com/bytecodealliance/wasmtime"
-version = "0.0.0"
-publish = false
-edition.workspace = true
-
-[lints]
-workspace = true
-
-[lib]
-proc-macro = true
-test = false
-doctest = false
-
-[dependencies]
-quote = "1.0"
-syn = { workspace = true, features = ["full"]}
-proc-macro2 = "1.0"
-glob = { workspace = true }
diff --git a/winch/test-macros/src/lib.rs b/winch/test-macros/src/lib.rs
deleted file mode 100644
index df9f447d8..000000000
--- a/winch/test-macros/src/lib.rs
+++ /dev/null
@@ -1,80 +0,0 @@
-use std::path::Path;
-
-use glob::glob;
-use proc_macro::TokenStream;
-use quote::quote;
-use syn::ItemFn;
-
-fn get_test_name_for_root(root: &Path, path: &Path) -> String {
-    let test_name = path
-        .strip_prefix(root)
-        .unwrap()
-        .to_str()
-        .unwrap()
-        .replace("/", "_")
-        .replace("\\", "_")
-        .replace(".wat", "");
-
-    format!("winch_filetests_{}", test_name)
-}
-
-/// Generate a test case for every .wat file in the filetests directory.
-/// This should only be used from the filetests crate.
-#[proc_macro_attribute]
-pub fn generate_file_tests(_attr: TokenStream, input: TokenStream) -> TokenStream {
-    // Parse the input as a function.
-    let input = proc_macro2::TokenStream::from(input);
-
-    let fn_ast: ItemFn =
-        syn::parse(input.clone().into()).expect("Failed to parse tokens as function");
-
-    // Get the function's name and body.
-    let name = &fn_ast.sig.ident;
-
-    let filetests_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../filetests/filetests");
-
-    let test_file_entries = glob(format!("{}/**/*.wat", filetests_dir.to_str().unwrap()).as_str())
-        .expect("Failed to read glob pattern");
-
-    // Create a list of test cases by opening every .wat file in the directory.
-    let test_cases = test_file_entries.map(|entry| {
-        let path = entry.expect("Failed to read glob entry");
-
-        let full = path.to_str().expect("Path for file was empty");
-
-        let test_name = proc_macro2::Ident::new(
-            &get_test_name_for_root(&filetests_dir, &path),
-            proc_macro2::Span::call_site(),
-        );
-        quote! {
-            #[test]
-            fn #test_name() {
-                #name(#full);
-            }
-        }
-    });
-
-    // Assemble the output by combining the function and test cases.
-    let output = quote! {
-        #input
-
-        #(#test_cases)*
-    };
-
-    output.into()
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_get_test_name_for_root_unix() {
-        let root = Path::new("/home/user/Documents/winch/filetests/filetests");
-        let path = Path::new("/home/user/Documents/winch/filetests/filetests/simd/simple.wat");
-
-        let test_name = get_test_name_for_root(root, path);
-
-        assert_eq!(test_name, "winch_filetests_simd_simple");
-    }
-}