From a76e0e8aa57220fe9c75f810c293ba2694d65e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Cabrera?= Date: Tue, 13 Dec 2022 07:13:45 -0500 Subject: [PATCH] Decouple `MachBufferFinalized` from `ir::FunctionParameters` (#5419) This commit allows retrieving a `MachBufferFinalized` from a `MachBufferFinalized` without relying on `ir::FunctionParameters`. Instead it uses the function's base source location, which is the only piece used by the previous `apply_params` definition. This change allows other uses cases (e.g. Winch) to use an opaque, common concept, exposed outside of `cranelift-codegen` to get the finalized state of the machine buffer. This change implies that Winch will transitively know about the `Stencil` compilation phase, but the `Stencil` phase is not exposed to Winch. Other alternatives considered: Parametrizing `MachBufferFinzalized` in a way such that it allows specifying which compilation phase the caller is targetting. Such approach would require also parametrizing the `MachSrcLoc` definition. One of the main drawbacks of this approach is that it also requires changing how the `MachBuffer`'s `start_srcloc` works: for caller requesting a `Final` `MachBufferFinalized`, the `MachBuffer` will need to work in terms of `SourceLoc` rather than in `RelSourceLoc` terms. This approach doesn't necessarily present more advantages than the approach presented in this change, in which there's no need to make any fundamental changes and in which all the `cranelift-codegen` primitives are already exposed. --- cranelift/codegen/src/machinst/buffer.rs | 12 +++++------- cranelift/codegen/src/machinst/mod.rs | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/cranelift/codegen/src/machinst/buffer.rs b/cranelift/codegen/src/machinst/buffer.rs index 8a71d33cbc..316e916db3 100644 --- a/cranelift/codegen/src/machinst/buffer.rs +++ b/cranelift/codegen/src/machinst/buffer.rs @@ -141,7 +141,6 @@ //! semantics below (grep for "Preserves execution semantics"). use crate::binemit::{Addend, CodeOffset, Reloc, StackMap}; -use crate::ir::function::FunctionParameters; use crate::ir::{ExternalName, Opcode, RelSourceLoc, SourceLoc, TrapCode}; use crate::isa::unwind::UnwindInst; use crate::machinst::{ @@ -172,8 +171,6 @@ pub trait CompilePhase { } /// Status of a compiled artifact that needs patching before being used. -/// -/// Only used internally. #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] pub struct Stencil; @@ -267,7 +264,8 @@ pub struct MachBuffer { } impl MachBufferFinalized { - pub(crate) fn apply_params(self, params: &FunctionParameters) -> MachBufferFinalized { + /// Get a finalized machine buffer by applying the function's base source location. + pub fn apply_base_srcloc(self, base_srcloc: SourceLoc) -> MachBufferFinalized { MachBufferFinalized { data: self.data, relocs: self.relocs, @@ -276,7 +274,7 @@ impl MachBufferFinalized { srclocs: self .srclocs .into_iter() - .map(|srcloc| srcloc.apply_params(params)) + .map(|srcloc| srcloc.apply_base_srcloc(base_srcloc)) .collect(), stack_maps: self.stack_maps, unwind_info: self.unwind_info, @@ -1550,11 +1548,11 @@ pub struct MachSrcLoc { } impl MachSrcLoc { - fn apply_params(self, params: &FunctionParameters) -> MachSrcLoc { + fn apply_base_srcloc(self, base_srcloc: SourceLoc) -> MachSrcLoc { MachSrcLoc { start: self.start, end: self.end, - loc: self.loc.expand(params.base_srcloc()), + loc: self.loc.expand(base_srcloc), } } } diff --git a/cranelift/codegen/src/machinst/mod.rs b/cranelift/codegen/src/machinst/mod.rs index c08274c709..37f617fd6a 100644 --- a/cranelift/codegen/src/machinst/mod.rs +++ b/cranelift/codegen/src/machinst/mod.rs @@ -323,7 +323,7 @@ impl CompiledCodeStencil { /// Apply function parameters to finalize a stencil into its final form. pub fn apply_params(self, params: &FunctionParameters) -> CompiledCode { CompiledCode { - buffer: self.buffer.apply_params(params), + buffer: self.buffer.apply_base_srcloc(params.base_srcloc()), frame_size: self.frame_size, disasm: self.disasm, value_labels_ranges: self.value_labels_ranges,