winch: Refactoring wasmtime compiler integration pieces to share more between Cranelift and Winch (#5944)

* Enable the native target by default in winch

Match cranelift-codegen's build script where if no architecture is
explicitly enabled then the host architecture is implicitly enabled.

* Refactor Cranelift's ISA builder to share more with Winch

This commit refactors the `Builder` type to have a type parameter
representing the finished ISA with Cranelift and Winch having their own
typedefs for `Builder` to represent their own builders. The intention is
to use this shared functionality to produce more shared code between the
two codegen backends.

* Moving compiler shared components to a separate crate

* Restore native flag inference in compiler building

This fixes an oversight from the previous commits to use
`cranelift-native` to infer flags for the native host when using default
settings with Wasmtime.

* Move `Compiler::page_size_align` into wasmtime-environ

The `cranelift-codegen` crate doesn't need this and winch wants the same
implementation, so shuffle it around so everyone has access to it.

* Fill out `Compiler::{flags, isa_flags}` for Winch

These are easy enough to plumb through with some shared code for
Wasmtime.

* Plumb the `is_branch_protection_enabled` flag for Winch

Just forwarding an isa-specific setting accessor.

* Moving executable creation to shared compiler crate

* Adding builder back in and removing from shared crate

* Refactoring the shared pieces for the `CompilerBuilder`

I decided to move a couple things around from Alex's initial changes.
Instead of having the shared builder do everything, I went back to
having each compiler have a distinct builder implementation. I
refactored most of the flag setting logic into a single shared location,
so we can still reduce the amount of code duplication.

With them being separate, we don't need to maintain things like
`LinkOpts` which Winch doesn't currently use. We also have an avenue to
error when certain flags are sent to Winch if we don't support them. I'm
hoping this will make things more maintainable as we build out Winch.

I'm still unsure about keeping everything shared in a single crate
(`cranelift_shared`). It's starting to feel like this crate is doing too
much, which makes it difficult to name. There does seem to be a need for
two distinct abstraction: creating the final executable and the handling
of shared/ISA flags when building the compiler. I could make them into
two separate crates, but there doesn't seem to be enough there yet to
justify it.

* Documentation updates, and renaming the finish method

* Adding back in a default temporarily to pass tests, and removing some unused imports

* Fixing winch tests with wrong method name

* Removing unused imports from codegen shared crate

* Apply documentation formatting updates

Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>

* Adding back in cranelift_native flag inferring

* Adding new shared crate to publish list

* Adding write feature to pass cargo check

---------

Co-authored-by: Alex Crichton <alex@alexcrichton.com>
Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>

cranelift/codegen/meta/src/gen_settings.rs

@@ -21,7 +21,7 @@ pub(crate) enum ParentGroup {
 fn gen_constructor(group: &SettingGroup, parent: ParentGroup, fmt: &mut Formatter) {
     let args = match parent {
         ParentGroup::None => "builder: Builder",
-        ParentGroup::Shared => "shared: &settings::Flags, builder: Builder",
+        ParentGroup::Shared => "shared: &settings::Flags, builder: &Builder",
     };
     fmtln!(fmt, "impl Flags {");
     fmt.indent(|fmt| {

cranelift/codegen/src/isa/mod.rs

@@ -146,16 +146,34 @@ impl fmt::Display for LookupError {
 /// The type of a polymorphic TargetISA object which is 'static.
 pub type OwnedTargetIsa = Arc<dyn TargetIsa>;
 
+/// Type alias of `IsaBuilder` used for building Cranelift's ISAs.
+pub type Builder = IsaBuilder<CodegenResult<OwnedTargetIsa>>;
+
 /// Builder for a `TargetIsa`.
 /// Modify the ISA-specific settings before creating the `TargetIsa` trait object with `finish`.
 #[derive(Clone)]
-pub struct Builder {
+pub struct IsaBuilder<T> {
     triple: Triple,
     setup: settings::Builder,
-    constructor: fn(Triple, settings::Flags, settings::Builder) -> CodegenResult<OwnedTargetIsa>,
+    constructor: fn(Triple, settings::Flags, &settings::Builder) -> T,
 }
 
-impl Builder {
+impl<T> IsaBuilder<T> {
+    /// Creates a new ISA-builder from its components, namely the `triple` for
+    /// the ISA, the ISA-specific settings builder, and a final constructor
+    /// function to generate the ISA from its components.
+    pub fn new(
+        triple: Triple,
+        setup: settings::Builder,
+        constructor: fn(Triple, settings::Flags, &settings::Builder) -> T,
+    ) -> Self {
+        IsaBuilder {
+            triple,
+            setup,
+            constructor,
+        }
+    }
+
     /// Gets the triple for the builder.
     pub fn triple(&self) -> &Triple {
         &self.triple
@@ -172,12 +190,12 @@ impl Builder {
     /// flags are inconsistent or incompatible: for example, some
     /// platform-independent features, like general SIMD support, may
     /// need certain ISA extensions to be enabled.
-    pub fn finish(self, shared_flags: settings::Flags) -> CodegenResult<OwnedTargetIsa> {
-        (self.constructor)(self.triple, shared_flags, self.setup)
+    pub fn finish(&self, shared_flags: settings::Flags) -> T {
+        (self.constructor)(self.triple.clone(), shared_flags, &self.setup)
     }
 }
 
-impl settings::Configurable for Builder {
+impl<T> settings::Configurable for IsaBuilder<T> {
     fn set(&mut self, name: &str, value: &str) -> SetResult<()> {
         self.setup.set(name, value)
     }
@@ -339,24 +357,6 @@ impl<'a> dyn TargetIsa + 'a {
         }
     }
 
-    /// Returns the code (text) section alignment for this ISA.
-    pub fn code_section_alignment(&self) -> u64 {
-        use target_lexicon::*;
-        match (self.triple().operating_system, self.triple().architecture) {
-            (
-                OperatingSystem::MacOSX { .. }
-                | OperatingSystem::Darwin
-                | OperatingSystem::Ios
-                | OperatingSystem::Tvos,
-                Architecture::Aarch64(..),
-            ) => 0x4000,
-            // 64 KB is the maximal page size (i.e. memory translation granule size)
-            // supported by the architecture and is used on some platforms.
-            (_, Architecture::Aarch64(..)) => 0x10000,
-            _ => 0x1000,
-        }
-    }
-
     /// Returns the minimum symbol alignment for this ISA.
     pub fn symbol_alignment(&self) -> u64 {
         match self.triple().architecture {

cranelift/codegen/src/isa/riscv64/inst/emit_tests.rs

@@ -2095,7 +2095,7 @@ fn make_test_flags() -> (settings::Flags, super::super::riscv_settings::Flags) {
     let b = settings::builder();
     let flags = settings::Flags::new(b.clone());
     let b2 = super::super::riscv_settings::builder();
-    let isa_flags = super::super::riscv_settings::Flags::new(&flags, b2);
+    let isa_flags = super::super::riscv_settings::Flags::new(&flags, &b2);
     (flags, isa_flags)
 }
 

cranelift/codegen/src/isa/s390x/inst/emit_tests.rs

@@ -13362,7 +13362,7 @@ fn test_s390x_binemit() {
     use crate::settings::Configurable;
     let mut isa_flag_builder = s390x_settings::builder();
     isa_flag_builder.enable("arch13").unwrap();
-    let isa_flags = s390x_settings::Flags::new(&flags, isa_flag_builder);
+    let isa_flags = s390x_settings::Flags::new(&flags, &isa_flag_builder);
 
     let emit_info = EmitInfo::new(isa_flags);
     for (insn, expected_encoding, expected_printing) in insns {

cranelift/codegen/src/isa/x64/inst/emit_tests.rs

@@ -5155,7 +5155,7 @@ fn test_x64_emit() {
     isa_flag_builder.enable("has_avx512f").unwrap();
     isa_flag_builder.enable("has_avx512vbmi").unwrap();
     isa_flag_builder.enable("has_avx512vl").unwrap();
-    let isa_flags = x64::settings::Flags::new(&flags, isa_flag_builder);
+    let isa_flags = x64::settings::Flags::new(&flags, &isa_flag_builder);
 
     let emit_info = EmitInfo::new(flags, isa_flags);
     for (insn, expected_encoding, expected_printing) in insns {

cranelift/codegen/src/isa/x64/mod.rs

@@ -212,7 +212,7 @@ pub(crate) fn isa_builder(triple: Triple) -> IsaBuilder {
 fn isa_constructor(
     triple: Triple,
     shared_flags: Flags,
-    builder: shared_settings::Builder,
+    builder: &shared_settings::Builder,
 ) -> CodegenResult<OwnedTargetIsa> {
     let isa_flags = x64_settings::Flags::new(&shared_flags, builder);
 

cranelift/codegen/src/lib.rs

@@ -88,7 +88,9 @@ pub mod verifier;
 pub mod write;
 
 pub use crate::entity::packed_option;
-pub use crate::machinst::buffer::{MachCallSite, MachReloc, MachSrcLoc, MachStackMap, MachTrap};
+pub use crate::machinst::buffer::{
+    MachCallSite, MachReloc, MachSrcLoc, MachStackMap, MachTextSectionBuilder, MachTrap,
+};
 pub use crate::machinst::{
     CompiledCode, Final, MachBuffer, MachBufferFinalized, MachInst, MachInstEmit, Reg,
     TextSectionBuilder, Writable,

cranelift/codegen/src/machinst/buffer.rs

@@ -1610,6 +1610,8 @@ pub struct MachTextSectionBuilder<I: VCodeInst> {
 }
 
 impl<I: VCodeInst> MachTextSectionBuilder<I> {
+    /// Creates a new text section builder which will have `num_funcs` functions
+    /// pushed into it.
     pub fn new(num_funcs: usize) -> MachTextSectionBuilder<I> {
         let mut buf = MachBuffer::new();
         buf.reserve_labels_for_blocks(num_funcs);

cranelift/codegen/src/settings.rs

@@ -161,9 +161,9 @@ impl Builder {
     }
 
     /// Extract contents of builder once everything is configured.
-    pub fn state_for(self, name: &str) -> Box<[u8]> {
+    pub fn state_for(&self, name: &str) -> &[u8] {
         assert_eq!(name, self.template.name);
-        self.bytes
+        &self.bytes
     }
 
     /// Iterates the available settings in the builder.