From 947177732a0d3d30638f38d6baa96da6808d4380 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 18 Apr 2018 20:59:42 -0700 Subject: [PATCH 1/6] Update to filecheck 0.3.0. --- cranelift/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cranelift/Cargo.toml b/cranelift/Cargo.toml index 021440c024..4961a6fef6 100644 --- a/cranelift/Cargo.toml +++ b/cranelift/Cargo.toml @@ -23,7 +23,7 @@ cretonne-module = { path = "lib/module", version = "0.5.1" } cretonne-faerie = { path = "lib/faerie", version = "0.5.1" } cretonne-simplejit = { path = "lib/simplejit", version = "0.5.1" } cretonne = { path = "lib/umbrella", version = "0.5.1" } -filecheck = "0.2.1" +filecheck = "0.3.0" docopt = "0.8.0" serde = "1.0.8" serde_derive = "1.0.8" From 07576d3ed07d558a72cdc7e85c88e3fab12a1986 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 18 Apr 2018 20:59:51 -0700 Subject: [PATCH 2/6] Make test-all.sh run unit tests in debug mode. It turns out that "cargo test --release" doesn't use `[profile.release]`; it uses `[profile.bench]` instead; see [here](https://doc.rust-lang.org/cargo/reference/manifest.html) for details. So the plan to run the tests in optimized mode but with debug-assertions enabled didn't actually work, and we had an actual failing unit test that was hidden because assertions were disabled. So, this makes test-all.sh just run the unit tests in debug mode, as is the norm for Rust packages, and fixes the buggy test. This also removes the `[profile.release]` override from the top-level Cargo.toml file too. We don't need it now that we're not running tests in release mode, and it had confused multiple people because it made Cretonne's in-tree builds different from how Cretonne is built when used as a dependency in other projects. --- cranelift/Cargo.toml | 8 -------- cranelift/test-all.sh | 12 +++++++----- lib/module/src/data_context.rs | 4 ++-- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/cranelift/Cargo.toml b/cranelift/Cargo.toml index 4961a6fef6..543bbcf8a0 100644 --- a/cranelift/Cargo.toml +++ b/cranelift/Cargo.toml @@ -31,11 +31,3 @@ tempdir = "0.3.5" term = "0.5.1" [workspace] - -# Enable debug assertions and parallel compilation when building cretonne-tools -# since they are for testing and development mostly. This doesn't affect the -# flags used to build the cretonne-* crates when used as a dependency. -[profile.release] -opt-level = 2 -debug-assertions = true -codegen-units = 4 diff --git a/cranelift/test-all.sh b/cranelift/test-all.sh index ba3faf70b0..82ec347dac 100755 --- a/cranelift/test-all.sh +++ b/cranelift/test-all.sh @@ -41,15 +41,17 @@ if [ -n "$needcheck" ]; then touch $tsfile || echo no target directory fi +# Make sure the code builds in release mode. +banner "Rust release build" +cargo build --release + # Make sure the code builds in debug mode. banner "Rust debug build" cargo build -# Make sure the code builds in release mode, and run the unit tests. We run -# these in release mode for speed, but note that the top-level Cargo.toml file -# does enable debug assertions in release builds. -banner "Rust release build and unit tests" -cargo test --all --release +# Run the tests. We run these in debug mode so that assertions are enabled. +banner "Rust unit tests" +cargo test --all # Make sure the documentation builds. banner "Rust documentation: $topdir/target/doc/cretonne/index.html" diff --git a/lib/module/src/data_context.rs b/lib/module/src/data_context.rs index 510b549e90..51eac88b79 100644 --- a/lib/module/src/data_context.rs +++ b/lib/module/src/data_context.rs @@ -144,7 +144,7 @@ mod tests { fn basic_data_context() { let mut data_ctx = DataContext::new(); { - let description = data_ctx.description(); + let description = &data_ctx.description; assert_eq!(description.writable, Writability::Readonly); assert_eq!(description.init, Init::Uninitialized); assert!(description.function_decls.is_empty()); @@ -177,7 +177,7 @@ mod tests { data_ctx.clear(); { - let description = data_ctx.description(); + let description = &data_ctx.description; assert_eq!(description.writable, Writability::Readonly); assert_eq!(description.init, Init::Uninitialized); assert!(description.function_decls.is_empty()); From 5bc0e0e188dbb1f15018305f066dbe088d0f79ca Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 19 Apr 2018 09:40:46 -0700 Subject: [PATCH 3/6] Add a comment advertising `NullTrapSink`. --- lib/codegen/src/binemit/memorysink.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/codegen/src/binemit/memorysink.rs b/lib/codegen/src/binemit/memorysink.rs index 6871dc0942..fa4aacc233 100644 --- a/lib/codegen/src/binemit/memorysink.rs +++ b/lib/codegen/src/binemit/memorysink.rs @@ -68,6 +68,9 @@ pub trait RelocSink { } /// A trait for receiving trap codes and offsets. +/// +/// If you don't need information about possible traps, you can use the +/// [`NullTrapSink`](binemit/trait.TrapSink.html) implementation. pub trait TrapSink { /// Add trap information for a specific offset. fn trap(&mut self, CodeOffset, SourceLoc, TrapCode); From cb37c25d3a3a8194c14fd67d30a1b27804bc4c92 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 19 Apr 2018 10:54:57 -0700 Subject: [PATCH 4/6] Introduce Builder and Product types to the Module workflow. This eliminates API confusion and surface area with respect to what state the `Backend` needs to be in at different points. Now, API users will construct a `Builder`, and pass it into the `Module` which uses it to constrct a `Backend`. The `Backend` instance only lives inside the `Module`. And when finished, the `Module` can return a `Product` back to the user providing any outputs it has. --- lib/faerie/src/backend.rs | 105 ++++++++++++++++++++++++----------- lib/faerie/src/lib.rs | 2 +- lib/module/src/backend.rs | 19 ++++++- lib/module/src/module.rs | 14 ++--- lib/simplejit/src/backend.rs | 96 ++++++++++++++++++++------------ lib/simplejit/src/lib.rs | 2 +- 6 files changed, 161 insertions(+), 77 deletions(-) diff --git a/lib/faerie/src/backend.rs b/lib/faerie/src/backend.rs index 37e94e29e0..55af088295 100644 --- a/lib/faerie/src/backend.rs +++ b/lib/faerie/src/backend.rs @@ -11,19 +11,18 @@ use failure::Error; use std::fs::File; use target; -pub struct FaerieCompiledFunction {} - -pub struct FaerieCompiledData {} - -/// A `FaerieBackend` implements `Backend` and emits ".o" files using the `faerie` library. -pub struct FaerieBackend { +/// A builder for `FaerieBackend`. +pub struct FaerieBuilder { isa: Box, - artifact: faerie::Artifact, + name: String, format: container::Format, + faerie_target: faerie::Target, } -impl FaerieBackend { - /// Create a new `FaerieBackend` using the given Cretonne target. +impl FaerieBuilder { + /// Create a new `FaerieBuilder` using the given Cretonne target, that + /// can be passed to + /// [`Module::new`](cretonne_module/struct.Module.html#method.new]. pub fn new( isa: Box, name: String, @@ -33,34 +32,27 @@ impl FaerieBackend { let faerie_target = target::translate(&*isa)?; Ok(Self { isa, - artifact: faerie::Artifact::new(faerie_target, name), + name, format, + faerie_target, }) } - - /// Return the name of the output file. This is the name passed into `new`. - pub fn name(&self) -> &str { - &self.artifact.name - } - - /// Call `emit` on the faerie `Artifact`, producing bytes in memory. - pub fn emit(&self) -> Result, Error> { - match self.format { - container::Format::ELF => self.artifact.emit::(), - container::Format::MachO => self.artifact.emit::(), - } - } - - /// Call `write` on the faerie `Artifact`, writing to a file. - pub fn write(&self, sink: File) -> Result<(), Error> { - match self.format { - container::Format::ELF => self.artifact.write::(sink), - container::Format::MachO => self.artifact.write::(sink), - } - } } +/// A `FaerieBackend` implements `Backend` and emits ".o" files using the `faerie` library. +pub struct FaerieBackend { + isa: Box, + artifact: faerie::Artifact, + format: container::Format, +} + +pub struct FaerieCompiledFunction {} + +pub struct FaerieCompiledData {} + impl Backend for FaerieBackend { + type Builder = FaerieBuilder; + type CompiledFunction = FaerieCompiledFunction; type CompiledData = FaerieCompiledData; @@ -69,6 +61,19 @@ impl Backend for FaerieBackend { type FinalizedFunction = (); type FinalizedData = (); + /// The returned value here provides functions for emitting object files + /// to memory and files. + type Product = FaerieProduct; + + /// Create a new `FaerieBackend` using the given Cretonne target. + fn new(builder: FaerieBuilder) -> Self { + Self { + isa: builder.isa, + artifact: faerie::Artifact::new(builder.faerie_target, builder.name), + format: builder.format, + } + } + fn isa(&self) -> &TargetIsa { &*self.isa } @@ -216,6 +221,44 @@ impl Backend for FaerieBackend { fn finalize_data(&mut self, _data: &FaerieCompiledData, _namespace: &ModuleNamespace) { // Nothing to do. } + + fn finish(self) -> FaerieProduct { + FaerieProduct { + artifact: self.artifact, + format: self.format, + } + } +} + +/// This is the output of `Module`'s +/// [`finish`](../cretonne_module/struct.Module.html#method.finish) function. +/// It provides functions for writing out the object file to memory or a file. +pub struct FaerieProduct { + artifact: faerie::Artifact, + format: container::Format, +} + +impl FaerieProduct { + /// Return the name of the output file. This is the name passed into `new`. + pub fn name(&self) -> &str { + &self.artifact.name + } + + /// Call `emit` on the faerie `Artifact`, producing bytes in memory. + pub fn emit(&self) -> Result, Error> { + match self.format { + container::Format::ELF => self.artifact.emit::(), + container::Format::MachO => self.artifact.emit::(), + } + } + + /// Call `write` on the faerie `Artifact`, writing to a file. + pub fn write(&self, sink: File) -> Result<(), Error> { + match self.format { + container::Format::ELF => self.artifact.write::(sink), + container::Format::MachO => self.artifact.write::(sink), + } + } } fn translate_function_linkage(linkage: Linkage) -> faerie::Decl { diff --git a/lib/faerie/src/lib.rs b/lib/faerie/src/lib.rs index 3cf007357c..3a6d0cad28 100644 --- a/lib/faerie/src/lib.rs +++ b/lib/faerie/src/lib.rs @@ -29,5 +29,5 @@ mod backend; mod container; mod target; -pub use backend::FaerieBackend; +pub use backend::{FaerieBuilder, FaerieBackend}; pub use container::Format; diff --git a/lib/module/src/backend.rs b/lib/module/src/backend.rs index 0d6a6c6672..108b159e85 100644 --- a/lib/module/src/backend.rs +++ b/lib/module/src/backend.rs @@ -14,6 +14,9 @@ pub trait Backend where Self: marker::Sized, { + /// A builder for constructing `Backend` instances. + type Builder; + /// The results of compiling a function. type CompiledFunction; @@ -21,13 +24,21 @@ where type CompiledData; /// The completed output artifact for a function, if this is meaningful for - /// the Backend. + /// the `Backend`. type FinalizedFunction; /// The completed output artifact for a data object, if this is meaningful for - /// the Backend. + /// the `Backend`. type FinalizedData; + /// This is an object returned by `Module`'s + /// [`finish`](struct.Module.html#method.finish) function, + /// if the `Backend` has a purpose for this. + type Product; + + /// Create a new `Backend` instance. + fn new(Self::Builder) -> Self; + /// Return the `TargetIsa` to compile for. fn isa(&self) -> &TargetIsa; @@ -94,4 +105,8 @@ where data: &Self::CompiledData, namespace: &ModuleNamespace, ) -> Self::FinalizedData; + + /// Consume this `Backend` and return a result. Some implementations may + /// provide additional functionality through this result. + fn finish(self) -> Self::Product; } diff --git a/lib/module/src/module.rs b/lib/module/src/module.rs index 79ef29eba3..b47189f1d9 100644 --- a/lib/module/src/module.rs +++ b/lib/module/src/module.rs @@ -236,14 +236,14 @@ where B: Backend, { /// Create a new `Module`. - pub fn new(backend: B) -> Self { + pub fn new(backend_builder: B::Builder) -> Self { Self { names: HashMap::new(), contents: ModuleContents { functions: PrimaryMap::new(), data_objects: PrimaryMap::new(), }, - backend, + backend: B::new(backend_builder), } } @@ -532,10 +532,10 @@ where } } - /// Consume the module and return its contained `Backend`. Some `Backend` - /// implementations have additional features not available through the - /// `Module` interface. - pub fn consume(self) -> B { - self.backend + /// Consume the module and return the resulting `Product`. Some `Backend` + /// implementations may provide additional functionality available after + /// a `Module` is complete. + pub fn finish(self) -> B::Product { + self.backend.finish() } } diff --git a/lib/simplejit/src/backend.rs b/lib/simplejit/src/backend.rs index 15e6892e19..f5e85575ce 100644 --- a/lib/simplejit/src/backend.rs +++ b/lib/simplejit/src/backend.rs @@ -12,6 +12,43 @@ use std::ptr; use libc; use memory::Memory; +/// A builder for `SimpleJITBackend`. +pub struct SimpleJITBuilder { + isa: Box, +} + +impl SimpleJITBuilder { + /// Create a new `SimpleJITBuilder`. + pub fn new() -> Self { + let (flag_builder, isa_builder) = cretonne_native::builders().unwrap_or_else(|_| { + panic!("host machine is not a supported target"); + }); + let isa = isa_builder.finish(settings::Flags::new(&flag_builder)); + Self::with_isa(isa) + } + + /// Create a new `SimpleJITBuilder` with an arbitrary target. This is mainly + /// useful for testing. + /// + /// SimpleJIT requires a `TargetIsa` configured for non-PIC. + /// + /// To create a `SimpleJITBuilder` for native use, use the `new` constructor + /// instead. + pub fn with_isa(isa: Box) -> Self { + debug_assert!(!isa.flags().is_pic(), "SimpleJIT requires non-PIC code"); + Self { isa } + } +} + +/// A `SimpleJITBackend` implements `Backend` and emits code and data into memory where it can be +/// directly called and accessed. +pub struct SimpleJITBackend { + isa: Box, + code_memory: Memory, + readonly_memory: Memory, + writable_memory: Memory, +} + /// A record of a relocation to perform. struct RelocRecord { offset: CodeOffset, @@ -32,49 +69,34 @@ pub struct SimpleJITCompiledData { relocs: Vec, } -/// A `SimpleJITBackend` implements `Backend` and emits code and data into memory where it can be -/// directly called and accessed. -pub struct SimpleJITBackend { - isa: Box, - code_memory: Memory, - readonly_memory: Memory, - writable_memory: Memory, -} +impl<'simple_jit_backend> Backend for SimpleJITBackend { + type Builder = SimpleJITBuilder; + + /// SimpleJIT compiled function and data objects may have outstanding + /// relocations that need to be performed before the memory can be used. + /// These relocations are performed within `finalize_function` and + /// `finalize_data`. + type CompiledFunction = SimpleJITCompiledFunction; + type CompiledData = SimpleJITCompiledData; + + /// SimpleJIT emits code and data into memory, and provides raw pointers + /// to them. + type FinalizedFunction = *const u8; + type FinalizedData = (*mut u8, usize); + + /// SimpleJIT emits code and data into memory as it processes them, so it + /// doesn't need to provide anything after the `Module` is complete. + type Product = (); -impl SimpleJITBackend { /// Create a new `SimpleJITBackend`. - pub fn new() -> Self { - let (flag_builder, isa_builder) = cretonne_native::builders().unwrap_or_else(|_| { - panic!("host machine is not a supported target"); - }); - let isa = isa_builder.finish(settings::Flags::new(&flag_builder)); - Self::with_isa(isa) - } - - /// Create a new `SimpleJITBackend` with an arbitrary target. This is mainly - /// useful for testing. - /// - /// SimpleJIT requires a `TargetIsa` configured for non-PIC. - /// - /// To create a `SimpleJITBackend` for native use, use the `new` constructor - /// instead. - pub fn with_isa(isa: Box) -> Self { - debug_assert!(!isa.flags().is_pic(), "SimpleJIT requires non-PIC code"); + fn new(builder: SimpleJITBuilder) -> Self { Self { - isa, + isa: builder.isa, code_memory: Memory::new(), readonly_memory: Memory::new(), writable_memory: Memory::new(), } } -} - -impl<'simple_jit_backend> Backend for SimpleJITBackend { - type CompiledFunction = SimpleJITCompiledFunction; - type CompiledData = SimpleJITCompiledData; - - type FinalizedFunction = *const u8; - type FinalizedData = (*mut u8, usize); fn isa(&self) -> &TargetIsa { &*self.isa @@ -317,6 +339,10 @@ impl<'simple_jit_backend> Backend for SimpleJITBackend { self.readonly_memory.set_readonly(); (data.storage, data.size) } + + /// SimpleJIT emits code and data into memory as it processes them, so it + /// doesn't need to provide anything after the `Module` is complete. + fn finish(self) -> () {} } fn lookup_with_dlsym(name: &str) -> *const u8 { diff --git a/lib/simplejit/src/lib.rs b/lib/simplejit/src/lib.rs index 1917a3db5e..68a2ca00ea 100644 --- a/lib/simplejit/src/lib.rs +++ b/lib/simplejit/src/lib.rs @@ -26,4 +26,4 @@ extern crate libc; mod backend; mod memory; -pub use backend::SimpleJITBackend; +pub use backend::{SimpleJITBuilder, SimpleJITBackend}; From 8e4511ed6c814e26a37a063df00be880f1d0ebc0 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 19 Apr 2018 11:46:49 -0700 Subject: [PATCH 5/6] Elaborate on the prelude. Rename the re-exported crates, so that prelude users can type "cretonne::codegen" rather than "cretonne::cretonne_codegen". And, add a few more useful items to the prelude. --- lib/umbrella/src/lib.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/umbrella/src/lib.rs b/lib/umbrella/src/lib.rs index 179a33224a..f2a93ad230 100644 --- a/lib/umbrella/src/lib.rs +++ b/lib/umbrella/src/lib.rs @@ -16,17 +16,20 @@ use_self, ))] -pub extern crate cretonne_codegen; -pub extern crate cretonne_frontend; +/// Provide these crates, renamed to reduce stutter. +pub extern crate cretonne_codegen as codegen; +pub extern crate cretonne_frontend as frontend; /// A prelude providing convenient access to commonly-used cretonne features. Use /// as `use cretonne::prelude::*`. pub mod prelude { - pub use cretonne_codegen; - pub use cretonne_codegen::entity::EntityRef; - pub use cretonne_codegen::ir::{AbiParam, InstBuilder, Value, Ebb, Signature, CallConv}; - pub use cretonne_codegen::ir::types; - pub use cretonne_codegen::ir::condcodes::IntCC; + pub use codegen; + pub use codegen::entity::EntityRef; + pub use codegen::ir::{AbiParam, InstBuilder, Value, Ebb, Signature, CallConv, Type, + JumpTableData, MemFlags}; + pub use codegen::ir::types; + pub use codegen::ir::condcodes::{IntCC, FloatCC}; + pub use codegen::ir::immediates::{Ieee32, Ieee64}; - pub use cretonne_frontend::{FunctionBuilderContext, FunctionBuilder, Variable}; + pub use frontend::{FunctionBuilderContext, FunctionBuilder, Variable}; } From d9cd94f20de2ca891aec4e6eb0b232b85d836403 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 19 Apr 2018 12:19:21 -0700 Subject: [PATCH 6/6] Add settings::Configurable and isa to the prelude. These are useful for API users that customize the TargetIsa. --- lib/umbrella/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/umbrella/src/lib.rs b/lib/umbrella/src/lib.rs index f2a93ad230..de89cd5788 100644 --- a/lib/umbrella/src/lib.rs +++ b/lib/umbrella/src/lib.rs @@ -30,6 +30,8 @@ pub mod prelude { pub use codegen::ir::types; pub use codegen::ir::condcodes::{IntCC, FloatCC}; pub use codegen::ir::immediates::{Ieee32, Ieee64}; + pub use codegen::settings::{self, Configurable}; + pub use codegen::isa; pub use frontend::{FunctionBuilderContext, FunctionBuilder, Variable}; }