Skip to content

Commit 6c9ef22

Browse files
committed
wasmparser: merge VisitSimdOperator back into VisitOperator
The `simd` feature remains in place, but now does not affect the API in this way. Rather parts of implementation pertaining to SIMD in wasmparser get disabled, reducing the compilation time and artifact size impact slightly, though less than in the original approach. The results on my machine are roughly: * origin/main simd: 4.26s producing 30M .rlib * origin/main !simd: 3.49s producing 23M .rlib * this simd: 4.19s producing a 26M .rlib * this !simd: 3.96s producing a 25M .rlib I'm not quite sure why the rlib size decreased so significantly when the SIMD feature is enabled (I guess less inlining is occurring?) and there is indeed a .5s hit on the compilation time to this PR. Are those half a second worth a clunkier API? I'm not convinced :)
1 parent b0d1dd7 commit 6c9ef22

File tree

10 files changed

+767
-926
lines changed

10 files changed

+767
-926
lines changed

crates/wasmparser/benches/benchmark.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use once_cell::unsync::Lazy;
44
use std::fs;
55
use std::path::Path;
66
use std::path::PathBuf;
7-
use wasmparser::VisitSimdOperator;
87
use wasmparser::{DataKind, ElementKind, Parser, Payload, Validator, VisitOperator, WasmFeatures};
98

109
/// A benchmark input.
@@ -375,15 +374,5 @@ macro_rules! define_visit_operator {
375374
#[allow(unused_variables)]
376375
impl<'a> VisitOperator<'a> for NopVisit {
377376
type Output = ();
378-
379-
fn simd_visitor(&mut self) -> Option<&mut dyn VisitSimdOperator<'a, Output = Self::Output>> {
380-
Some(self)
381-
}
382-
383-
wasmparser::for_each_visit_operator!(define_visit_operator);
384-
}
385-
386-
#[allow(unused_variables)]
387-
impl<'a> VisitSimdOperator<'a> for NopVisit {
388-
wasmparser::for_each_visit_simd_operator!(define_visit_operator);
377+
wasmparser::for_each_operator!(define_visit_operator);
389378
}

crates/wasmparser/src/lib.rs

Lines changed: 14 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -827,21 +827,14 @@ macro_rules! define_for_each_non_simd_operator {
827827
$m! { $($t)* }
828828
}
829829
}
830-
831-
// When simd is disabled then this macro is additionally the
832-
// `for_each_operator!` macro implementation
833-
#[cfg(not(feature = "simd"))]
834-
#[doc(hidden)]
835-
pub use _for_each_visit_operator_impl as _for_each_operator_impl;
836830
};
837831
}
838832
_for_each_operator_group!(define_for_each_non_simd_operator);
839833

