From 326837d7d4e1dd4de1f0cd922bc486a70cdf6fbc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 31 Aug 2023 19:02:29 +0200 Subject: [PATCH 1/8] Enhance `async` configuration of `bindgen!` macro (#6942) This commit takes a leaf out of `wiggle`'s book to enable bindings generation for async host functions where only some host functions are async instead of all of them. This enhances the `async` key with a few more options: async: { except_imports: ["foo"], only_imports: ["bar"], } This is beyond what `wiggle` supports where either an allow-list or deny-list can be specified (although only one can be specified). This can be useful if either the list of sync imports or the list of async imports is small. --- crates/component-macro/src/bindgen.rs | 56 ++++++++++++++-- crates/wasmtime/src/component/mod.rs | 16 +++++ crates/wit-bindgen/src/lib.rs | 70 +++++++++++++++----- tests/all/component_model/bindgen.rs | 94 +++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 21 deletions(-) diff --git a/crates/component-macro/src/bindgen.rs b/crates/component-macro/src/bindgen.rs index 3dfd7f846ea4..a87616e55d82 100644 --- a/crates/component-macro/src/bindgen.rs +++ b/crates/component-macro/src/bindgen.rs @@ -1,10 +1,11 @@ use proc_macro2::{Span, TokenStream}; use std::collections::HashMap; +use std::collections::HashSet; use std::path::{Path, PathBuf}; use syn::parse::{Error, Parse, ParseStream, Result}; use syn::punctuated::Punctuated; use syn::{braced, token, Ident, Token}; -use wasmtime_wit_bindgen::{Opts, Ownership, TrappableError}; +use wasmtime_wit_bindgen::{AsyncConfig, Opts, Ownership, TrappableError}; use wit_parser::{PackageId, Resolve, UnresolvedPackage, WorldId}; pub struct Config { @@ -15,7 +16,7 @@ pub struct Config { } pub fn expand(input: &Config) -> Result { - if !cfg!(feature = "async") && input.opts.async_ { + if !cfg!(feature = "async") && input.opts.async_.maybe_async() { return Err(Error::new( Span::call_site(), "cannot enable async bindings unless `async` crate feature is active", @@ -45,6 +46,7 @@ impl Parse for Config { let mut world = None; let mut inline = None; let mut path = None; + let mut async_configured = false; if input.peek(token::Brace) { let content; @@ -71,7 +73,13 @@ impl Parse for Config { inline = Some(s.value()); } Opt::Tracing(val) => opts.tracing = val, - Opt::Async(val) => opts.async_ = val, + Opt::Async(val, span) => { + if async_configured { + return Err(Error::new(span, "cannot specify second async config")); + } + async_configured = true; + opts.async_ = val; + } Opt::TrappableErrorType(val) => opts.trappable_error_type = val, Opt::Ownership(val) => opts.ownership = val, Opt::Interfaces(s) => { @@ -171,6 +179,8 @@ mod kw { syn::custom_keyword!(ownership); syn::custom_keyword!(interfaces); syn::custom_keyword!(with); + syn::custom_keyword!(except_imports); + syn::custom_keyword!(only_imports); } enum Opt { @@ -178,7 +188,7 @@ enum Opt { Path(syn::LitStr), Inline(syn::LitStr), Tracing(bool), - Async(bool), + Async(AsyncConfig, Span), TrappableErrorType(Vec), Ownership(Ownership), Interfaces(syn::LitStr), @@ -205,9 +215,43 @@ impl Parse for Opt { input.parse::()?; Ok(Opt::Tracing(input.parse::()?.value)) } else if l.peek(Token![async]) { - input.parse::()?; + let span = input.parse::()?.span; input.parse::()?; - Ok(Opt::Async(input.parse::()?.value)) + if input.peek(syn::LitBool) { + match input.parse::()?.value { + true => Ok(Opt::Async(AsyncConfig::All, span)), + false => Ok(Opt::Async(AsyncConfig::None, span)), + } + } else { + let contents; + syn::braced!(contents in input); + + let l = contents.lookahead1(); + let ctor: fn(HashSet) -> AsyncConfig = if l.peek(kw::except_imports) { + contents.parse::()?; + contents.parse::()?; + AsyncConfig::AllExceptImports + } else if l.peek(kw::only_imports) { + contents.parse::()?; + contents.parse::()?; + AsyncConfig::OnlyImports + } else { + return Err(l.error()); + }; + + let list; + syn::bracketed!(list in contents); + let fields: Punctuated = + list.parse_terminated(Parse::parse, Token![,])?; + + if contents.peek(Token![,]) { + contents.parse::()?; + } + Ok(Opt::Async( + ctor(fields.iter().map(|s| s.value()).collect()), + span, + )) + } } else if l.peek(kw::ownership) { input.parse::()?; input.parse::()?; diff --git a/crates/wasmtime/src/component/mod.rs b/crates/wasmtime/src/component/mod.rs index b8e79df1fd45..618f829b2540 100644 --- a/crates/wasmtime/src/component/mod.rs +++ b/crates/wasmtime/src/component/mod.rs @@ -281,6 +281,22 @@ pub(crate) use self::store::ComponentStoreData; /// // This option defaults to `false`. /// async: true, /// +/// // Alternative mode of async configuration where this still implies +/// // async instantiation happens, for example, but more control is +/// // provided over which imports are async and which aren't. +/// // +/// // Note that in this mode all exports are still async. +/// async: { +/// // All imports are async except for functions with these names +/// except_imports: ["foo", "bar"], +/// +/// // All imports are synchronous except for functions with these names +/// // +/// // Note that this key cannot be specified with `except_imports`, +/// // only one or the other is accepted. +/// only_imports: ["foo", "bar"], +/// }, +/// /// // This can be used to translate WIT return values of the form /// // `result` into `Result` in Rust. /// // The `RustErrorType` structure will have an automatically generated diff --git a/crates/wit-bindgen/src/lib.rs b/crates/wit-bindgen/src/lib.rs index 44431ba2e0f3..21ae6367b489 100644 --- a/crates/wit-bindgen/src/lib.rs +++ b/crates/wit-bindgen/src/lib.rs @@ -3,7 +3,7 @@ use crate::types::{TypeInfo, Types}; use anyhow::{anyhow, bail, Context}; use heck::*; use indexmap::IndexMap; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::Write as _; use std::io::{Read, Write}; use std::mem; @@ -94,7 +94,7 @@ pub struct Opts { pub tracing: bool, /// Whether or not to use async rust functions and traits. - pub async_: bool, + pub async_: AsyncConfig, /// A list of "trappable errors" which are used to replace the `E` in /// `result` found in WIT. @@ -123,6 +123,42 @@ pub struct TrappableError { pub rust_type_name: String, } +#[derive(Default, Debug, Clone)] +pub enum AsyncConfig { + /// No functions are `async`. + #[default] + None, + /// All generated functions should be `async`. + All, + /// These imported functions should not be async, but everything else is. + AllExceptImports(HashSet), + /// These functions are the only imports that are async, all other imports + /// are sync. + /// + /// Note that all exports are still async in this situation. + OnlyImports(HashSet), +} + +impl AsyncConfig { + pub fn is_import_async(&self, f: &str) -> bool { + match self { + AsyncConfig::None => false, + AsyncConfig::All => true, + AsyncConfig::AllExceptImports(set) => !set.contains(f), + AsyncConfig::OnlyImports(set) => set.contains(f), + } + } + + pub fn maybe_async(&self) -> bool { + match self { + AsyncConfig::None => false, + AsyncConfig::All | AsyncConfig::AllExceptImports(_) | AsyncConfig::OnlyImports(_) => { + true + } + } + } +} + impl Opts { pub fn generate(&self, resolve: &Resolve, world: WorldId) -> String { let mut r = Wasmtime::default(); @@ -412,7 +448,7 @@ impl Wasmtime { } self.src.push_str("}\n"); - let (async_, async__, send, await_) = if self.opts.async_ { + let (async_, async__, send, await_) = if self.opts.async_.maybe_async() { ("async", "_async", ":Send", ".await") } else { ("", "", "", "") @@ -577,7 +613,7 @@ impl Wasmtime { } let world_camel = to_rust_upper_camel_case(&resolve.worlds[world].name); - if self.opts.async_ { + if self.opts.async_.maybe_async() { uwriteln!(self.src, "#[wasmtime::component::__internal::async_trait]") } uwrite!(self.src, "pub trait {world_camel}Imports"); @@ -646,7 +682,7 @@ impl Wasmtime { self.src.push_str(&name); } - let maybe_send = if self.opts.async_ { + let maybe_send = if self.opts.async_.maybe_async() { " + Send, T: Send" } else { "" @@ -854,7 +890,7 @@ impl<'a> InterfaceGenerator<'a> { self.rustdoc(docs); uwriteln!(self.src, "pub enum {camel} {{}}"); - if self.gen.opts.async_ { + if self.gen.opts.async_.maybe_async() { uwriteln!(self.src, "#[wasmtime::component::__internal::async_trait]") } @@ -1375,7 +1411,7 @@ impl<'a> InterfaceGenerator<'a> { let iface = &self.resolve.interfaces[id]; let owner = TypeOwner::Interface(id); - if self.gen.opts.async_ { + if self.gen.opts.async_.maybe_async() { uwriteln!(self.src, "#[wasmtime::component::__internal::async_trait]") } // Generate the `pub trait` which represents the host functionality for @@ -1400,7 +1436,7 @@ impl<'a> InterfaceGenerator<'a> { } uwriteln!(self.src, "}}"); - let where_clause = if self.gen.opts.async_ { + let where_clause = if self.gen.opts.async_.maybe_async() { "T: Send, U: Host + Send".to_string() } else { "U: Host".to_string() @@ -1443,7 +1479,7 @@ impl<'a> InterfaceGenerator<'a> { uwrite!( self.src, "{linker}.{}(\"{}\", ", - if self.gen.opts.async_ { + if self.gen.opts.async_.is_import_async(&func.name) { "func_wrap_async" } else { "func_wrap" @@ -1472,7 +1508,7 @@ impl<'a> InterfaceGenerator<'a> { self.src.push_str(", "); } self.src.push_str(") |"); - if self.gen.opts.async_ { + if self.gen.opts.async_.is_import_async(&func.name) { self.src.push_str(" Box::new(async move { \n"); } else { self.src.push_str(" { \n"); @@ -1541,7 +1577,7 @@ impl<'a> InterfaceGenerator<'a> { for (i, _) in func.params.iter().enumerate() { uwrite!(self.src, "arg{},", i); } - if self.gen.opts.async_ { + if self.gen.opts.async_.is_import_async(&func.name) { uwrite!(self.src, ").await;\n"); } else { uwrite!(self.src, ");\n"); @@ -1571,7 +1607,7 @@ impl<'a> InterfaceGenerator<'a> { uwrite!(self.src, "r\n"); } - if self.gen.opts.async_ { + if self.gen.opts.async_.is_import_async(&func.name) { // Need to close Box::new and async block self.src.push_str("})"); } else { @@ -1582,7 +1618,7 @@ impl<'a> InterfaceGenerator<'a> { fn generate_function_trait_sig(&mut self, func: &Function) { self.rustdoc(&func.docs); - if self.gen.opts.async_ { + if self.gen.opts.async_.is_import_async(&func.name) { self.push_str("async "); } self.push_str("fn "); @@ -1658,7 +1694,11 @@ impl<'a> InterfaceGenerator<'a> { ns: Option<&WorldKey>, func: &Function, ) { - let (async_, async__, await_) = if self.gen.opts.async_ { + // Exports must be async if anything could be async, it's just imports + // that get to be optionally async/sync. + let is_async = self.gen.opts.async_.maybe_async(); + + let (async_, async__, await_) = if is_async { ("async", "_async", ".await") } else { ("", "", "") @@ -1681,7 +1721,7 @@ impl<'a> InterfaceGenerator<'a> { self.src.push_str(") -> wasmtime::Result<"); self.print_result_ty(&func.results, TypeMode::Owned); - if self.gen.opts.async_ { + if is_async { self.src .push_str("> where ::Data: Send {\n"); } else { diff --git a/tests/all/component_model/bindgen.rs b/tests/all/component_model/bindgen.rs index dbe65a3ddc6f..f0160dc92203 100644 --- a/tests/all/component_model/bindgen.rs +++ b/tests/all/component_model/bindgen.rs @@ -316,3 +316,97 @@ mod resources_at_interface_level { Ok(()) } } + +mod async_config { + use super::*; + + wasmtime::component::bindgen!({ + inline: " + package foo:foo + + world t1 { + import x: func() + import y: func() + export z: func() + } + ", + async: true, + }); + + struct T; + + #[async_trait::async_trait] + impl T1Imports for T { + async fn x(&mut self) -> Result<()> { + Ok(()) + } + + async fn y(&mut self) -> Result<()> { + Ok(()) + } + } + + async fn _test_t1(t1: &T1, store: &mut Store<()>) { + let _ = t1.call_z(&mut *store).await; + } + + wasmtime::component::bindgen!({ + inline: " + package foo:foo + + world t2 { + import x: func() + import y: func() + export z: func() + } + ", + async: { + except_imports: ["x"], + }, + }); + + #[async_trait::async_trait] + impl T2Imports for T { + fn x(&mut self) -> Result<()> { + Ok(()) + } + + async fn y(&mut self) -> Result<()> { + Ok(()) + } + } + + async fn _test_t2(t2: &T2, store: &mut Store<()>) { + let _ = t2.call_z(&mut *store).await; + } + + wasmtime::component::bindgen!({ + inline: " + package foo:foo + + world t3 { + import x: func() + import y: func() + export z: func() + } + ", + async: { + only_imports: ["x"], + }, + }); + + #[async_trait::async_trait] + impl T3Imports for T { + async fn x(&mut self) -> Result<()> { + Ok(()) + } + + fn y(&mut self) -> Result<()> { + Ok(()) + } + } + + async fn _test_t3(t3: &T3, store: &mut Store<()>) { + let _ = t3.call_z(&mut *store).await; + } +} From 6a5bddf02d9f136bb0f998d26a91f97dce9860bc Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 31 Aug 2023 18:13:23 +0100 Subject: [PATCH 2/8] cranelift-interpreter: Fix SIMD shifts and rotates (#6939) * cranelift-interpreter: Fix SIMD `ishl`/`{s,u}`shr * fuzzgen: Enable a few more ops * cranelift: Fix tests for {u,s}shr * fuzzgen: Change pattern matching arms for shifts Co-Authored-By: Jamey Sharp --------- Co-authored-by: Jamey Sharp --- .../filetests/runtests/simd-ishl.clif | 1 + .../filetests/runtests/simd-sshr.clif | 3 +- .../filetests/runtests/simd-ushr.clif | 3 +- cranelift/fuzzgen/src/function_generator.rs | 137 ++++-------------- cranelift/interpreter/src/step.rs | 42 +++--- 5 files changed, 57 insertions(+), 129 deletions(-) diff --git a/cranelift/filetests/filetests/runtests/simd-ishl.clif b/cranelift/filetests/filetests/runtests/simd-ishl.clif index 524bc2745ca5..648d8f9fe9b0 100644 --- a/cranelift/filetests/filetests/runtests/simd-ishl.clif +++ b/cranelift/filetests/filetests/runtests/simd-ishl.clif @@ -1,3 +1,4 @@ +test interpret test run target aarch64 target s390x diff --git a/cranelift/filetests/filetests/runtests/simd-sshr.clif b/cranelift/filetests/filetests/runtests/simd-sshr.clif index a5fc4782176e..2df402270917 100644 --- a/cranelift/filetests/filetests/runtests/simd-sshr.clif +++ b/cranelift/filetests/filetests/runtests/simd-sshr.clif @@ -1,3 +1,4 @@ +test interpret test run target aarch64 target s390x @@ -70,7 +71,7 @@ block0(v0: i8x16): v2 = sshr v0, v1 return v2 } -; run: %i8x16_shl_const([0x01 0x02 0x04 0x08 0x10 0x20 0x40 0x80 0 0 0 0 0 0 0 0]) == [0 0 0x01 0x02 0x04 0x08 0x10 0xe0 0 0 0 0 0 0 0 0] +; run: %i8x16_sshr_const([0x01 0x02 0x04 0x08 0x10 0x20 0x40 0x80 0 0 0 0 0 0 0 0]) == [0 0 0x01 0x02 0x04 0x08 0x10 0xe0 0 0 0 0 0 0 0 0] function %i16x8_sshr_const(i16x8) -> i16x8 { block0(v0: i16x8): diff --git a/cranelift/filetests/filetests/runtests/simd-ushr.clif b/cranelift/filetests/filetests/runtests/simd-ushr.clif index 5b188523e417..5d8cf88bef2f 100644 --- a/cranelift/filetests/filetests/runtests/simd-ushr.clif +++ b/cranelift/filetests/filetests/runtests/simd-ushr.clif @@ -1,3 +1,4 @@ +test interpret test run target aarch64 target s390x @@ -54,7 +55,7 @@ block0(v0: i8x16): v2 = ushr v0, v1 return v2 } -; run: %i8x16_shl_const([0x01 0x02 0x04 0x08 0x10 0x20 0x40 0x80 0 0 0 0 0 0 0 0]) == [0 0 0x01 0x02 0x04 0x08 0x10 0x20 0 0 0 0 0 0 0 0] +; run: %i8x16_ushr_const([0x01 0x02 0x04 0x08 0x10 0x20 0x40 0x80 0 0 0 0 0 0 0 0]) == [0 0 0x01 0x02 0x04 0x08 0x10 0x20 0 0 0 0 0 0 0 0] function %i16x8_ushr_const(i16x8) -> i16x8 { block0(v0: i16x8): diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 15e78443317e..216adce67662 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -525,6 +525,8 @@ fn valid_for_target(triple: &Triple, op: Opcode, args: &[Type], rets: &[Type]) - (Opcode::Cls, &[I32], &[I32]), (Opcode::Cls, &[I64], &[I64]), (Opcode::Cls, &[I128], &[I128]), + // TODO + (Opcode::Bitselect, &[_, _, _], &[F32 | F64]), // https://github.com/bytecodealliance/wasmtime/issues/4897 // https://github.com/bytecodealliance/wasmtime/issues/4899 ( @@ -589,6 +591,15 @@ fn valid_for_target(triple: &Triple, op: Opcode, args: &[Type], rets: &[Type]) - &[], &[I8X16 | I16X8 | I32X4 | I64X2 | F32X4 | F64X2] ), + // TODO + ( + Opcode::Sshr | Opcode::Ushr | Opcode::Ishl, + &[I8X16 | I16X8 | I32X4 | I64X2, I128] + ), + ( + Opcode::Rotr | Opcode::Rotl, + &[I8X16 | I16X8 | I32X4 | I64X2, _] + ), ) } @@ -647,6 +658,17 @@ fn valid_for_target(triple: &Triple, op: Opcode, args: &[Type], rets: &[Type]) - // https://github.com/bytecodealliance/wasmtime/issues/6104 (Opcode::Bitcast, &[I128], &[_]), (Opcode::Bitcast, &[_], &[I128]), + // TODO + ( + Opcode::Sshr | Opcode::Ushr | Opcode::Ishl, + &[I8X16 | I16X8 | I32X4 | I64X2, I128] + ), + ( + Opcode::Rotr | Opcode::Rotl, + &[I8X16 | I16X8 | I32X4 | I64X2, _] + ), + // TODO + (Opcode::Bitselect, &[_, _, _], &[F32 | F64]), ) } @@ -689,6 +711,8 @@ fn valid_for_target(triple: &Triple, op: Opcode, args: &[Type], rets: &[Type]) - // https://github.com/bytecodealliance/wasmtime/issues/6104 (Opcode::Bitcast, &[I128], &[_]), (Opcode::Bitcast, &[_], &[I128]), + // TODO + (Opcode::Bitselect, &[_, _, _], &[F32 | F64]), ) } @@ -737,8 +761,9 @@ fn valid_for_target(triple: &Triple, op: Opcode, args: &[Type], rets: &[Type]) - (Opcode::Bitcast, &[I128], &[_]), (Opcode::Bitcast, &[_], &[I128]), // TODO - (Opcode::SelectSpectreGuard, &[_, _, _], &[F32]), - (Opcode::SelectSpectreGuard, &[_, _, _], &[F64]), + (Opcode::SelectSpectreGuard, &[_, _, _], &[F32 | F64]), + // TODO + (Opcode::Bitselect, &[_, _, _], &[F32 | F64]), ) } @@ -896,10 +921,6 @@ static OPCODE_SIGNATURES: Lazy> = Lazy::new(|| { (Opcode::ScalarToVector), (Opcode::X86Pmaddubsw), (Opcode::X86Cvtt2dq), - (Opcode::Bitselect, &[F32, F32, F32], &[F32]), - (Opcode::Bitselect, &[F64, F64, F64], &[F64]), - (Opcode::Bitselect, &[F32X4, F32X4, F32X4], &[F32X4]), - (Opcode::Bitselect, &[F64X2, F64X2, F64X2], &[F64X2]), (Opcode::VanyTrue, &[F32X4], &[I8]), (Opcode::VanyTrue, &[F64X2], &[I8]), (Opcode::VhighBits, &[F32X4], &[I8]), @@ -952,10 +973,6 @@ static OPCODE_SIGNATURES: Lazy> = Lazy::new(|| { (Opcode::VhighBits, &[I64X2], &[I64X2]), (Opcode::VhighBits, &[F32X4], &[I64X2]), (Opcode::VhighBits, &[F64X2], &[I64X2]), - (Opcode::Ineg, &[I8X16], &[I8X16]), - (Opcode::Ineg, &[I16X8], &[I16X8]), - (Opcode::Ineg, &[I32X4], &[I32X4]), - (Opcode::Ineg, &[I64X2], &[I64X2]), (Opcode::Umulhi, &[I128, I128], &[I128]), (Opcode::Smulhi, &[I128, I128], &[I128]), // https://github.com/bytecodealliance/wasmtime/issues/6073 @@ -966,106 +983,6 @@ static OPCODE_SIGNATURES: Lazy> = Lazy::new(|| { (Opcode::Isplit, &[I64], &[I32, I32]), (Opcode::Isplit, &[I32], &[I16, I16]), (Opcode::Isplit, &[I16], &[I8, I8]), - (Opcode::Rotl, &[I8X16, I8], &[I8X16]), - (Opcode::Rotl, &[I8X16, I16], &[I8X16]), - (Opcode::Rotl, &[I8X16, I32], &[I8X16]), - (Opcode::Rotl, &[I8X16, I64], &[I8X16]), - (Opcode::Rotl, &[I8X16, I128], &[I8X16]), - (Opcode::Rotl, &[I16X8, I8], &[I16X8]), - (Opcode::Rotl, &[I16X8, I16], &[I16X8]), - (Opcode::Rotl, &[I16X8, I32], &[I16X8]), - (Opcode::Rotl, &[I16X8, I64], &[I16X8]), - (Opcode::Rotl, &[I16X8, I128], &[I16X8]), - (Opcode::Rotl, &[I32X4, I8], &[I32X4]), - (Opcode::Rotl, &[I32X4, I16], &[I32X4]), - (Opcode::Rotl, &[I32X4, I32], &[I32X4]), - (Opcode::Rotl, &[I32X4, I64], &[I32X4]), - (Opcode::Rotl, &[I32X4, I128], &[I32X4]), - (Opcode::Rotl, &[I64X2, I8], &[I64X2]), - (Opcode::Rotl, &[I64X2, I16], &[I64X2]), - (Opcode::Rotl, &[I64X2, I32], &[I64X2]), - (Opcode::Rotl, &[I64X2, I64], &[I64X2]), - (Opcode::Rotl, &[I64X2, I128], &[I64X2]), - (Opcode::Rotr, &[I8X16, I8], &[I8X16]), - (Opcode::Rotr, &[I8X16, I16], &[I8X16]), - (Opcode::Rotr, &[I8X16, I32], &[I8X16]), - (Opcode::Rotr, &[I8X16, I64], &[I8X16]), - (Opcode::Rotr, &[I8X16, I128], &[I8X16]), - (Opcode::Rotr, &[I16X8, I8], &[I16X8]), - (Opcode::Rotr, &[I16X8, I16], &[I16X8]), - (Opcode::Rotr, &[I16X8, I32], &[I16X8]), - (Opcode::Rotr, &[I16X8, I64], &[I16X8]), - (Opcode::Rotr, &[I16X8, I128], &[I16X8]), - (Opcode::Rotr, &[I32X4, I8], &[I32X4]), - (Opcode::Rotr, &[I32X4, I16], &[I32X4]), - (Opcode::Rotr, &[I32X4, I32], &[I32X4]), - (Opcode::Rotr, &[I32X4, I64], &[I32X4]), - (Opcode::Rotr, &[I32X4, I128], &[I32X4]), - (Opcode::Rotr, &[I64X2, I8], &[I64X2]), - (Opcode::Rotr, &[I64X2, I16], &[I64X2]), - (Opcode::Rotr, &[I64X2, I32], &[I64X2]), - (Opcode::Rotr, &[I64X2, I64], &[I64X2]), - (Opcode::Rotr, &[I64X2, I128], &[I64X2]), - (Opcode::Ishl, &[I8X16, I8], &[I8X16]), - (Opcode::Ishl, &[I8X16, I16], &[I8X16]), - (Opcode::Ishl, &[I8X16, I32], &[I8X16]), - (Opcode::Ishl, &[I8X16, I64], &[I8X16]), - (Opcode::Ishl, &[I8X16, I128], &[I8X16]), - (Opcode::Ishl, &[I16X8, I8], &[I16X8]), - (Opcode::Ishl, &[I16X8, I16], &[I16X8]), - (Opcode::Ishl, &[I16X8, I32], &[I16X8]), - (Opcode::Ishl, &[I16X8, I64], &[I16X8]), - (Opcode::Ishl, &[I16X8, I128], &[I16X8]), - (Opcode::Ishl, &[I32X4, I8], &[I32X4]), - (Opcode::Ishl, &[I32X4, I16], &[I32X4]), - (Opcode::Ishl, &[I32X4, I32], &[I32X4]), - (Opcode::Ishl, &[I32X4, I64], &[I32X4]), - (Opcode::Ishl, &[I32X4, I128], &[I32X4]), - (Opcode::Ishl, &[I64X2, I8], &[I64X2]), - (Opcode::Ishl, &[I64X2, I16], &[I64X2]), - (Opcode::Ishl, &[I64X2, I32], &[I64X2]), - (Opcode::Ishl, &[I64X2, I64], &[I64X2]), - (Opcode::Ishl, &[I64X2, I128], &[I64X2]), - (Opcode::Ushr, &[I8X16, I8], &[I8X16]), - (Opcode::Ushr, &[I8X16, I16], &[I8X16]), - (Opcode::Ushr, &[I8X16, I32], &[I8X16]), - (Opcode::Ushr, &[I8X16, I64], &[I8X16]), - (Opcode::Ushr, &[I8X16, I128], &[I8X16]), - (Opcode::Ushr, &[I16X8, I8], &[I16X8]), - (Opcode::Ushr, &[I16X8, I16], &[I16X8]), - (Opcode::Ushr, &[I16X8, I32], &[I16X8]), - (Opcode::Ushr, &[I16X8, I64], &[I16X8]), - (Opcode::Ushr, &[I16X8, I128], &[I16X8]), - (Opcode::Ushr, &[I32X4, I8], &[I32X4]), - (Opcode::Ushr, &[I32X4, I16], &[I32X4]), - (Opcode::Ushr, &[I32X4, I32], &[I32X4]), - (Opcode::Ushr, &[I32X4, I64], &[I32X4]), - (Opcode::Ushr, &[I32X4, I128], &[I32X4]), - (Opcode::Ushr, &[I64X2, I8], &[I64X2]), - (Opcode::Ushr, &[I64X2, I16], &[I64X2]), - (Opcode::Ushr, &[I64X2, I32], &[I64X2]), - (Opcode::Ushr, &[I64X2, I64], &[I64X2]), - (Opcode::Ushr, &[I64X2, I128], &[I64X2]), - (Opcode::Sshr, &[I8X16, I8], &[I8X16]), - (Opcode::Sshr, &[I8X16, I16], &[I8X16]), - (Opcode::Sshr, &[I8X16, I32], &[I8X16]), - (Opcode::Sshr, &[I8X16, I64], &[I8X16]), - (Opcode::Sshr, &[I8X16, I128], &[I8X16]), - (Opcode::Sshr, &[I16X8, I8], &[I16X8]), - (Opcode::Sshr, &[I16X8, I16], &[I16X8]), - (Opcode::Sshr, &[I16X8, I32], &[I16X8]), - (Opcode::Sshr, &[I16X8, I64], &[I16X8]), - (Opcode::Sshr, &[I16X8, I128], &[I16X8]), - (Opcode::Sshr, &[I32X4, I8], &[I32X4]), - (Opcode::Sshr, &[I32X4, I16], &[I32X4]), - (Opcode::Sshr, &[I32X4, I32], &[I32X4]), - (Opcode::Sshr, &[I32X4, I64], &[I32X4]), - (Opcode::Sshr, &[I32X4, I128], &[I32X4]), - (Opcode::Sshr, &[I64X2, I8], &[I64X2]), - (Opcode::Sshr, &[I64X2, I16], &[I64X2]), - (Opcode::Sshr, &[I64X2, I32], &[I64X2]), - (Opcode::Sshr, &[I64X2, I64], &[I64X2]), - (Opcode::Sshr, &[I64X2, I128], &[I64X2]), (Opcode::Fmin, &[F32X4, F32X4], &[F32X4]), (Opcode::Fmin, &[F64X2, F64X2], &[F64X2]), (Opcode::Fmax, &[F32X4, F32X4], &[F32X4]), diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index f13d2c918400..a8ae06f906ea 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -795,16 +795,16 @@ where Opcode::BandImm => binary(DataValueExt::and, arg(0), imm_as_ctrl_ty()?)?, Opcode::BorImm => binary(DataValueExt::or, arg(0), imm_as_ctrl_ty()?)?, Opcode::BxorImm => binary(DataValueExt::xor, arg(0), imm_as_ctrl_ty()?)?, - Opcode::Rotl => binary(DataValueExt::rotl, arg(0), arg(1))?, - Opcode::Rotr => binary(DataValueExt::rotr, arg(0), arg(1))?, - Opcode::RotlImm => binary(DataValueExt::rotl, arg(0), imm_as_ctrl_ty()?)?, - Opcode::RotrImm => binary(DataValueExt::rotr, arg(0), imm_as_ctrl_ty()?)?, - Opcode::Ishl => binary(DataValueExt::shl, arg(0), arg(1))?, - Opcode::Ushr => binary(DataValueExt::ushr, arg(0), arg(1))?, - Opcode::Sshr => binary(DataValueExt::sshr, arg(0), arg(1))?, - Opcode::IshlImm => binary(DataValueExt::shl, arg(0), imm_as_ctrl_ty()?)?, - Opcode::UshrImm => binary(DataValueExt::ushr, arg(0), imm_as_ctrl_ty()?)?, - Opcode::SshrImm => binary(DataValueExt::sshr, arg(0), imm_as_ctrl_ty()?)?, + Opcode::Rotl => binary(DataValueExt::rotl, arg(0), shift_amt(ctrl_ty, arg(1))?)?, + Opcode::Rotr => binary(DataValueExt::rotr, arg(0), shift_amt(ctrl_ty, arg(1))?)?, + Opcode::RotlImm => binary(DataValueExt::rotl, arg(0), shift_amt(ctrl_ty, imm())?)?, + Opcode::RotrImm => binary(DataValueExt::rotr, arg(0), shift_amt(ctrl_ty, imm())?)?, + Opcode::Ishl => binary(DataValueExt::shl, arg(0), shift_amt(ctrl_ty, arg(1))?)?, + Opcode::Ushr => binary(DataValueExt::ushr, arg(0), shift_amt(ctrl_ty, arg(1))?)?, + Opcode::Sshr => binary(DataValueExt::sshr, arg(0), shift_amt(ctrl_ty, arg(1))?)?, + Opcode::IshlImm => binary(DataValueExt::shl, arg(0), shift_amt(ctrl_ty, imm())?)?, + Opcode::UshrImm => binary(DataValueExt::ushr, arg(0), shift_amt(ctrl_ty, imm())?)?, + Opcode::SshrImm => binary(DataValueExt::sshr, arg(0), shift_amt(ctrl_ty, imm())?)?, Opcode::Bitrev => unary(DataValueExt::reverse_bits, arg(0))?, Opcode::Bswap => unary(DataValueExt::swap_bytes, arg(0))?, Opcode::Clz => unary(DataValueExt::leading_zeros, arg(0))?, @@ -994,13 +994,7 @@ where } assign(DataValueExt::vector(new, types::I8X16)?) } - Opcode::Splat => { - let mut new_vector = SimdVec::new(); - for _ in 0..ctrl_ty.lane_count() { - new_vector.push(arg(0)); - } - assign(vectorizelanes(&new_vector, ctrl_ty)?) - } + Opcode::Splat => assign(splat(ctrl_ty, arg(0))?), Opcode::Insertlane => { let idx = imm().into_int_unsigned()? as usize; let mut vector = extractlanes(&arg(0), ctrl_ty)?; @@ -1575,3 +1569,17 @@ fn bitselect(c: DataValue, x: DataValue, y: DataValue) -> ValueResult let mask_y = DataValueExt::and(DataValueExt::not(c)?, y)?; DataValueExt::or(mask_x, mask_y) } + +fn splat(ty: Type, val: DataValue) -> ValueResult { + let mut new_vector = SimdVec::new(); + for _ in 0..ty.lane_count() { + new_vector.push(val.clone()); + } + vectorizelanes(&new_vector, ty) +} + +// Prepares the shift amount for a shift/rotate operation. +// The shift amount must be the same type and have the same number of lanes as the vector. +fn shift_amt(ty: Type, val: DataValue) -> ValueResult { + splat(ty, val.convert(ValueConversionKind::Exact(ty.lane_type()))?) +} From df8d3698b1e33ddb85453708740c46f6b6558d1a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 31 Aug 2023 20:12:37 +0200 Subject: [PATCH 3/8] Partially revert CLI argument changes from #6737 (#6944) * Partially revert CLI argument changes from #6737 This commit is a partial revert of #6737. That change was reverted in #6830 for the 12.0.0 release of Wasmtime and otherwise it's currently slated to get released with the 13.0.0 release of Wasmtime. Discussion at today's Wasmtime meeting concluded that it's best to couple this change with #6925 as a single release rather than spread out across multiple releases. This commit is thus the revert of #6737, although it's a partial revert in that I've kept many of the new tests added to showcase the differences before/after when the change lands. This means that Wasmtime 13.0.0 will exhibit the same CLI behavior as 12.0.0 and all prior releases. The 14.0.0 release will have both a new CLI and new argument passing semantics. I'll revert this revert (aka re-land #6737) once the 13.0.0 release branch is created and `main` becomes 14.0.0. * Update release notes --- RELEASES.md | 7 --- src/bin/wasmtime.rs | 46 ++++++++----------- src/commands/run.rs | 102 +++++++++++++++++++++++------------------ tests/all/cli_tests.rs | 28 +++++++---- 4 files changed, 94 insertions(+), 89 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 1162d8765031..8cc2cdb13d0a 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -80,13 +80,6 @@ Unreleased. These methods do not affect the size of the pre-allocated pool. [#6835](https://github.com/bytecodealliance/wasmtime/pull/6835) -* Options to the `wasmtime` CLI for Wasmtime itself must now come before the - WebAssembly module. For example `wasmtime run foo.wasm --disable-cache` now - must be specified as `wasmtime run --disable-cache foo.wasm`. Any - argument/option after the WebAssembly module is now interpreted as an argument - to the wasm module itself. - [#6737](https://github.com/bytecodealliance/wasmtime/pull/6737) - * Builder methods for WASI contexts onw use `&mut self` instead of `self`. [#6770](https://github.com/bytecodealliance/wasmtime/pull/6770) diff --git a/src/bin/wasmtime.rs b/src/bin/wasmtime.rs index d32a0b63f373..6d425625dfa4 100644 --- a/src/bin/wasmtime.rs +++ b/src/bin/wasmtime.rs @@ -4,7 +4,7 @@ //! See `wasmtime --help` for usage. use anyhow::Result; -use clap::Parser; +use clap::{error::ErrorKind, Parser}; use wasmtime_cli::commands::{ CompileCommand, ConfigCommand, ExploreCommand, RunCommand, SettingsCommand, WastCommand, }; @@ -27,24 +27,10 @@ use wasmtime_cli::commands::{ \n\ Invoking a specific function (e.g. `add`) in a WebAssembly module:\n\ \n \ - wasmtime example.wasm --invoke add 1 2\n", - - // This option enables the pattern below where we ask clap to parse twice - // sorta: once where it's trying to find a subcommand and once assuming - // a subcommand doesn't get passed. Clap should then, apparently, - // fill in the `subcommand` if found and otherwise fill in the - // `RunCommand`. - args_conflicts_with_subcommands = true + wasmtime example.wasm --invoke add 1 2\n" )] -struct Wasmtime { - #[clap(subcommand)] - subcommand: Option, - #[clap(flatten)] - run: RunCommand, -} - -#[derive(Parser)] -enum Subcommand { +enum Wasmtime { + // !!! IMPORTANT: if subcommands are added or removed, update `parse_module` in `src/commands/run.rs`. !!! /// Controls Wasmtime configuration settings Config(ConfigCommand), /// Compiles a WebAssembly module. @@ -62,20 +48,26 @@ enum Subcommand { impl Wasmtime { /// Executes the command. pub fn execute(self) -> Result<()> { - let subcommand = self.subcommand.unwrap_or(Subcommand::Run(self.run)); - match subcommand { - Subcommand::Config(c) => c.execute(), - Subcommand::Compile(c) => c.execute(), - Subcommand::Explore(c) => c.execute(), - Subcommand::Run(c) => c.execute(), - Subcommand::Settings(c) => c.execute(), - Subcommand::Wast(c) => c.execute(), + match self { + Self::Config(c) => c.execute(), + Self::Compile(c) => c.execute(), + Self::Explore(c) => c.execute(), + Self::Run(c) => c.execute(), + Self::Settings(c) => c.execute(), + Self::Wast(c) => c.execute(), } } } fn main() -> Result<()> { - Wasmtime::parse().execute() + Wasmtime::try_parse() + .unwrap_or_else(|e| match e.kind() { + ErrorKind::InvalidSubcommand | ErrorKind::UnknownArgument => { + Wasmtime::Run(RunCommand::parse()) + } + _ => e.exit(), + }) + .execute() } #[test] diff --git a/src/commands/run.rs b/src/commands/run.rs index 3a3f080c9dca..398b9bf686dc 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -6,8 +6,11 @@ )] use anyhow::{anyhow, bail, Context as _, Error, Result}; +use clap::builder::{OsStringValueParser, TypedValueParser}; use clap::Parser; use once_cell::sync::Lazy; +use std::ffi::OsStr; +use std::ffi::OsString; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; @@ -35,6 +38,18 @@ use wasmtime_wasi_threads::WasiThreadsCtx; // #[cfg(feature = "wasi-http")] // use wasmtime_wasi_http::WasiHttpCtx; +fn parse_module(s: OsString) -> anyhow::Result { + // Do not accept wasmtime subcommand names as the module name + match s.to_str() { + Some("help") | Some("config") | Some("run") | Some("wast") | Some("compile") => { + bail!("module name cannot be the same as a subcommand") + } + #[cfg(unix)] + Some("-") => Ok(PathBuf::from("/dev/stdin")), + _ => Ok(s.into()), + } +} + fn parse_env_var(s: &str) -> Result<(String, Option)> { let mut parts = s.splitn(2, '='); Ok(( @@ -103,7 +118,7 @@ static AFTER_HELP: Lazy = Lazy::new(|| crate::FLAG_EXPLANATIONS.to_strin /// Runs a WebAssembly module #[derive(Parser)] -#[structopt(name = "run", after_help = AFTER_HELP.as_str())] +#[structopt(name = "run", trailing_var_arg = true, after_help = AFTER_HELP.as_str())] pub struct RunCommand { #[clap(flatten)] common: CommonOptions, @@ -177,6 +192,14 @@ pub struct RunCommand { #[clap(long = "wasi-nn-graph", value_name = "FORMAT::HOST_DIR", value_parser = parse_graphs)] graphs: Vec<(String, String)>, + /// The path of the WebAssembly module to run + #[clap( + required = true, + value_name = "MODULE", + value_parser = OsStringValueParser::new().try_map(parse_module), + )] + module: PathBuf, + /// Load the given WebAssembly module before the main module #[clap( long = "preload", @@ -220,6 +243,11 @@ pub struct RunCommand { #[clap(long = "coredump-on-trap", value_name = "PATH")] coredump_on_trap: Option, + // NOTE: this must come last for trailing varargs + /// The arguments to pass to the module + #[clap(value_name = "ARGS")] + module_args: Vec, + /// Maximum size, in bytes, that a linear memory is allowed to reach. /// /// Growth beyond this limit will cause `memory.grow` instructions in @@ -258,14 +286,6 @@ pub struct RunCommand { #[clap(long)] wmemcheck: bool, - /// The WebAssembly module to run and arguments to pass to it. - /// - /// Arguments passed to the wasm module will be configured as WASI CLI - /// arguments unless the `--invoke` CLI argument is passed in which case - /// arguments will be interpreted as arguments to the function specified. - #[clap(value_name = "WASM", trailing_var_arg = true, required = true)] - module_and_args: Vec, - /// Indicates that the implementation of WASI preview1 should be backed by /// the preview2 implementation for components. /// @@ -337,7 +357,7 @@ impl RunCommand { let engine = Engine::new(&config)?; // Read the wasm module binary either as `*.wat` or a raw binary. - let main = self.load_module(&engine, &self.module_and_args[0])?; + let main = self.load_module(&engine, &self.module)?; // Validate coredump-on-trap argument if let Some(coredump_path) = self.coredump_on_trap.as_ref() { @@ -429,12 +449,8 @@ impl RunCommand { // Load the main wasm module. match self .load_main_module(&mut store, &mut linker, &main, modules) - .with_context(|| { - format!( - "failed to run main module `{}`", - self.module_and_args[0].display() - ) - }) { + .with_context(|| format!("failed to run main module `{}`", self.module.display())) + { Ok(()) => (), Err(e) => { // Exit the process if Wasmtime understands the error; @@ -483,25 +499,27 @@ impl RunCommand { Ok(listeners) } - fn compute_argv(&self) -> Result> { + fn compute_argv(&self) -> Vec { let mut result = Vec::new(); - for (i, arg) in self.module_and_args.iter().enumerate() { - // For argv[0], which is the program name. Only include the base - // name of the main wasm module, to avoid leaking path information. - let arg = if i == 0 { - arg.components().next_back().unwrap().as_os_str() - } else { - arg.as_ref() - }; - result.push( - arg.to_str() - .ok_or_else(|| anyhow!("failed to convert {arg:?} to utf-8"))? - .to_string(), - ); + // Add argv[0], which is the program name. Only include the base name of the + // main wasm module, to avoid leaking path information. + result.push( + self.module + .components() + .next_back() + .map(|c| c.as_os_str()) + .and_then(OsStr::to_str) + .unwrap_or("") + .to_owned(), + ); + + // Add the remaining arguments. + for arg in self.module_args.iter() { + result.push(arg.clone()); } - Ok(result) + result } fn setup_epoch_handler( @@ -510,7 +528,7 @@ impl RunCommand { modules: Vec<(String, Module)>, ) -> Box)> { if let Some(Profile::Guest { path, interval }) = &self.profile { - let module_name = self.module_and_args[0].to_str().unwrap_or("
"); + let module_name = self.module.to_str().unwrap_or("
"); let interval = *interval; store.data_mut().guest_profiler = Some(Arc::new(GuestProfiler::new(module_name, interval, modules))); @@ -616,10 +634,9 @@ impl RunCommand { CliLinker::Core(linker) => { // Use "" as a default module name. let module = module.unwrap_core(); - linker.module(&mut *store, "", &module).context(format!( - "failed to instantiate {:?}", - self.module_and_args[0] - ))?; + linker + .module(&mut *store, "", &module) + .context(format!("failed to instantiate {:?}", self.module))?; // If a function to invoke was given, invoke it. let func = if let Some(name) = &self.invoke { @@ -678,7 +695,7 @@ impl RunCommand { is experimental and may break in the future" ); } - let mut args = self.module_and_args.iter().skip(1); + let mut args = self.module_args.iter(); let mut values = Vec::new(); for ty in ty.params() { let val = match args.next() { @@ -691,9 +708,6 @@ impl RunCommand { } } }; - let val = val - .to_str() - .ok_or_else(|| anyhow!("argument is not valid utf-8: {val:?}"))?; values.push(match ty { // TODO: integer parsing here should handle hexadecimal notation // like `0x0...`, but the Rust standard library currently only @@ -751,9 +765,7 @@ impl RunCommand { if !err.is::() { return err; } - let source_name = self.module_and_args[0] - .to_str() - .unwrap_or_else(|| "unknown"); + let source_name = self.module.to_str().unwrap_or_else(|| "unknown"); if let Err(coredump_err) = generate_coredump(&err, &source_name, coredump_path) { eprintln!("warning: coredump failed to generate: {}", coredump_err); @@ -990,7 +1002,7 @@ impl RunCommand { fn set_preview1_ctx(&self, store: &mut Store) -> Result<()> { let mut builder = WasiCtxBuilder::new(); - builder.inherit_stdio().args(&self.compute_argv()?)?; + builder.inherit_stdio().args(&self.compute_argv())?; for (key, value) in self.vars.iter() { let value = match value { @@ -1022,7 +1034,7 @@ impl RunCommand { fn set_preview2_ctx(&self, store: &mut Store) -> Result<()> { let mut builder = preview2::WasiCtxBuilder::new(); - builder.inherit_stdio().args(&self.compute_argv()?); + builder.inherit_stdio().args(&self.compute_argv()); for (key, value) in self.vars.iter() { let value = match value { diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 29561129ce61..f144dd933e66 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -604,6 +604,7 @@ fn wasm_flags() -> Result<()> { let stdout = run_wasmtime(&[ "run", "tests/all/cli_tests/print-arguments.wat", + "--", "--argument", "-for", "the", @@ -619,7 +620,7 @@ fn wasm_flags() -> Result<()> { command\n\ " ); - let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "-"])?; + let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--", "-"])?; assert_eq!( stdout, "\ @@ -627,7 +628,7 @@ fn wasm_flags() -> Result<()> { -\n\ " ); - let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--"])?; + let stdout = run_wasmtime(&["run", "tests/all/cli_tests/print-arguments.wat", "--", "--"])?; assert_eq!( stdout, "\ @@ -648,7 +649,6 @@ fn wasm_flags() -> Result<()> { "\ print-arguments.wat\n\ --\n\ - --\n\ -a\n\ b\n\ " @@ -658,30 +658,37 @@ fn wasm_flags() -> Result<()> { #[test] fn name_same_as_builtin_command() -> Result<()> { - // a bare subcommand shouldn't run successfully + // A bare subcommand shouldn't run successfully. + // + // This is ambiguous between a missing module argument and a module named + // `run` with no other options. let output = get_wasmtime_command()? .current_dir("tests/all/cli_tests") .arg("run") .output()?; assert!(!output.status.success()); - // a `--` prefix should let everything else get interpreted as a wasm - // module and arguments, even if the module has a name like `run` + // Currently even `--` isn't enough to disambiguate, so this still is an + // error. + // + // NB: this will change in Wasmtime 14 when #6737 is relanded. let output = get_wasmtime_command()? .current_dir("tests/all/cli_tests") .arg("--") .arg("run") .output()?; - assert!(output.status.success(), "expected success got {output:#?}"); + assert!(!output.status.success(), "expected failure got {output:#?}"); - // Passing options before the subcommand should work and doesn't require - // `--` to disambiguate + // Passing `--foo` options before the module is also not enough to + // disambiguate for now. + // + // NB: this will change in Wasmtime 14 when #6737 is relanded. let output = get_wasmtime_command()? .current_dir("tests/all/cli_tests") .arg("--disable-cache") .arg("run") .output()?; - assert!(output.status.success(), "expected success got {output:#?}"); + assert!(!output.status.success(), "expected failure got {output:#?}"); Ok(()) } @@ -701,6 +708,7 @@ fn wasm_flags_without_subcommand() -> Result<()> { let output = get_wasmtime_command()? .current_dir("tests/all/cli_tests/") .arg("print-arguments.wat") + .arg("--") .arg("-foo") .arg("bar") .output()?; From 5ec731874768aaf64e9184132c124de87798394d Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Thu, 31 Aug 2023 19:24:25 +0100 Subject: [PATCH 4/8] riscv64: Use `PCRelLo12I` relocation on Loads (#6938) * riscv64: Use `PCRelLo12I` relocation on Loads * riscv64: Strenghten pattern matching when emitting Load's * riscv64: Clarify some of the load address logic * riscv64: Even stronger matching --- .../codegen/src/isa/riscv64/inst/args.rs | 12 +++ .../codegen/src/isa/riscv64/inst/emit.rs | 47 ++++++++--- .../filetests/isa/riscv64/constants.clif | 78 +++++++++---------- .../filetests/isa/riscv64/fcvt-small.clif | 26 +++---- .../filetests/isa/riscv64/float.clif | 72 +++++++---------- .../filetests/isa/riscv64/return-call.clif | 6 +- .../filetests/isa/riscv64/simd-ceil.clif | 6 +- .../filetests/isa/riscv64/simd-floor.clif | 6 +- .../filetests/isa/riscv64/simd-fmax.clif | 6 +- .../filetests/isa/riscv64/simd-fmin.clif | 6 +- .../isa/riscv64/simd-iadd_pairwise.clif | 30 +++---- .../filetests/isa/riscv64/simd-nearest.clif | 6 +- .../filetests/isa/riscv64/simd-popcnt.clif | 20 ++--- .../filetests/isa/riscv64/simd-trunc.clif | 6 +- 14 files changed, 163 insertions(+), 164 deletions(-) diff --git a/cranelift/codegen/src/isa/riscv64/inst/args.rs b/cranelift/codegen/src/isa/riscv64/inst/args.rs index 40ebbc94fd1f..18d8ae9b7546 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/args.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/args.rs @@ -161,6 +161,18 @@ impl AMode { } } + /// Retrieve a MachLabel that corresponds to this addressing mode, if it exists. + pub(crate) fn get_label_with_sink(&self, sink: &mut MachBuffer) -> Option { + match self { + &AMode::Const(addr) => Some(sink.get_label_for_constant(addr)), + &AMode::Label(label) => Some(label), + &AMode::RegOffset(..) + | &AMode::SPOffset(..) + | &AMode::FPOffset(..) + | &AMode::NominalSPOffset(..) => None, + } + } + pub(crate) fn to_string_with_alloc(&self, allocs: &mut AllocationConsumer<'_>) -> String { format!("{}", self.clone().with_allocs(allocs)) } diff --git a/cranelift/codegen/src/isa/riscv64/inst/emit.rs b/cranelift/codegen/src/isa/riscv64/inst/emit.rs index 10ab5649412f..48b2c8ea3a0e 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/emit.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/emit.rs @@ -620,18 +620,49 @@ impl MachInstEmit for Inst { let base = from.get_base_register(); let offset = from.get_offset_with_state(state); let offset_imm12 = Imm12::maybe_from_i64(offset); + let label = from.get_label_with_sink(sink); - // TODO: We shouldn't just fall back to `LoadAddr` immediately. For `MachLabel`s - // we should try to emit the `auipc` and add a relocation on this load. - let (addr, imm12) = match (base, offset_imm12) { - // If the offset fits into an imm12 we can directly encode it. - (Some(base), Some(imm12)) => (base, imm12), - // Otherwise load the address it into a reg and load from it. - _ => { + let (addr, imm12) = match (base, offset_imm12, label) { + // When loading from a Reg+Offset, if the offset fits into an imm12 we can directly encode it. + (Some(base), Some(imm12), None) => (base, imm12), + + // Otherwise, if the offset does not fit into a imm12, we need to materialize it into a + // register and load from that. + (Some(_), None, None) => { let tmp = writable_spilltmp_reg(); Inst::LoadAddr { rd: tmp, mem: from }.emit(&[], sink, emit_info, state); (tmp.to_reg(), Imm12::zero()) } + + // If the AMode contains a label we can emit an internal relocation that gets + // resolved with the correct address later. + (None, Some(imm), Some(label)) => { + debug_assert_eq!(imm.as_i16(), 0); + + // Get the current PC. + sink.use_label_at_offset(sink.cur_offset(), label, LabelUse::PCRelHi20); + Inst::Auipc { + rd, + imm: Imm20::from_bits(0), + } + .emit(&[], sink, emit_info, state); + + // Emit a relocation for the load. This patches the offset into the instruction. + sink.use_label_at_offset(sink.cur_offset(), label, LabelUse::PCRelLo12I); + + // Imm12 here is meaningless since it's going to get replaced. + (rd.to_reg(), Imm12::zero()) + } + + // These cases are impossible with the current AModes that we have. We either + // always have a register, or always have a label. Never both, and never neither. + (None, None, None) + | (None, Some(_), None) + | (Some(_), None, Some(_)) + | (Some(_), Some(_), Some(_)) + | (None, None, Some(_)) => { + unreachable!("Invalid load address") + } }; let srcloc = state.cur_srcloc(); @@ -650,8 +681,6 @@ impl MachInstEmit for Inst { let offset = to.get_offset_with_state(state); let offset_imm12 = Imm12::maybe_from_i64(offset); - // TODO: We shouldn't just fall back to `LoadAddr` immediately. For `MachLabel`s - // we should try to emit the `auipc` and add a relocation on this store. let (addr, imm12) = match (base, offset_imm12) { // If the offset fits into an imm12 we can directly encode it. (Some(base), Some(imm12)) => (base, imm12), diff --git a/cranelift/filetests/filetests/isa/riscv64/constants.clif b/cranelift/filetests/filetests/isa/riscv64/constants.clif index 9a1afa3b29df..ed8ef4192dbd 100644 --- a/cranelift/filetests/filetests/isa/riscv64/constants.clif +++ b/cranelift/filetests/filetests/isa/riscv64/constants.clif @@ -81,10 +81,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xff, 0xff ; .byte 0x00, 0x00, 0x00, 0x00 @@ -101,11 +101,11 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xff, 0xff, 0x00, 0x00 function %f() -> i64 { @@ -121,11 +121,11 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xff, 0xff function %f() -> i64 { @@ -173,10 +173,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xff, 0xff, 0x00, 0x00 ; .byte 0xff, 0xff, 0xff, 0xff @@ -193,10 +193,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xff, 0xff, 0xff, 0xff ; .byte 0x00, 0x00, 0xff, 0xff @@ -213,10 +213,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xff, 0xff, 0xff, 0xff ; .byte 0xff, 0xff, 0x00, 0x00 @@ -233,10 +233,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x3a, 0x00, 0x12, 0x12 ; .byte 0xa3, 0xf0, 0x4b, 0xf3 @@ -253,10 +253,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf4, 0x1e ; .byte 0x00, 0x00, 0xe9, 0x12 @@ -273,10 +273,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xff, 0xff, 0xf4, 0x1e ; .byte 0xff, 0xff, 0xe9, 0x12 @@ -325,10 +325,10 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x10 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x10(a0) ; ret +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0xf7, 0xff, 0xff, 0xff ; .byte 0x00, 0x00, 0x00, 0x00 @@ -362,13 +362,11 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x18 -; ld t0, 0(t6) +; auipc t0, 0 +; ld t0, 0x10(t0) ; fmv.d.x fa0, t0 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 -; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0x3f function %f() -> f32 { @@ -403,13 +401,11 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x18 -; ld t0, 0(t6) +; auipc t0, 0 +; ld t0, 0x10(t0) ; fmv.d.x fa0, t0 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 -; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x49, 0x40 function %f() -> f32 { @@ -480,13 +476,11 @@ block0: ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x18 -; ld t0, 0(t6) +; auipc t0, 0 +; ld t0, 0x10(t0) ; fmv.d.x fa0, t0 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 -; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0xc0 function %f() -> f32 { diff --git a/cranelift/filetests/filetests/isa/riscv64/fcvt-small.clif b/cranelift/filetests/filetests/isa/riscv64/fcvt-small.clif index feb8e7c7d452..d85137c8e3f2 100644 --- a/cranelift/filetests/filetests/isa/riscv64/fcvt-small.clif +++ b/cranelift/filetests/filetests/isa/riscv64/fcvt-small.clif @@ -92,10 +92,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x00, 0x00, 0x80, 0xbf ; fmv.w.x ft3, t6 @@ -126,10 +125,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0xbf @@ -138,8 +136,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x70, 0x40 @@ -166,10 +163,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x00, 0x00, 0x80, 0xbf ; fmv.w.x ft3, t6 @@ -200,10 +196,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0xbf @@ -212,8 +207,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0x40 diff --git a/cranelift/filetests/filetests/isa/riscv64/float.clif b/cranelift/filetests/filetests/isa/riscv64/float.clif index bf33bab5cf5d..7b839f037b5b 100644 --- a/cranelift/filetests/filetests/isa/riscv64/float.clif +++ b/cranelift/filetests/filetests/isa/riscv64/float.clif @@ -453,10 +453,9 @@ block0(v0: f64): ; block0: ; offset 0x0 ; fmv.d ft5, fa0 ; feq.d a0, ft5, ft5 -; beqz a0, 0x3c +; beqz a0, 0x38 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x40, 0x43 @@ -520,10 +519,9 @@ block0(v0: f64): ; block0: ; offset 0x0 ; fmv.d ft5, fa0 ; feq.d a0, ft5, ft5 -; beqz a0, 0x3c +; beqz a0, 0x38 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x40, 0x43 @@ -587,10 +585,9 @@ block0(v0: f64): ; block0: ; offset 0x0 ; fmv.d ft5, fa0 ; feq.d a0, ft5, ft5 -; beqz a0, 0x3c +; beqz a0, 0x38 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x40, 0x43 @@ -654,10 +651,9 @@ block0(v0: f64): ; block0: ; offset 0x0 ; fmv.d ft5, fa0 ; feq.d a0, ft5, ft5 -; beqz a0, 0x3c +; beqz a0, 0x38 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x40, 0x43 @@ -752,10 +748,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x00, 0x00, 0x80, 0xbf ; fmv.w.x ft3, t6 @@ -786,10 +781,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x01, 0x00, 0x00, 0xcf ; fmv.w.x ft3, t6 @@ -820,10 +814,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x00, 0x00, 0x80, 0xbf ; fmv.w.x ft3, t6 @@ -854,10 +847,9 @@ block0(v0: f32): ; Disassembled: ; block0: ; offset 0x0 ; feq.s a0, fa0, fa0 -; beqz a0, 0x44 +; beqz a0, 0x40 ; auipc t6, 0 -; addi t6, t6, 0x10 -; lw t6, 0(t6) +; lw t6, 0xc(t6) ; j 8 ; .byte 0x01, 0x00, 0x00, 0xdf ; fmv.w.x ft3, t6 @@ -888,10 +880,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0xbf @@ -900,8 +891,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0x41 @@ -928,10 +918,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x20, 0x00 ; .byte 0x00, 0x00, 0xe0, 0xc1 @@ -940,8 +929,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xe0, 0x41 @@ -968,10 +956,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0xbf @@ -980,8 +967,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf0, 0x43 @@ -1008,10 +994,9 @@ block0(v0: f64): ; Disassembled: ; block0: ; offset 0x0 ; feq.d a0, fa0, fa0 -; beqz a0, 0x5c +; beqz a0, 0x54 ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x01, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xe0, 0xc3 @@ -1020,8 +1005,7 @@ block0(v0: f64): ; beqz a0, 8 ; .byte 0x00, 0x00, 0x00, 0x00 ; trap: int_ovf ; auipc t6, 0 -; addi t6, t6, 0x10 -; ld t6, 0(t6) +; ld t6, 0xc(t6) ; j 0xc ; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xe0, 0x43 diff --git a/cranelift/filetests/filetests/isa/riscv64/return-call.clif b/cranelift/filetests/filetests/isa/riscv64/return-call.clif index f09e9bf611e7..335e3cb7532d 100644 --- a/cranelift/filetests/filetests/isa/riscv64/return-call.clif +++ b/cranelift/filetests/filetests/isa/riscv64/return-call.clif @@ -103,13 +103,13 @@ block0(v0: f64): ; ; Disassembled: ; block0: ; offset 0x0 -; auipc t6, 0 -; addi t6, t6, 0x18 -; ld a0, 0(t6) +; auipc a0, 0 +; ld a0, 0x18(a0) ; fmv.d.x ft5, a0 ; fadd.d ft0, ft0, ft5 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0x40 function %call_f64(f64) -> f64 tail { diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif b/cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif index 3cc3b9b21a46..d9fd8fe4ca4e 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif @@ -107,9 +107,8 @@ block0(v0: f64x2): ; .byte 0x87, 0x80, 0x0f, 0x02 ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0x92, 0x10, 0x2a -; auipc t6, 0 -; addi t6, t6, 0x4c -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x4c(a3) ; fmv.d.x fa0, a3 ; .byte 0x57, 0x50, 0x45, 0x6e ; fsrmi t4, 3 @@ -127,5 +126,6 @@ block0(v0: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0x43 diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-floor.clif b/cranelift/filetests/filetests/isa/riscv64/simd-floor.clif index 6ecdcc7c88f0..3c045f8f95f6 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-floor.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-floor.clif @@ -107,9 +107,8 @@ block0(v0: f64x2): ; .byte 0x87, 0x80, 0x0f, 0x02 ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0x92, 0x10, 0x2a -; auipc t6, 0 -; addi t6, t6, 0x4c -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x4c(a3) ; fmv.d.x fa0, a3 ; .byte 0x57, 0x50, 0x45, 0x6e ; fsrmi t4, 2 @@ -127,5 +126,6 @@ block0(v0: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0x43 diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-fmax.clif b/cranelift/filetests/filetests/isa/riscv64/simd-fmax.clif index d3805bfb9a6e..d576a7dcd744 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-fmax.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-fmax.clif @@ -45,9 +45,8 @@ block0(v0: f64x2, v1: f64x2): ; .byte 0x57, 0x93, 0x10, 0x62 ; .byte 0x57, 0x94, 0x31, 0x62 ; .byte 0x57, 0x20, 0x64, 0x66 -; auipc t6, 0 -; addi t6, t6, 0x34 -; ld t4, 0(t6) +; auipc t4, 0 +; ld t4, 0x2c(t4) ; .byte 0x57, 0xc7, 0x0e, 0x5e ; .byte 0x57, 0x98, 0x11, 0x1a ; .byte 0x57, 0x09, 0xe8, 0x5c @@ -58,7 +57,6 @@ block0(v0: f64x2, v1: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 -; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf8, 0x7f function %fmax_f32x4(f32x4, f32x4) -> f32x4 { diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-fmin.clif b/cranelift/filetests/filetests/isa/riscv64/simd-fmin.clif index 80fe5210dc69..d653dae0a22e 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-fmin.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-fmin.clif @@ -45,9 +45,8 @@ block0(v0: f64x2, v1: f64x2): ; .byte 0x57, 0x93, 0x10, 0x62 ; .byte 0x57, 0x94, 0x31, 0x62 ; .byte 0x57, 0x20, 0x64, 0x66 -; auipc t6, 0 -; addi t6, t6, 0x34 -; ld t4, 0(t6) +; auipc t4, 0 +; ld t4, 0x2c(t4) ; .byte 0x57, 0xc7, 0x0e, 0x5e ; .byte 0x57, 0x98, 0x11, 0x12 ; .byte 0x57, 0x09, 0xe8, 0x5c @@ -58,7 +57,6 @@ block0(v0: f64x2, v1: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 -; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0xf8, 0x7f function %fmin_f32x4(f32x4, f32x4) -> f32x4 { diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-iadd_pairwise.clif b/cranelift/filetests/filetests/isa/riscv64/simd-iadd_pairwise.clif index 02ea0cb5766d..c99beb5fa68f 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-iadd_pairwise.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-iadd_pairwise.clif @@ -45,18 +45,16 @@ block0(v0: i8x16, v1: i8x16): ; .byte 0x87, 0x80, 0x0f, 0x02 ; addi t6, s0, 0x20 ; .byte 0x87, 0x81, 0x0f, 0x02 -; auipc t6, 0 -; addi t6, t6, 0x64 -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x5c(a3) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe4, 0x06, 0x42 ; .byte 0x57, 0x70, 0x08, 0xcc ; .byte 0x57, 0x26, 0x14, 0x5e ; .byte 0xd7, 0x26, 0x34, 0x5e ; .byte 0x57, 0x36, 0xd4, 0x3a -; auipc t6, 0 -; addi t6, t6, 0x48 -; ld a1, 0(t6) +; auipc a1, 0 +; ld a1, 0x44(a1) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe9, 0x05, 0x42 ; .byte 0x57, 0x70, 0x08, 0xcc @@ -118,18 +116,16 @@ block0(v0: i16x8, v1: i16x8): ; .byte 0x87, 0x80, 0x0f, 0x02 ; addi t6, s0, 0x20 ; .byte 0x87, 0x81, 0x0f, 0x02 -; auipc t6, 0 -; addi t6, t6, 0x64 -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x5c(a3) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe4, 0x06, 0x42 ; .byte 0x57, 0x70, 0x84, 0xcc ; .byte 0x57, 0x26, 0x14, 0x5e ; .byte 0xd7, 0x26, 0x34, 0x5e ; .byte 0x57, 0x36, 0xd2, 0x3a -; auipc t6, 0 -; addi t6, t6, 0x48 -; ld a1, 0(t6) +; auipc a1, 0 +; ld a1, 0x44(a1) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe9, 0x05, 0x42 ; .byte 0x57, 0x70, 0x84, 0xcc @@ -191,18 +187,16 @@ block0(v0: i32x4, v1: i32x4): ; .byte 0x87, 0x80, 0x0f, 0x02 ; addi t6, s0, 0x20 ; .byte 0x87, 0x81, 0x0f, 0x02 -; auipc t6, 0 -; addi t6, t6, 0x64 -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x5c(a3) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe4, 0x06, 0x42 ; .byte 0x57, 0x70, 0x02, 0xcd ; .byte 0x57, 0x26, 0x14, 0x5e ; .byte 0xd7, 0x26, 0x34, 0x5e ; .byte 0x57, 0x36, 0xd1, 0x3a -; auipc t6, 0 -; addi t6, t6, 0x48 -; ld a1, 0(t6) +; auipc a1, 0 +; ld a1, 0x44(a1) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xe9, 0x05, 0x42 ; .byte 0x57, 0x70, 0x02, 0xcd diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-nearest.clif b/cranelift/filetests/filetests/isa/riscv64/simd-nearest.clif index 359fdf6a1c90..cf2e1892d13a 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-nearest.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-nearest.clif @@ -107,9 +107,8 @@ block0(v0: f64x2): ; .byte 0x87, 0x80, 0x0f, 0x02 ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0x92, 0x10, 0x2a -; auipc t6, 0 -; addi t6, t6, 0x4c -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x4c(a3) ; fmv.d.x fa0, a3 ; .byte 0x57, 0x50, 0x45, 0x6e ; fsrmi t4, 0 @@ -127,5 +126,6 @@ block0(v0: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0x43 diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-popcnt.clif b/cranelift/filetests/filetests/isa/riscv64/simd-popcnt.clif index 40246e80ea1f..e81fe132e19f 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-popcnt.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-popcnt.clif @@ -273,29 +273,25 @@ block0(v0: i64x2): ; .byte 0x57, 0x70, 0x08, 0xcc ; addi t6, s0, 0x10 ; .byte 0x87, 0x80, 0x0f, 0x02 -; auipc t6, 0 -; addi t6, t6, 0x84 -; ld a1, 0(t6) +; auipc a1, 0 +; ld a1, 0x74(a1) ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0xb3, 0x10, 0xa2 ; .byte 0x57, 0xc4, 0x65, 0x26 ; .byte 0x57, 0x05, 0x14, 0x0a -; auipc t6, 0 -; addi t6, t6, 0x70 -; ld t4, 0(t6) +; auipc t4, 0 +; ld t4, 0x64(t4) ; .byte 0x57, 0x37, 0xa1, 0xa2 ; .byte 0x57, 0xc8, 0xee, 0x26 ; .byte 0x57, 0xc9, 0xae, 0x26 ; .byte 0x57, 0x0a, 0x28, 0x03 -; auipc t6, 0 -; addi t6, t6, 0x5c -; ld a6, 0(t6) +; auipc a6, 0 +; ld a6, 0x54(a6) ; .byte 0x57, 0x3c, 0x42, 0xa3 ; .byte 0x57, 0x0d, 0x4c, 0x03 ; .byte 0x57, 0x4e, 0xa8, 0x27 -; auipc t6, 0 -; addi t6, t6, 0x4c -; ld a1, 0(t6) +; auipc a1, 0 +; ld a1, 0x48(a1) ; .byte 0x57, 0xe0, 0xc5, 0x97 ; addi a5, zero, 0x38 ; .byte 0x57, 0xc2, 0x07, 0xa2 diff --git a/cranelift/filetests/filetests/isa/riscv64/simd-trunc.clif b/cranelift/filetests/filetests/isa/riscv64/simd-trunc.clif index 515643924a38..2a9346870bf7 100644 --- a/cranelift/filetests/filetests/isa/riscv64/simd-trunc.clif +++ b/cranelift/filetests/filetests/isa/riscv64/simd-trunc.clif @@ -101,9 +101,8 @@ block0(v0: f64x2): ; .byte 0x87, 0x80, 0x0f, 0x02 ; .byte 0x57, 0x70, 0x81, 0xcd ; .byte 0x57, 0x92, 0x10, 0x2a -; auipc t6, 0 -; addi t6, t6, 0x44 -; ld a3, 0(t6) +; auipc a3, 0 +; ld a3, 0x44(a3) ; fmv.d.x fa0, a3 ; .byte 0x57, 0x50, 0x45, 0x6e ; .byte 0x57, 0x96, 0x13, 0x4a @@ -119,5 +118,6 @@ block0(v0: f64x2): ; addi sp, sp, 0x10 ; ret ; .byte 0x00, 0x00, 0x00, 0x00 +; .byte 0x00, 0x00, 0x00, 0x00 ; .byte 0x00, 0x00, 0x30, 0x43 From a04c4930bf2ab86c4a0ade141a529d5597c75511 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 31 Aug 2023 22:04:59 +0200 Subject: [PATCH 5/8] Update Rust in CI to 1.72.0, clarify Wasmtime's MSRV (#6900) * Update Rust in CI to 1.72.0 * Update CI, tooling, and docs for MSRV This commit codifies an MSRV policy for Wasmtime at "stable minus two" meaning that the latest three releases of Rust will be supported. This is enforced on CI with a full test suite job running on Linux x86_64 with the minimum supported Rust version. The full test suite will use the latest stable version. A downside of this approach is that new changes may break MSRV support on non-Linux or non-x86_64 platforms and we won't know about it, but that's deemed a minor enough risk at this time. A minor fix is applied to Wasmtime's `Cargo.toml` to support Rust 1.70.0 instead of requiring Rust 1.71.0 * Fix installation of rust * Scrape MSRV from Cargo.toml * Cranelift is the same as Wasmtime's MSRV now, more words too * Fix a typo --- .github/actions/install-rust/action.yml | 27 +++++++++++++++++-------- .github/workflows/main.yml | 2 ++ Cargo.toml | 4 +++- ci/build-test-matrix.js | 15 ++++++++++++++ crates/wasmtime/Cargo.toml | 2 +- docs/contributing-coding-guidelines.md | 15 ++++++++------ 6 files changed, 49 insertions(+), 16 deletions(-) diff --git a/.github/actions/install-rust/action.yml b/.github/actions/install-rust/action.yml index 04b227cbc60a..003c994f6dfc 100644 --- a/.github/actions/install-rust/action.yml +++ b/.github/actions/install-rust/action.yml @@ -5,7 +5,7 @@ inputs: toolchain: description: 'Default toolchan to install' required: false - default: '1.71.0' + default: 'default' lockfiles: description: 'Path glob for Cargo.lock files to use as cache keys' required: false @@ -14,13 +14,28 @@ inputs: runs: using: composite steps: + - name: Install Rust + shell: bash + id: select + run: | + # Determine MSRV as N in `1.N.0` by looking at the `rust-version` + # located in the root `Cargo.toml`. + msrv=$(grep 'rust-version.*1' Cargo.toml | sed 's/.*\.\([0-9]*\)\..*/\1/') + + if [ "${{ inputs.toolchain }}" = "default" ]; then + echo "version=1.$((msrv+2)).0" >> "$GITHUB_OUTPUT" + elif [ "${{ inputs.toolchain }}" = "msrv" ]; then + echo "version=1.$msrv.0" >> "$GITHUB_OUTPUT" + else + echo "version=${{ inputs.toolchain }}" >> "$GITHUB_OUTPUT" + fi - name: Install Rust shell: bash run: | rustup set profile minimal - rustup update "${{ inputs.toolchain }}" --no-self-update - rustup default "${{ inputs.toolchain }}" + rustup update "${{ steps.select.outputs.version }}" --no-self-update + rustup default "${{ steps.select.outputs.version }}" # Save disk space by avoiding incremental compilation. Also turn down # debuginfo from 2 to 0 to help save disk space. @@ -31,11 +46,7 @@ runs: EOF # Deny warnings on CI to keep our code warning-free as it lands in-tree. - # Don't do this on nightly though, since there's a fair amount of - # warning churn there. - if [[ "${{ inputs.toolchain }}" != nightly* ]]; then - echo RUSTFLAGS="-D warnings" >> "$GITHUB_ENV" - fi + echo RUSTFLAGS="-D warnings" >> "$GITHUB_ENV" if [[ "${{ runner.os }}" = "macOS" ]]; then cat >> "$GITHUB_ENV" < Date: Thu, 31 Aug 2023 22:18:25 +0200 Subject: [PATCH 6/8] aarch64: Use `RegScaled*` addressing modes (#6945) This commit adds a few cases to `amode` construction on AArch64 for using the `RegScaled*` variants of `AMode`. This won't affect wasm due to this only matching the sign-extension happening before the shift, but it should otherwise help non-wasm Cranelift use cases. Closes #6742 --- cranelift/codegen/src/isa/aarch64/inst.isle | 26 +- cranelift/codegen/src/prelude.isle | 1 + .../filetests/isa/aarch64/amodes.clif | 266 ++++++++++++++++++ 3 files changed, 291 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 806387d9183a..edfa31432ecf 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -3122,15 +3122,37 @@ (rule 5 (amode ty (iadd (sextend x @ (value_type $I32)) y) offset) (AMode.RegExtended (amode_add y offset) x (ExtendOp.SXTW))) +;; `RegScaled*` rules where this matches an addition of an "index register" to a +;; base register. The index register is shifted by the size of the type loaded +;; in bytes to enable this mode matching. +;; +;; Note that this can additionally bundle an extending operation but the +;; extension must happen before the shift. This will pattern-match the shift +;; first and then if that succeeds afterwards try to find an extend. +(rule 6 (amode ty (iadd x (ishl y (iconst (u64_from_imm64 n)))) offset) + (if-let $true (u64_eq (ty_bytes ty) (u64_shl 1 n))) + (amode_reg_scaled (amode_add x offset) y ty)) +(rule 7 (amode ty (iadd (ishl y (iconst (u64_from_imm64 n))) x) offset) + (if-let $true (u64_eq (ty_bytes ty) (u64_shl 1 n))) + (amode_reg_scaled (amode_add x offset) y ty)) + +(decl amode_reg_scaled (Reg Value Type) AMode) +(rule 0 (amode_reg_scaled base index ty) + (AMode.RegScaled base index ty)) +(rule 1 (amode_reg_scaled base (uextend index @ (value_type $I32)) ty) + (AMode.RegScaledExtended base index ty (ExtendOp.UXTW))) +(rule 2 (amode_reg_scaled base (sextend index @ (value_type $I32)) ty) + (AMode.RegScaledExtended base index ty (ExtendOp.SXTW))) + ;; Small optimizations where constants found in `iadd` are folded into the ;; `offset` immediate. ;; ;; NB: this should probably be done by mid-end optimizations rather than here ;; in the backend, but currently Cranelift doesn't do that. -(rule 6 (amode ty (iadd x (iconst (simm32 y))) offset) +(rule 8 (amode ty (iadd x (iconst (simm32 y))) offset) (if-let new_offset (s32_add_fallible y offset)) (amode ty x new_offset)) -(rule 7 (amode ty (iadd (iconst (simm32 x)) y) offset) +(rule 9 (amode ty (iadd (iconst (simm32 x)) y) offset) (if-let new_offset (s32_add_fallible x offset)) (amode ty y new_offset)) diff --git a/cranelift/codegen/src/prelude.isle b/cranelift/codegen/src/prelude.isle index d83d4b518e86..2f4b720b1063 100644 --- a/cranelift/codegen/src/prelude.isle +++ b/cranelift/codegen/src/prelude.isle @@ -87,6 +87,7 @@ (decl pure u16_as_u64 (u16) u64) (extern constructor u16_as_u64 u16_as_u64) +(convert u16 u64 u16_as_u64) (decl pure u32_as_u64 (u32) u64) (extern constructor u32_as_u64 u32_as_u64) diff --git a/cranelift/filetests/filetests/isa/aarch64/amodes.clif b/cranelift/filetests/filetests/isa/aarch64/amodes.clif index 1bb7e0b2333e..176f70da7bd3 100644 --- a/cranelift/filetests/filetests/isa/aarch64/amodes.clif +++ b/cranelift/filetests/filetests/isa/aarch64/amodes.clif @@ -519,3 +519,269 @@ block0(v0: i64, v1: i32): ; stp x0, x1, [x6] ; ret +function %load_scaled16(i64, i64) -> i8 { +block0(v0: i64, v1: i64): + v2 = ishl_imm v1, 0 + v3 = iadd v0, v2 + v4 = load.i8 v3 + return v4 +} + +; VCode: +; block0: +; ldrb w0, [x0, x1, LSL #0] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrb w0, [x0, x1, lsl #0] +; ret + +function %load_scaled16(i64, i64) -> i16 { +block0(v0: i64, v1: i64): + v2 = ishl_imm v1, 1 + v3 = iadd v0, v2 + v4 = load.i16 v3 + return v4 +} + +; VCode: +; block0: +; ldrh w0, [x0, x1, LSL #1] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrh w0, [x0, x1, lsl #1] +; ret + +function %load_scaled32(i64, i64) -> i32 { +block0(v0: i64, v1: i64): + v2 = ishl_imm v1, 2 + v3 = iadd v0, v2 + v4 = load.i32 v3 + return v4 +} + +; VCode: +; block0: +; ldr w0, [x0, x1, LSL #2] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr w0, [x0, x1, lsl #2] +; ret + +function %load_scaled64(i64, i64) -> i64 { +block0(v0: i64, v1: i64): + v2 = ishl_imm v1, 3 + v3 = iadd v0, v2 + v4 = load.i64 v3 + return v4 +} + +; VCode: +; block0: +; ldr x0, [x0, x1, LSL #3] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr x0, [x0, x1, lsl #3] +; ret + +function %load_not_scaled64(i64, i64) -> i64 { +block0(v0: i64, v1: i64): + v2 = ishl_imm v1, 2 + v3 = iadd v0, v2 + v4 = load.i64 v3 + return v4 +} + +; VCode: +; block0: +; lsl x4, x1, #2 +; ldr x0, [x0, x4] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; lsl x4, x1, #2 +; ldr x0, [x0, x4] +; ret + +function %load_uextend_scaled16(i64, i32) -> i8 { +block0(v0: i64, v1: i32): + v2 = uextend.i64 v1 + v3 = ishl_imm v2, 0 + v4 = iadd v0, v3 + v5 = load.i8 v4 + return v5 +} + +; VCode: +; block0: +; ldrb w0, [x0, w1, UXTW #0] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrb w0, [x0, w1, uxtw #0] +; ret + +function %load_uextend_scaled16(i64, i32) -> i16 { +block0(v0: i64, v1: i32): + v2 = uextend.i64 v1 + v3 = ishl_imm v2, 1 + v4 = iadd v0, v3 + v5 = load.i16 v4 + return v5 +} + +; VCode: +; block0: +; ldrh w0, [x0, w1, UXTW #1] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrh w0, [x0, w1, uxtw #1] +; ret + +function %load_uextend_scaled32(i64, i32) -> i32 { +block0(v0: i64, v1: i32): + v2 = uextend.i64 v1 + v3 = ishl_imm v2, 2 + v4 = iadd v0, v3 + v5 = load.i32 v4 + return v5 +} + +; VCode: +; block0: +; ldr w0, [x0, w1, UXTW #2] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr w0, [x0, w1, uxtw #2] +; ret + + +function %load_uextend_scaled64(i64, i32) -> i64 { +block0(v0: i64, v1: i32): + v2 = uextend.i64 v1 + v3 = ishl_imm v2, 3 + v4 = iadd v0, v3 + v5 = load.i64 v4 + return v5 +} + +; VCode: +; block0: +; ldr x0, [x0, w1, UXTW #3] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr x0, [x0, w1, uxtw #3] +; ret + +function %load_not_extend_scaled64(i64, i32) -> i64 { +block0(v0: i64, v1: i32): + v2 = ishl_imm v1, 3 + v3 = uextend.i64 v2 + v4 = iadd v0, v3 + v5 = load.i64 v4 + return v5 +} + +; VCode: +; block0: +; lsl w4, w1, #3 +; ldr x0, [x0, w4, UXTW] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; lsl w4, w1, #3 +; ldr x0, [x0, w4, uxtw] +; ret + +function %load_sextend_scaled8(i64, i32) -> i8 { +block0(v0: i64, v1: i32): + v2 = sextend.i64 v1 + v3 = ishl_imm v2, 0 + v4 = iadd v0, v3 + v5 = load.i8 v4 + return v5 +} + +; VCode: +; block0: +; ldrb w0, [x0, w1, SXTW #0] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrb w0, [x0, w1, sxtw #0] +; ret + +function %load_sextend_scaled16(i64, i32) -> i16 { +block0(v0: i64, v1: i32): + v2 = sextend.i64 v1 + v3 = ishl_imm v2, 1 + v4 = iadd v0, v3 + v5 = load.i16 v4 + return v5 +} + +; VCode: +; block0: +; ldrh w0, [x0, w1, SXTW #1] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldrh w0, [x0, w1, sxtw #1] +; ret + +function %load_sextend_scaled32(i64, i32) -> i32 { +block0(v0: i64, v1: i32): + v2 = sextend.i64 v1 + v3 = ishl_imm v2, 2 + v4 = iadd v0, v3 + v5 = load.i32 v4 + return v5 +} + +; VCode: +; block0: +; ldr w0, [x0, w1, SXTW #2] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr w0, [x0, w1, sxtw #2] +; ret + +function %load_sextend_scaled64(i64, i32) -> i64 { +block0(v0: i64, v1: i32): + v2 = sextend.i64 v1 + v3 = ishl_imm v2, 3 + v4 = iadd v0, v3 + v5 = load.i64 v4 + return v5 +} + +; VCode: +; block0: +; ldr x0, [x0, w1, SXTW #3] +; ret +; +; Disassembled: +; block0: ; offset 0x0 +; ldr x0, [x0, w1, sxtw #3] +; ret + From cd33a1b50e3039f110c8ffb3693cbe6dc20788cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Jourde?= Date: Fri, 1 Sep 2023 01:08:07 +0200 Subject: [PATCH 7/8] cranelift: Validate `iconst` ranges (#6850) * cranelift: Validate `iconst` ranges Add the following checks: `iconst.i8` immediate must be within 0 .. 2^8-1 `iconst.i16` immediate must be within 0 .. 2^16-1 `iconst.i32` immediate must be within 0 .. 2^32-1 Resolves #3059 * cranelift: Parse `iconst` according to its type Modifies the parser for textual CLIF so that V in `iconst.T V` is parsed according to T. Before this commit, something like `iconst.i32 0xffff_ffff_ffff` was valid because all `iconst` were parsed the same as an `iconst.i64`. Now the above example will throw an error. Also, a negative immediate as in `iconst.iN -X` is now converted to `2^N - X`. This commit also fixes some broken tests. * cranelift: Update tests to match new CLIF parser --- cranelift/codegen/src/legalizer/mod.rs | 14 ++- cranelift/codegen/src/verifier/mod.rs | 104 +++++++++++++++++- .../filetests/filetests/egraph/extends.clif | 2 +- .../filetests/isa/s390x/constants.clif | 8 +- .../isa/x64/simd-lane-access-compile.clif | 4 +- .../filetests/filetests/parser/tiny.clif | 2 +- cranelift/reader/src/parser.rs | 24 +++- cranelift/tests/bugpoint_test.clif | 2 +- cranelift/wasm/src/code_translator.rs | 4 +- cranelift/wasm/src/environ/dummy.rs | 12 +- 10 files changed, 153 insertions(+), 23 deletions(-) diff --git a/cranelift/codegen/src/legalizer/mod.rs b/cranelift/codegen/src/legalizer/mod.rs index 3b09ed1fc733..e186b153091d 100644 --- a/cranelift/codegen/src/legalizer/mod.rs +++ b/cranelift/codegen/src/legalizer/mod.rs @@ -16,7 +16,7 @@ use crate::cursor::{Cursor, FuncCursor}; use crate::flowgraph::ControlFlowGraph; use crate::ir::immediates::Imm64; -use crate::ir::types::{I128, I64}; +use crate::ir::types::{self, I128, I64}; use crate::ir::{self, InstBuilder, InstructionData, MemFlags, Value}; use crate::isa::TargetIsa; use crate::trace; @@ -38,7 +38,17 @@ fn imm_const(pos: &mut FuncCursor, arg: Value, imm: Imm64, is_signed: bool) -> V let imm = pos.ins().iconst(I64, imm); pos.ins().uextend(I128, imm) } - _ => pos.ins().iconst(ty.lane_type(), imm), + _ => { + let bits = imm.bits(); + let unsigned = match ty.lane_type() { + types::I8 => bits as u8 as i64, + types::I16 => bits as u16 as i64, + types::I32 => bits as u32 as i64, + types::I64 => bits, + _ => unreachable!(), + }; + pos.ins().iconst(ty.lane_type(), unsigned) + } } } diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 0927fe9f06f6..5eaecaa190fa 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -1659,6 +1659,39 @@ impl<'a> Verifier<'a> { } } + fn iconst_bounds(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> { + use crate::ir::instructions::InstructionData::UnaryImm; + + let inst_data = &self.func.dfg.insts[inst]; + if let UnaryImm { + opcode: Opcode::Iconst, + imm, + } = inst_data + { + let ctrl_typevar = self.func.dfg.ctrl_typevar(inst); + let bounds_mask = match ctrl_typevar { + types::I8 => u8::MAX.into(), + types::I16 => u16::MAX.into(), + types::I32 => u32::MAX.into(), + types::I64 => u64::MAX, + _ => unreachable!(), + }; + + let value = imm.bits() as u64; + if value & bounds_mask != value { + errors.fatal(( + inst, + self.context(inst), + "constant immediate is out of bounds", + )) + } else { + Ok(()) + } + } else { + Ok(()) + } + } + fn typecheck_function_signature(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { let params = self .func @@ -1735,6 +1768,7 @@ impl<'a> Verifier<'a> { self.instruction_integrity(inst, errors)?; self.typecheck(inst, errors)?; self.immediate_constraints(inst, errors)?; + self.iconst_bounds(inst, errors)?; } self.encodable_as_bb(block, errors)?; @@ -1755,7 +1789,7 @@ impl<'a> Verifier<'a> { mod tests { use super::{Verifier, VerifierError, VerifierErrors}; use crate::ir::instructions::{InstructionData, Opcode}; - use crate::ir::{types, AbiParam, Function}; + use crate::ir::{types, AbiParam, Function, Type}; use crate::settings; macro_rules! assert_err_with_msg { @@ -1812,6 +1846,74 @@ mod tests { assert_err_with_msg!(errors, "instruction format"); } + fn test_iconst_bounds(immediate: i64, ctrl_typevar: Type) -> VerifierErrors { + let mut func = Function::new(); + let block0 = func.dfg.make_block(); + func.layout.append_block(block0); + + let test_inst = func.dfg.make_inst(InstructionData::UnaryImm { + opcode: Opcode::Iconst, + imm: immediate.into(), + }); + + let end_inst = func.dfg.make_inst(InstructionData::MultiAry { + opcode: Opcode::Return, + args: Default::default(), + }); + + func.dfg.append_result(test_inst, ctrl_typevar); + func.layout.append_inst(test_inst, block0); + func.layout.append_inst(end_inst, block0); + + let flags = &settings::Flags::new(settings::builder()); + let verifier = Verifier::new(&func, flags.into()); + let mut errors = VerifierErrors::default(); + + let _ = verifier.run(&mut errors); + errors + } + + fn test_iconst_bounds_err(immediate: i64, ctrl_typevar: Type) { + assert_err_with_msg!( + test_iconst_bounds(immediate, ctrl_typevar), + "constant immediate is out of bounds" + ); + } + + fn test_iconst_bounds_ok(immediate: i64, ctrl_typevar: Type) { + assert!(test_iconst_bounds(immediate, ctrl_typevar).is_empty()); + } + + #[test] + fn negative_iconst_8() { + test_iconst_bounds_err(-10, types::I8); + } + + #[test] + fn negative_iconst_32() { + test_iconst_bounds_err(-1, types::I32); + } + + #[test] + fn large_iconst_8() { + test_iconst_bounds_err(1 + u8::MAX as i64, types::I8); + } + + #[test] + fn large_iconst_16() { + test_iconst_bounds_err(10 + u16::MAX as i64, types::I16); + } + + #[test] + fn valid_iconst_8() { + test_iconst_bounds_ok(10, types::I8); + } + + #[test] + fn valid_iconst_32() { + test_iconst_bounds_ok(u32::MAX as i64, types::I32); + } + #[test] fn test_function_invalid_param() { let mut func = Function::new(); diff --git a/cranelift/filetests/filetests/egraph/extends.clif b/cranelift/filetests/filetests/egraph/extends.clif index 03454ca6d0d1..7ebb2642f572 100644 --- a/cranelift/filetests/filetests/egraph/extends.clif +++ b/cranelift/filetests/filetests/egraph/extends.clif @@ -4,7 +4,7 @@ target x86_64 function %f1() -> i64 { block0: - v0 = iconst.i32 0xffff_ffff_9876_5432 + v0 = iconst.i32 0x9876_5432 v1 = uextend.i64 v0 return v1 ; check: v2 = iconst.i64 0x9876_5432 diff --git a/cranelift/filetests/filetests/isa/s390x/constants.clif b/cranelift/filetests/filetests/isa/s390x/constants.clif index 4ae8d7f78567..24a4c1f3552c 100644 --- a/cranelift/filetests/filetests/isa/s390x/constants.clif +++ b/cranelift/filetests/filetests/isa/s390x/constants.clif @@ -9,12 +9,12 @@ block0: ; VCode: ; block0: -; lhi %r2, -1 +; lhi %r2, 255 ; br %r14 ; ; Disassembled: ; block0: ; offset 0x0 -; lhi %r2, -1 +; lhi %r2, 0xff ; br %r14 function %f() -> i16 { @@ -189,11 +189,11 @@ block0: ; VCode: ; block0: -; lhi %r2, -1 +; iilf %r2, 4294967295 ; br %r14 ; ; Disassembled: ; block0: ; offset 0x0 -; lhi %r2, -1 +; iilf %r2, 0xffffffff ; br %r14 diff --git a/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif b/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif index 27f212ff2c7d..3331c776586c 100644 --- a/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif +++ b/cranelift/filetests/filetests/isa/x64/simd-lane-access-compile.clif @@ -193,7 +193,7 @@ block0: ; pushq %rbp ; movq %rsp, %rbp ; block0: -; movl $-1, %ecx +; movl $65535, %ecx ; movd %ecx, %xmm1 ; pshuflw $0, %xmm1, %xmm3 ; pshufd $0, %xmm3, %xmm0 @@ -206,7 +206,7 @@ block0: ; pushq %rbp ; movq %rsp, %rbp ; block1: ; offset 0x4 -; movl $0xffffffff, %ecx +; movl $0xffff, %ecx ; movd %ecx, %xmm1 ; pshuflw $0, %xmm1, %xmm3 ; pshufd $0, %xmm3, %xmm0 diff --git a/cranelift/filetests/filetests/parser/tiny.clif b/cranelift/filetests/filetests/parser/tiny.clif index 9b73213ab1cf..90a8f28aa309 100644 --- a/cranelift/filetests/filetests/parser/tiny.clif +++ b/cranelift/filetests/filetests/parser/tiny.clif @@ -36,7 +36,7 @@ block0: } ; sameln: function %bvalues() fast { ; nextln: block0: -; nextln: v0 = iconst.i32 -1 +; nextln: v0 = iconst.i32 0xffff_ffff ; nextln: v1 = iconst.i8 0 ; nextln: v2 = sextend.i32 v1 ; nextln: v3 = bxor v0, v2 diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 7beaedb8e95e..28b6ae18600c 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -12,6 +12,7 @@ use cranelift_codegen::entity::{EntityRef, PrimaryMap}; use cranelift_codegen::ir::entities::{AnyEntity, DynamicType}; use cranelift_codegen::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64}; use cranelift_codegen::ir::instructions::{InstructionData, InstructionFormat, VariableArgs}; +use cranelift_codegen::ir::types; use cranelift_codegen::ir::types::INVALID; use cranelift_codegen::ir::types::*; use cranelift_codegen::ir::{self, UserExternalNameRef}; @@ -2456,10 +2457,25 @@ impl<'a> Parser<'a> { opcode, arg: self.match_value("expected SSA value operand")?, }, - InstructionFormat::UnaryImm => InstructionData::UnaryImm { - opcode, - imm: self.match_imm64("expected immediate integer operand")?, - }, + InstructionFormat::UnaryImm => { + let msg = |bits| format!("expected immediate {bits}-bit integer operand"); + let unsigned = match explicit_control_type { + Some(types::I8) => self.match_imm8(&msg(8))? as u8 as i64, + Some(types::I16) => self.match_imm16(&msg(16))? as u16 as i64, + Some(types::I32) => self.match_imm32(&msg(32))? as u32 as i64, + Some(types::I64) => self.match_imm64(&msg(64))?.bits(), + _ => { + return err!( + self.loc, + "expected one of the following type: i8, i16, i32 or i64" + ) + } + }; + InstructionData::UnaryImm { + opcode, + imm: Imm64::new(unsigned), + } + } InstructionFormat::UnaryIeee32 => InstructionData::UnaryIeee32 { opcode, imm: self.match_ieee32("expected immediate 32-bit float operand")?, diff --git a/cranelift/tests/bugpoint_test.clif b/cranelift/tests/bugpoint_test.clif index f52264543bc5..47bcb734adba 100644 --- a/cranelift/tests/bugpoint_test.clif +++ b/cranelift/tests/bugpoint_test.clif @@ -910,7 +910,7 @@ block51: v428 = iadd_imm.i64 v28, 8 v429 = load.i16 v428 v435 -> v429 - v430 = iconst.i16 0xffff_ffff_ffff_8000 + v430 = iconst.i16 0x8000 v431 = icmp eq v429, v430 v433 = uextend.i32 v431 brif v433, block154, block52 diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index ac9b28b893e3..d683a420d667 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -928,7 +928,9 @@ pub fn translate_operator( translate_store(memarg, ir::Opcode::Store, builder, state, environ)?; } /****************************** Nullary Operators ************************************/ - Operator::I32Const { value } => state.push1(builder.ins().iconst(I32, i64::from(*value))), + Operator::I32Const { value } => { + state.push1(builder.ins().iconst(I32, *value as u32 as i64)) + } Operator::I64Const { value } => state.push1(builder.ins().iconst(I64, *value)), Operator::F32Const { value } => { state.push1(builder.ins().f32const(f32_translation(*value))); diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs index 86622c98c41e..95f9fecece62 100644 --- a/cranelift/wasm/src/environ/dummy.rs +++ b/cranelift/wasm/src/environ/dummy.rs @@ -509,7 +509,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ _heap: Heap, _val: ir::Value, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_memory_size( @@ -518,7 +518,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ _index: MemoryIndex, _heap: Heap, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_memory_copy( @@ -570,7 +570,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ _index: TableIndex, _table: ir::Table, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_table_grow( @@ -581,7 +581,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ _delta: ir::Value, _init_value: ir::Value, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_table_get( @@ -660,7 +660,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ mut pos: FuncCursor, _global_index: GlobalIndex, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_custom_global_set( @@ -681,7 +681,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ _expected: ir::Value, _timeout: ir::Value, ) -> WasmResult { - Ok(pos.ins().iconst(I32, -1)) + Ok(pos.ins().iconst(I32, -1i32 as u32 as i64)) } fn translate_atomic_notify( From f62baeedc1abc701beb1f6b6ef6d9302500da414 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 1 Sep 2023 05:11:06 +0200 Subject: [PATCH 8/8] Some minor fixes and features for WASI and sockets (#6948) * Use `command::add_to_linker` in tests to reduce the number of times all the `add_to_linker` are listed. * Add all `wasi:sockets` interfaces currently implemented to both the sync and async `command` functions (this enables all the interfaces in the CLI for example). * Use `tokio::net::TcpStream::try_io` whenever I/O is performed on a socket, ensuring that readable/writable flags are set/cleared appropriately (otherwise once readable a socket is infinitely readable). * Add a `with_ambient_tokio_runtime` helper function to use when creating a `tokio::net::TcpStream` since otherwise it panics due to a lack of active runtime in a synchronous context. * Add `WouldBlock` handling to return a 0-length read. * Add an `--inherit-network` CLI flag to enable basic usage of sockets in the CLI. This will conflict a small amount with #6877 but should be easy to resolve, and otherwise this targets different usability points/issues than that PR. --- crates/test-programs/tests/reactor.rs | 17 +------ crates/test-programs/tests/wasi-sockets.rs | 19 +------- crates/wasi/src/preview2/command.rs | 8 +++- crates/wasi/src/preview2/host/tcp.rs | 10 ++-- crates/wasi/src/preview2/mod.rs | 10 ++++ crates/wasi/src/preview2/tcp.rs | 54 +++++++++++++--------- src/commands/run.rs | 9 ++++ 7 files changed, 65 insertions(+), 62 deletions(-) diff --git a/crates/test-programs/tests/reactor.rs b/crates/test-programs/tests/reactor.rs index 7dff956b6179..521c67af8c9e 100644 --- a/crates/test-programs/tests/reactor.rs +++ b/crates/test-programs/tests/reactor.rs @@ -71,24 +71,9 @@ async fn instantiate( wasi_ctx: ReactorCtx, ) -> Result<(Store, TestReactor)> { let mut linker = Linker::new(&ENGINE); - - // All of the imports available to the world are provided by the wasi-common crate: - preview2::bindings::filesystem::types::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::filesystem::preopens::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::io::streams::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::environment::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::exit::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stdin::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stdout::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stderr::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_input::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_output::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stdin::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stdout::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stderr::add_to_linker(&mut linker, |x| x)?; + preview2::command::add_to_linker(&mut linker)?; let mut store = Store::new(&ENGINE, wasi_ctx); - let (testreactor, _instance) = TestReactor::instantiate_async(&mut store, &component, &linker).await?; Ok((store, testreactor)) diff --git a/crates/test-programs/tests/wasi-sockets.rs b/crates/test-programs/tests/wasi-sockets.rs index ff119108fc0c..c16e9d4fadf7 100644 --- a/crates/test-programs/tests/wasi-sockets.rs +++ b/crates/test-programs/tests/wasi-sockets.rs @@ -45,24 +45,7 @@ async fn run(name: &str) -> anyhow::Result<()> { let component = get_component(name); let mut linker = Linker::new(&ENGINE); - preview2::bindings::io::streams::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::poll::poll::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::exit::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stdin::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stdout::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::stderr::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_input::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_output::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stdin::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stdout::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::terminal_stderr::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::cli::environment::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::filesystem::types::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::filesystem::preopens::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::sockets::tcp::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::sockets::tcp_create_socket::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::sockets::network::add_to_linker(&mut linker, |x| x)?; - preview2::bindings::sockets::instance_network::add_to_linker(&mut linker, |x| x)?; + preview2::command::add_to_linker(&mut linker)?; // Create our wasi context. let mut table = Table::new(); diff --git a/crates/wasi/src/preview2/command.rs b/crates/wasi/src/preview2/command.rs index ae01eeb0219d..1ba80b93923c 100644 --- a/crates/wasi/src/preview2/command.rs +++ b/crates/wasi/src/preview2/command.rs @@ -37,7 +37,6 @@ pub fn add_to_linker(l: &mut wasmtime::component::Linker) -> any crate::preview2::bindings::clocks::timezone::add_to_linker(l, |t| t)?; crate::preview2::bindings::filesystem::types::add_to_linker(l, |t| t)?; crate::preview2::bindings::filesystem::preopens::add_to_linker(l, |t| t)?; - crate::preview2::bindings::sockets::tcp::add_to_linker(l, |t| t)?; crate::preview2::bindings::poll::poll::add_to_linker(l, |t| t)?; crate::preview2::bindings::io::streams::add_to_linker(l, |t| t)?; crate::preview2::bindings::random::random::add_to_linker(l, |t| t)?; @@ -51,6 +50,10 @@ pub fn add_to_linker(l: &mut wasmtime::component::Linker) -> any crate::preview2::bindings::cli::terminal_stdin::add_to_linker(l, |t| t)?; crate::preview2::bindings::cli::terminal_stdout::add_to_linker(l, |t| t)?; crate::preview2::bindings::cli::terminal_stderr::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::tcp::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::tcp_create_socket::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::instance_network::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::network::add_to_linker(l, |t| t)?; Ok(()) } @@ -110,6 +113,9 @@ pub mod sync { crate::preview2::bindings::cli::terminal_stdout::add_to_linker(l, |t| t)?; crate::preview2::bindings::cli::terminal_stderr::add_to_linker(l, |t| t)?; crate::preview2::bindings::sockets::tcp::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::tcp_create_socket::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::instance_network::add_to_linker(l, |t| t)?; + crate::preview2::bindings::sockets::network::add_to_linker(l, |t| t)?; Ok(()) } } diff --git a/crates/wasi/src/preview2/host/tcp.rs b/crates/wasi/src/preview2/host/tcp.rs index 2ec61f26b95d..acdca7ce018c 100644 --- a/crates/wasi/src/preview2/host/tcp.rs +++ b/crates/wasi/src/preview2/host/tcp.rs @@ -201,10 +201,12 @@ impl tcp::Host for T { } // Do the OS accept call. - let (connection, _addr) = socket - .tcp_socket() - .as_socketlike_view::() - .accept_with(Blocking::No)?; + let tcp_socket = socket.tcp_socket(); + let (connection, _addr) = tcp_socket.try_io(Interest::READABLE, || { + tcp_socket + .as_socketlike_view::() + .accept_with(Blocking::No) + })?; let tcp_socket = HostTcpSocket::from_tcp_stream(connection)?; let input_clone = tcp_socket.clone_inner(); diff --git a/crates/wasi/src/preview2/mod.rs b/crates/wasi/src/preview2/mod.rs index 2fcf78a7efe6..170465ba38ef 100644 --- a/crates/wasi/src/preview2/mod.rs +++ b/crates/wasi/src/preview2/mod.rs @@ -179,3 +179,13 @@ pub(crate) fn in_tokio(f: F) -> F::Output { } } } + +fn with_ambient_tokio_runtime(f: impl FnOnce() -> R) -> R { + match tokio::runtime::Handle::try_current() { + Ok(_) => f(), + Err(_) => { + let _enter = RUNTIME.enter(); + f() + } + } +} diff --git a/crates/wasi/src/preview2/tcp.rs b/crates/wasi/src/preview2/tcp.rs index 3821f4e55a1d..a563a6e09b3b 100644 --- a/crates/wasi/src/preview2/tcp.rs +++ b/crates/wasi/src/preview2/tcp.rs @@ -7,6 +7,7 @@ use io_lifetimes::AsSocketlike; use std::io; use std::sync::Arc; use system_interface::io::IoExt; +use tokio::io::Interest; /// The state of a TCP socket. /// @@ -64,15 +65,17 @@ impl HostTcpSocket { // by our async implementation. let tcp_socket = TcpListener::new(family, Blocking::No)?; - let tcp_socket = unsafe { - tokio::net::TcpStream::try_from(std::net::TcpStream::from_raw_socketlike( - tcp_socket.into_raw_socketlike(), - )) - .unwrap() - }; + let std_socket = + unsafe { std::net::TcpStream::from_raw_socketlike(tcp_socket.into_raw_socketlike()) }; + + let tokio_tcp_socket = crate::preview2::with_ambient_tokio_runtime(|| { + tokio::net::TcpStream::try_from(std_socket).unwrap() + }); Ok(Self { - inner: Arc::new(HostTcpSocketInner { tcp_socket }), + inner: Arc::new(HostTcpSocketInner { + tcp_socket: tokio_tcp_socket, + }), tcp_state: HostTcpState::Default, }) } @@ -84,15 +87,16 @@ impl HostTcpSocket { let fd = rustix::fd::OwnedFd::from(tcp_socket); let tcp_socket = TcpListener::from(fd); - let tcp_socket = unsafe { - tokio::net::TcpStream::try_from(std::net::TcpStream::from_raw_socketlike( - tcp_socket.into_raw_socketlike(), - )) - .unwrap() - }; + let std_tcp_socket = + unsafe { std::net::TcpStream::from_raw_socketlike(tcp_socket.into_raw_socketlike()) }; + let tokio_tcp_socket = crate::preview2::with_ambient_tokio_runtime(|| { + tokio::net::TcpStream::try_from(std_tcp_socket).unwrap() + }); Ok(Self { - inner: Arc::new(HostTcpSocketInner { tcp_socket }), + inner: Arc::new(HostTcpSocketInner { + tcp_socket: tokio_tcp_socket, + }), tcp_state: HostTcpState::Default, }) } @@ -121,10 +125,10 @@ impl HostInputStream for Arc { return Ok((Bytes::new(), StreamState::Open)); } let mut buf = BytesMut::zeroed(size); - let r = self - .tcp_socket() - .as_socketlike_view::() - .read(&mut buf); + let socket = self.tcp_socket(); + let r = socket.try_io(Interest::READABLE, || { + socket.as_socketlike_view::().read(&mut buf) + }); let (n, state) = read_result(r)?; buf.truncate(n); Ok((buf.freeze(), state)) @@ -142,10 +146,10 @@ impl HostOutputStream for Arc { if buf.is_empty() { return Ok((0, StreamState::Open)); } - let r = self - .tcp_socket - .as_socketlike_view::() - .write(buf.as_ref()); + let socket = self.tcp_socket(); + let r = socket.try_io(Interest::WRITABLE, || { + socket.as_socketlike_view::().write(buf.as_ref()) + }); let (n, state) = write_result(r)?; Ok((n, state)) } @@ -186,7 +190,11 @@ pub(crate) fn read_result(r: io::Result) -> io::Result<(usize, StreamStat match r { Ok(0) => Ok((0, StreamState::Closed)), Ok(n) => Ok((n, StreamState::Open)), - Err(e) if e.kind() == io::ErrorKind::Interrupted => Ok((0, StreamState::Open)), + Err(e) + if e.kind() == io::ErrorKind::Interrupted || e.kind() == io::ErrorKind::WouldBlock => + { + Ok((0, StreamState::Open)) + } Err(e) => Err(e), } } diff --git a/src/commands/run.rs b/src/commands/run.rs index 398b9bf686dc..a93eaecd0227 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -293,6 +293,11 @@ pub struct RunCommand { /// removed. For now this is primarily here for testing. #[clap(long)] preview2: bool, + + /// Flag for WASI preview2 to inherit the host's network within the guest so + /// it has full access to all addresses/ports/etc. + #[clap(long)] + inherit_network: bool, } #[derive(Clone)] @@ -1061,6 +1066,10 @@ impl RunCommand { ); } + if self.inherit_network { + builder.inherit_network(ambient_authority()); + } + let data = store.data_mut(); let table = Arc::get_mut(&mut data.preview2_table).unwrap(); let ctx = builder.build(table)?;