From 03d77d4d6b1160a6be07c518d816b083443bc7b4 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 12 Oct 2022 07:58:27 -0700 Subject: [PATCH] Cranelift: Derive `Copy` for `InstructionData` (#5043) * Cranelift: Derive `Copy` for `InstructionData` And update `clone` calls to be copies. * Add a test for `InstructionData`'s size --- cranelift/codegen/meta/src/gen_inst.rs | 13 ++++++------- cranelift/codegen/src/ir/function.rs | 2 +- cranelift/codegen/src/ir/instructions.rs | 13 +++++++++++++ cranelift/codegen/src/machinst/isle.rs | 2 +- cranelift/codegen/src/simple_gvn.rs | 2 +- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index 3ff454d46f..581297a40e 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -71,17 +71,16 @@ fn gen_formats(formats: &[&InstructionFormat], fmt: &mut Formatter) { /// the SSA `Value` arguments. fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) { for (name, include_args) in &[("InstructionData", true), ("InstructionImms", false)] { - fmt.line("#[derive(Clone, Debug, PartialEq, Hash)]"); + fmt.line("#[derive(Copy, Clone, Debug, PartialEq, Hash)]"); if !include_args { - // `InstructionImms` gets some extra derives: it acts like - // a sort of extended opcode and we want to allow for - // hashconsing via Eq. `Copy` also turns out to be useful. - fmt.line("#[derive(Copy, Eq)]"); + // `InstructionImms` gets some extra derives: it acts like a sort of + // extended opcode and we want to allow for hashconsing via `Eq`. + fmt.line("#[derive(Eq)]"); } fmt.line(r#"#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]"#); fmt.line("#[allow(missing_docs)]"); - // generate `enum InstructionData` or `enum InstructionImms`. - // (This comment exists so one can grep for `enum InstructionData`!) + // Generate `enum InstructionData` or `enum InstructionImms`. (This + // comment exists so one can grep for `enum InstructionData`!) fmtln!(fmt, "pub enum {} {{", name); fmt.indent(|fmt| { for format in formats { diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index 4fdcfdde64..06f0bcd756 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -386,7 +386,7 @@ impl FunctionStencil { .zip(self.dfg.inst_results(src)) .all(|(a, b)| self.dfg.value_type(*a) == self.dfg.value_type(*b))); - self.dfg[dst] = self.dfg[src].clone(); + self.dfg[dst] = self.dfg[src]; self.layout.remove_inst(src); } diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs index a147816e42..b6c20a575c 100644 --- a/cranelift/codegen/src/ir/instructions.rs +++ b/cranelift/codegen/src/ir/instructions.rs @@ -755,6 +755,19 @@ mod tests { use super::*; use alloc::string::ToString; + #[test] + fn inst_data_is_copy() { + fn is_copy() {} + is_copy::(); + } + + #[test] + fn inst_data_size() { + // The size of `InstructionData` is performance sensitive, so make sure + // we don't regress it unintentionally. + assert_eq!(std::mem::size_of::(), 16); + } + #[test] fn opcodes() { use core::mem; diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index e82b7152eb..dd59b676a2 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -226,7 +226,7 @@ macro_rules! isle_lower_prelude_methods { #[inline] fn inst_data(&mut self, inst: Inst) -> InstructionData { - self.lower_ctx.dfg()[inst].clone() + self.lower_ctx.dfg()[inst] } #[inline] diff --git a/cranelift/codegen/src/simple_gvn.rs b/cranelift/codegen/src/simple_gvn.rs index 07a7663039..327ff8f4bd 100644 --- a/cranelift/codegen/src/simple_gvn.rs +++ b/cranelift/codegen/src/simple_gvn.rs @@ -115,7 +115,7 @@ pub fn do_simple_gvn(func: &mut Function, domtree: &mut DominatorTree) { let ctrl_typevar = func.dfg.ctrl_typevar(inst); let key = HashKey { - inst: func.dfg[inst].clone(), + inst: func.dfg[inst], ty: ctrl_typevar, pos: &pos, };