Repository: bytecodealliance/wasmtime
Pull request #8248 (2024-03-27 15:18:55)

Remove the `winch-tools` crate and CLI
With all Winch tests moved to `tests/disas` in #8243 plus the support of `wasmtime compile -C compiler=winch` this tool should in theory be supplanted nowadays with other alternatives. This commit removes the executable and the `winch-filetests` support.

<!--
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
-->


alexcrichton: I'll note that I'm not wed to this at all, and @saulecabrera or @elliottt or others find this tool useful I'm happy to close this. This can mostly be seen as asking the question of "with today's integration of Winch, is this executable still needed?"
github-actions[bot]: #### Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "winch"

Thus the following users have been cc'd because of the following labels:

* saulecabrera: winch

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

[Learn more.](https://github.com/bytecodealliance/subscribe-to-label-action)
</details>

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/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/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");
-    }
-}