840834
/// When the simd feature is enabled then `_for_each_operator_impl` is defined
841835
/// to be the same as the above `define_for_each_non_simd_operator` macro except
842836
/// with all proposals thrown in.
843-
#[cfg(feature = "simd")]
844-
macro_rules! define_for_each_operator_impl_with_simd {
837+
macro_rules! define_for_each_operator_impl {
845838
(
846839
$(
847840
@$proposal:ident {
@@ -864,14 +857,12 @@ macro_rules! define_for_each_operator_impl_with_simd {
864857
}
865858
};
866859
}
867-
#[cfg(feature = "simd")]
868-
_for_each_operator_group!(define_for_each_operator_impl_with_simd);
860+
_for_each_operator_group!(define_for_each_operator_impl);
869861

870862
/// Helper macro to define the `_for_each_simd_operator_impl` macro.
871863
///
872864
/// This is basically the same as `define_for_each_non_simd_operator` above
873865
/// except that it's filtering on different proposals.
874-
#[cfg(feature = "simd")]
875866
macro_rules! define_for_each_simd_operator {
876867
// Switch to "tt muncher" mode
877868
(@ $($t:tt)*) => {define_for_each_simd_operator!(filter [] @ $($t)*);};
@@ -924,7 +915,7 @@ macro_rules! define_for_each_simd_operator {
924915
}
925916
};
926917
}
927-
#[cfg(feature = "simd")]
918+
928919
_for_each_operator_group!(define_for_each_simd_operator);
929920

930921
/// Used to implement routines for the [`Operator`] enum.
@@ -1085,10 +1076,10 @@ pub use _for_each_operator_impl as for_each_operator;
10851076
/// - `@stack_switching`: [Wasm `stack-switching` proposal]
10861077
/// - `@wide_arithmetic`: [Wasm `wide-arithmetic` proposal]
10871078
///
1088-
/// Note that this macro does not iterate over the SIMD-related proposals. Those
1089-
/// are provided in [`VisitSimdOperator`] and [`for_each_visit_simd_operator`].
1090-
/// This macro only handles non-SIMD related operators and so users wanting to
1091-
/// handle the SIMD-class of operators need to use that trait/macro as well.
1079+
/// Note that this macro does not iterate over the SIMD-related proposals. To include SIMD
1080+
/// instructions use [`for_each_visit_simd_operator`] or [`for_each_operator`] instead. This macro
1081+
/// only handles non-SIMD related operators and so users wanting to handle the SIMD-class of
1082+
/// operators need to use that trait/macro as well.
10921083
///
10931084
/// [Wasm `exception-handling` proposal]:
10941085
/// https://github.com/WebAssembly/exception-handling
@@ -1218,7 +1209,7 @@ pub use _for_each_operator_impl as for_each_operator;
12181209
#[doc(inline)]
12191210
pub use _for_each_visit_operator_impl as for_each_visit_operator;
12201211

1221-
/// Used to implement the [`VisitSimdOperator`] trait.
1212+
/// Used to implement the methods for simd extension of `VisitOperator` trait.
12221213
///
12231214
/// The list of specializable Wasm proposals is as follows:
12241215
///
@@ -1234,22 +1225,12 @@ pub use _for_each_visit_operator_impl as for_each_visit_operator;
12341225
/// [Wasm `relaxed-simd` proposal]:
12351226
/// https://github.com/WebAssembly/relaxed-simd
12361227
///
1237-
/// [`VisitSimdOperator`]: crate::VisitSimdOperator
1238-
///
12391228
/// ```
12401229
/// # macro_rules! define_visit_operator {
12411230
/// # ($( @$proposal:ident $op:ident $({ $($arg:ident: $argty:ty),* })? => $visit:ident ($($ann:tt)*))*) => {
12421231
/// # $( fn $visit(&mut self $($(,$arg: $argty)*)?) {} )*
12431232
/// # }
12441233
/// # }
1245-
/// pub struct VisitAndDoNothing;
1246-
///
1247-
/// impl<'a> wasmparser::VisitOperator<'a> for VisitAndDoNothing {
1248-
/// type Output = ();
1249-
///
1250-
/// // implement all the visit methods ..
1251-
/// # wasmparser::for_each_visit_operator!(define_visit_operator);
1252-
/// }
12531234
///
12541235
/// macro_rules! define_visit_simd_operator {
12551236
/// // The outer layer of repetition represents how all operators are
@@ -1267,12 +1248,16 @@ pub use _for_each_visit_operator_impl as for_each_visit_operator;
12671248
/// )*
12681249
/// }
12691250
/// }
1251+
/// pub struct VisitAndDoNothing;
1252+
///
1253+
/// impl<'a> wasmparser::VisitOperator<'a> for VisitAndDoNothing {
1254+
/// type Output = ();
12701255
///
1271-
/// impl<'a> wasmparser::VisitSimdOperator<'a> for VisitAndDoNothing {
1256+
/// // implement all the visit methods ..
1257+
/// wasmparser::for_each_visit_operator!(define_visit_operator);
12721258
/// wasmparser::for_each_visit_simd_operator!(define_visit_simd_operator);
12731259
/// }
12741260
/// ```
1275-
#[cfg(feature = "simd")]
12761261
#[doc(inline)]
12771262
pub use _for_each_visit_simd_operator_impl as for_each_visit_simd_operator;
12781263

0 commit comments

Comments
 (0)