cranelift: Avoid making MachLabel part of the exposed API

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

@@ -1898,10 +1898,9 @@ impl Inst {
                 //
                 // https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#global-dynamic
 
-                // Create a lable that is going to be published to the final binary object.
+                // Create the lable that is going to be published to the final binary object.
                 let auipc_label = sink.get_label();
                 sink.bind_label(auipc_label, &mut state.ctrl_plane);
-                sink.set_label_visiblity(auipc_label, LabelVisibility::Public);
 
                 // Get the current PC.
                 sink.add_reloc(Reloc::RiscvTlsGdHi20, &**name, 0);

cranelift/codegen/src/lib.rs

@@ -56,8 +56,8 @@ pub mod write;
 
 pub use crate::entity::packed_option;
 pub use crate::machinst::buffer::{
-    MachCallSite, MachLabelSite, MachReloc, MachSrcLoc, MachStackMap, MachTextSectionBuilder,
-    MachTrap, RelocTarget,
+    FinalizedMachReloc, FinalizedRelocTarget, MachCallSite, MachSrcLoc, MachStackMap,
+    MachTextSectionBuilder, MachTrap,
 };
 pub use crate::machinst::{
     CompiledCode, Final, MachBuffer, MachBufferFinalized, MachInst, MachInstEmit,

cranelift/codegen/src/machinst/buffer.rs

@@ -229,15 +229,6 @@ enum ForceVeneers {
     No,
 }
 
-/// Determines if a label should be emitted to the final binary object.
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum LabelVisibility {
-    /// Public labels are published in the final binary object.
-    Public,
-    /// Private labels are internal only.
-    Private,
-}
-
 /// A buffer of output to be produced, fixed up, and then emitted to a CodeSink
 /// in bulk.
 ///
@@ -276,12 +267,6 @@ pub struct MachBuffer<I: VCodeInst> {
     /// target (iteratively until a non-aliased label); if B is already
     /// aliased to A, then we cannot alias A back to B.
     label_aliases: SmallVec<[MachLabel; 16]>,
-    /// Label Visibility: If a label is visible, we publish it in the final binary
-    /// object, otherwise it is internal only.
-    ///
-    /// Public labels are the exception, so we only keep track of them and assume
-    /// all others are private.
-    public_labels: SmallVec<[MachLabel; 4]>,
     /// Constants that must be emitted at some point.
     pending_constants: SmallVec<[VCodeConstant; 16]>,
     /// Byte size of all constants in `pending_constants`.
@@ -334,7 +319,6 @@ impl MachBufferFinalized<Stencil> {
         MachBufferFinalized {
             data: self.data,
             relocs: self.relocs,
-            labels: self.labels,
             traps: self.traps,
             call_sites: self.call_sites,
             srclocs: self
@@ -362,9 +346,7 @@ pub struct MachBufferFinalized<T: CompilePhase> {
     /// Any relocations referring to this code. Note that only *external*
     /// relocations are tracked here; references to labels within the buffer are
     /// resolved before emission.
-    pub(crate) relocs: SmallVec<[MachReloc; 16]>,
-    /// Any published labels referring to this code.
-    pub(crate) labels: SmallVec<[MachLabelSite; 4]>,
+    pub(crate) relocs: SmallVec<[FinalizedMachReloc; 16]>,
     /// Any trap records referring to this code.
     pub(crate) traps: SmallVec<[MachTrap; 16]>,
     /// Any call site records referring to this code.
@@ -391,10 +373,6 @@ const LABEL_LIST_THRESHOLD: usize = 100;
 /// for references to the label, and will come back and patch the code
 /// appropriately when the label's location is eventually known.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(
-    feature = "enable-serde",
-    derive(serde_derive::Serialize, serde_derive::Deserialize)
-)]
 pub struct MachLabel(u32);
 entity_impl!(MachLabel);
 
@@ -448,7 +426,6 @@ impl<I: VCodeInst> MachBuffer<I> {
             cur_srcloc: None,
             label_offsets: SmallVec::new(),
             label_aliases: SmallVec::new(),
-            public_labels: SmallVec::new(),
             pending_constants: SmallVec::new(),
             pending_constants_size: 0,
             pending_traps: SmallVec::new(),
@@ -655,20 +632,6 @@ impl<I: VCodeInst> MachBuffer<I> {
         // Post-invariant: by `optimize_branches()` (see argument there).
     }
 
-    /// Set the visibility of a label. The label must have been bound already.
-    pub fn set_label_visiblity(&mut self, label: MachLabel, visibility: LabelVisibility) {
-        trace!(
-            "MachBuffer: set label {:?} visibility to {:?}",
-            label,
-            visibility
-        );
-        debug_assert_ne!(self.resolve_label_offset(label), UNKNOWN_LABEL_OFFSET);
-        match visibility {
-            LabelVisibility::Public => self.public_labels.push(label),
-            LabelVisibility::Private => self.public_labels.retain(|l| *l != label),
-        }
-    }
-
     /// Lazily clear `labels_at_tail` if the tail offset has moved beyond the
     /// offset that it applies to.
     fn lazily_clear_labels_at_tail(&mut self) {
@@ -1489,13 +1452,22 @@ impl<I: VCodeInst> MachBuffer<I> {
 
         let alignment = self.finish_constants(constants);
 
-        let labels = self
-            .public_labels
+        // Resolve all labels to their offsets.
+        let finalized_relocs = self
+            .relocs
             .iter()
-            .copied()
-            .map(|label| MachLabelSite {
-                offset: self.resolve_label_offset(label),
-                label,
+            .map(|reloc| FinalizedMachReloc {
+                offset: reloc.offset,
+                kind: reloc.kind,
+                addend: reloc.addend,
+                target: match &reloc.target {
+                    RelocTarget::ExternalName(name) => {
+                        FinalizedRelocTarget::ExternalName(name.clone())
+                    }
+                    RelocTarget::Label(label) => {
+                        FinalizedRelocTarget::Func(self.resolve_label_offset(*label))
+                    }
+                },
             })
             .collect();
 
@@ -1504,8 +1476,7 @@ impl<I: VCodeInst> MachBuffer<I> {
 
         MachBufferFinalized {
             data: self.data,
-            relocs: self.relocs,
-            labels,
+            relocs: finalized_relocs,
             traps: self.traps,
             call_sites: self.call_sites,
             srclocs,
@@ -1676,15 +1647,10 @@ impl<T: CompilePhase> MachBufferFinalized<T> {
     }
 
     /// Get the list of external relocations for this code.
-    pub fn relocs(&self) -> &[MachReloc] {
+    pub fn relocs(&self) -> &[FinalizedMachReloc] {
         &self.relocs[..]
     }
 
-    /// Get the list of published labels for this code.
-    pub fn labels(&self) -> &[MachLabelSite] {
-        &self.labels[..]
-    }
-
     /// Get the list of trap records for this code.
     pub fn traps(&self) -> &[MachTrap] {
         &self.traps[..]
@@ -1776,24 +1742,25 @@ impl<I: VCodeInst> Ord for MachLabelFixup<I> {
     feature = "enable-serde",
     derive(serde_derive::Serialize, serde_derive::Deserialize)
 )]
-pub struct MachReloc {
+pub struct MachRelocBase<T> {
     /// The offset at which the relocation applies, *relative to the
     /// containing section*.
     pub offset: CodeOffset,
     /// The kind of relocation.
     pub kind: Reloc,
     /// The external symbol / name to which this relocation refers.
-    pub target: RelocTarget,
+    pub target: T,
     /// The addend to add to the symbol value.
     pub addend: i64,
 }
 
+type MachReloc = MachRelocBase<RelocTarget>;
+
+/// A relocation resulting from a compilation.
+pub type FinalizedMachReloc = MachRelocBase<FinalizedRelocTarget>;
+
 /// A Relocation target
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-#[cfg_attr(
-    feature = "enable-serde",
-    derive(serde_derive::Serialize, serde_derive::Deserialize)
-)]
 pub enum RelocTarget {
     /// Points to an [ExternalName] outside the current function.
     ExternalName(ExternalName),
@@ -1817,31 +1784,30 @@ impl From<MachLabel> for RelocTarget {
     }
 }
 
-impl RelocTarget {
-    /// Returns a display for the current `RelocTarget`, with extra context to prettify the
+/// A Relocation target
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[cfg_attr(
+    feature = "enable-serde",
+    derive(serde_derive::Serialize, serde_derive::Deserialize)
+)]
+pub enum FinalizedRelocTarget {
+    /// Points to an [ExternalName] outside the current function.
+    ExternalName(ExternalName),
+    /// Points to a [CodeOffset] from the start of the current function.
+    Func(CodeOffset),
+}
+
+impl FinalizedRelocTarget {
+    /// Returns a display for the current [FinalizedRelocTarget], with extra context to prettify the
     /// output.
     pub fn display<'a>(&'a self, params: Option<&'a FunctionParameters>) -> String {
         match self {
-            RelocTarget::ExternalName(name) => format!("{}", name.display(params)),
-            RelocTarget::Label(label) => format!(".L{}", label.as_u32()),
+            FinalizedRelocTarget::ExternalName(name) => format!("{}", name.display(params)),
+            FinalizedRelocTarget::Func(offset) => format!("func+{offset}"),
         }
     }
 }
 
-/// A label resulting from a compilation.
-#[derive(Clone, Debug, PartialEq)]
-#[cfg_attr(
-    feature = "enable-serde",
-    derive(serde_derive::Serialize, serde_derive::Deserialize)
-)]
-pub struct MachLabelSite {
-    /// The offset at which the relocation applies, *relative to the
-    /// containing section*.
-    pub offset: CodeOffset,
-    /// The label.
-    pub label: MachLabel,
-}
-
 /// A trap record resulting from a compilation.
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(

cranelift/filetests/filetests/isa/riscv64/tls-elf.clif

@@ -40,7 +40,7 @@ block0(v0: i32):
 ; block1: ; offset 0x18
 ;   mv s1, a0
 ;   auipc a0, 0 ; reloc_external RiscvTlsGdHi20 u1:0 0
-;   mv a0, a0 ; reloc_external RiscvPCRelLo12I .L1 0
+;   mv a0, a0 ; reloc_external RiscvPCRelLo12I func+28 0
 ;   auipc t5, 0
 ;   ld t5, 0xc(t5)
 ;   j 0xc

cranelift/jit/src/backend.rs

@@ -4,7 +4,7 @@ use crate::{compiled_blob::CompiledBlob, memory::BranchProtection, memory::Memor
 use cranelift_codegen::binemit::Reloc;
 use cranelift_codegen::isa::{OwnedTargetIsa, TargetIsa};
 use cranelift_codegen::settings::Configurable;
-use cranelift_codegen::{self, ir, settings, MachLabelSite, MachReloc};
+use cranelift_codegen::{self, ir, settings, FinalizedMachReloc};
 use cranelift_control::ControlPlane;
 use cranelift_entity::SecondaryMap;
 use cranelift_module::{
@@ -758,8 +758,7 @@ impl Module for JITModule {
         func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[MachReloc],
-        _labels: &[MachLabelSite],
+        relocs: &[FinalizedMachReloc],
     ) -> ModuleResult<()> {
         info!("defining function {} with bytes", id);
         let decl = self.declarations.get_function_decl(id);

cranelift/module/src/module.rs

@@ -13,8 +13,9 @@ use cranelift_codegen::entity::{entity_impl, PrimaryMap};
 use cranelift_codegen::ir::function::{Function, VersionMarker};
 use cranelift_codegen::ir::{ExternalName, UserFuncName};
 use cranelift_codegen::settings::SetError;
-use cranelift_codegen::{ir, isa, CodegenError, CompileError, Context, MachLabel, RelocTarget};
-use cranelift_codegen::{MachLabelSite, MachReloc};
+use cranelift_codegen::{
+    ir, isa, CodegenError, CompileError, Context, FinalizedMachReloc, FinalizedRelocTarget,
+};
 use cranelift_control::ControlPlane;
 use std::borrow::{Cow, ToOwned};
 use std::string::String;
@@ -34,21 +35,23 @@ pub struct ModuleReloc {
 }
 
 impl ModuleReloc {
-    /// Converts a `MachReloc` produced from a `Function` into a `ModuleReloc`.
-    pub fn from_mach_reloc(mach_reloc: &MachReloc, func: &Function) -> Self {
+    /// Converts a `FinalizedMachReloc` produced from a `Function` into a `ModuleReloc`.
+    pub fn from_mach_reloc(mach_reloc: &FinalizedMachReloc, func: &Function) -> Self {
         let name = match mach_reloc.target {
-            RelocTarget::ExternalName(ExternalName::User(reff)) => {
+            FinalizedRelocTarget::ExternalName(ExternalName::User(reff)) => {
                 let name = &func.params.user_named_funcs()[reff];
                 ModuleRelocTarget::user(name.namespace, name.index)
             }
-            RelocTarget::ExternalName(ExternalName::TestCase(_)) => unimplemented!(),
-            RelocTarget::ExternalName(ExternalName::LibCall(libcall)) => {
+            FinalizedRelocTarget::ExternalName(ExternalName::TestCase(_)) => unimplemented!(),
+            FinalizedRelocTarget::ExternalName(ExternalName::LibCall(libcall)) => {
                 ModuleRelocTarget::LibCall(libcall)
             }
-            RelocTarget::ExternalName(ExternalName::KnownSymbol(ks)) => {
+            FinalizedRelocTarget::ExternalName(ExternalName::KnownSymbol(ks)) => {
                 ModuleRelocTarget::KnownSymbol(ks)
             }
-            RelocTarget::Label(label) => ModuleRelocTarget::FunctionLabel(func.name.clone(), label),
+            FinalizedRelocTarget::Func(offset) => {
+                ModuleRelocTarget::FunctionOffset(func.name.clone(), offset)
+            }
         };
         Self {
             offset: mach_reloc.offset,
@@ -426,8 +429,8 @@ pub enum ModuleRelocTarget {
     LibCall(ir::LibCall),
     /// Symbols known to the linker.
     KnownSymbol(ir::KnownSymbol),
-    /// A label inside a function
-    FunctionLabel(UserFuncName, MachLabel),
+    /// A offset inside a function
+    FunctionOffset(UserFuncName, CodeOffset),
 }
 
 impl ModuleRelocTarget {
@@ -443,7 +446,7 @@ impl Display for ModuleRelocTarget {
             Self::User { namespace, index } => write!(f, "u{}:{}", namespace, index),
             Self::LibCall(lc) => write!(f, "%{}", lc),
             Self::KnownSymbol(ks) => write!(f, "{}", ks),
-            Self::FunctionLabel(fname, label) => write!(f, ".L{}_{}", fname, label.as_u32()),
+            Self::FunctionOffset(fname, offset) => write!(f, "{fname}+{offset}"),
         }
     }
 }
@@ -700,7 +703,7 @@ impl ModuleDeclarations {
             ModuleRelocTarget::User { namespace, .. } => *namespace == 0,
             ModuleRelocTarget::LibCall(_)
             | ModuleRelocTarget::KnownSymbol(_)
-            | ModuleRelocTarget::FunctionLabel(..) => {
+            | ModuleRelocTarget::FunctionOffset(..) => {
                 panic!("unexpected module ext name")
             }
         }
@@ -976,8 +979,7 @@ pub trait Module {
         func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[MachReloc],
-        labels: &[MachLabelSite],
+        relocs: &[FinalizedMachReloc],
     ) -> ModuleResult<()>;
 
     /// Define a data object, producing the data contents from the given `DataContext`.
@@ -1079,10 +1081,9 @@ impl<M: Module> Module for &mut M {
         func: &ir::Function,
         alignment: u64,
         bytes: &[u8],
-        relocs: &[MachReloc],
-        labels: &[MachLabelSite],
+        relocs: &[FinalizedMachReloc],
     ) -> ModuleResult<()> {
-        (**self).define_function_bytes(func_id, func, alignment, bytes, relocs, labels)
+        (**self).define_function_bytes(func_id, func, alignment, bytes, relocs)
     }
 
     fn define_data(&mut self, data_id: DataId, data: &DataDescription) -> ModuleResult<()> {