cranelift: Remove next_fixed_nonallocatable and with_allocs metho…

…ds (#8566)

The `next_fixed_nonallocatable` method doesn't do anything any more and
doesn't return anything so calls to it can just be deleted.

The `with_allocs`, `allocate`, and `to_string_with_alloc` methods are
all trivial at this point, so inline them. The bulk of this change was
performed this way:

git grep -lF '.with_allocs(' | xargs sed -i 's/\.with_allocs([^)]*)/.clone()/g'

In a couple cases, this makes the `AllocationConsumer` unused at these
methods' call sites. Rather than changing function signatures in this
PR, just mark those arguments as deliberately unused.

The number of structures being cloned here is unfortunate, and
unnecessary now that we don't need to mutate any of them. But switching
to borrowing them is a bigger change than I want to include here.

cranelift/codegen/src/isa/aarch64/inst/args.rs

@@ -145,20 +145,10 @@ impl AMode {
             extendop: op,
         }
     }
-
-    pub(crate) fn with_allocs(&self, _allocs: &mut AllocationConsumer) -> Self {
-        self.clone()
-    }
 }
 
 pub use crate::isa::aarch64::lower::isle::generated_code::PairAMode;
 
-impl PairAMode {
-    pub(crate) fn with_allocs(&self, _allocs: &mut AllocationConsumer) -> Self {
-        self.clone()
-    }
-}
-
 //=============================================================================
 // Instruction sub-components (conditions, branches and branch targets):
 // definitions

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

@@ -997,7 +997,7 @@ impl MachInstEmit for Inst {
             | &Inst::FpuLoad64 { rd, ref mem, flags }
             | &Inst::FpuLoad128 { rd, ref mem, flags } => {
                 let rd = allocs.next_writable(rd);
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_insts, mem) = mem_finalize(Some(sink), &mem, access_ty, state);
 
@@ -1138,7 +1138,7 @@ impl MachInstEmit for Inst {
             | &Inst::FpuStore64 { rd, ref mem, flags }
             | &Inst::FpuStore128 { rd, ref mem, flags } => {
                 let rd = allocs.next(rd);
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_insts, mem) = mem_finalize(Some(sink), &mem, access_ty, state);
 
@@ -1233,7 +1233,7 @@ impl MachInstEmit for Inst {
             } => {
                 let rt = allocs.next(rt);
                 let rt2 = allocs.next(rt2);
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
                 if let Some(trap_code) = flags.trap_code() {
                     // Register the offset at which the actual store instruction starts.
                     sink.add_trap(trap_code);
@@ -1264,7 +1264,7 @@ impl MachInstEmit for Inst {
             } => {
                 let rt = allocs.next(rt.to_reg());
                 let rt2 = allocs.next(rt2.to_reg());
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
                 if let Some(trap_code) = flags.trap_code() {
                     // Register the offset at which the actual load instruction starts.
                     sink.add_trap(trap_code);
@@ -1302,7 +1302,7 @@ impl MachInstEmit for Inst {
             } => {
                 let rt = allocs.next(rt.to_reg());
                 let rt2 = allocs.next(rt2.to_reg());
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
 
                 if let Some(trap_code) = flags.trap_code() {
                     // Register the offset at which the actual load instruction starts.
@@ -1347,7 +1347,7 @@ impl MachInstEmit for Inst {
             } => {
                 let rt = allocs.next(rt);
                 let rt2 = allocs.next(rt2);
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
 
                 if let Some(trap_code) = flags.trap_code() {
                     // Register the offset at which the actual store instruction starts.
@@ -1416,7 +1416,6 @@ impl MachInstEmit for Inst {
             }
             &Inst::MovFromPReg { rd, rm } => {
                 let rd = allocs.next_writable(rd);
-                allocs.next_fixed_nonallocatable(rm);
                 let rm: Reg = rm.into();
                 debug_assert!([
                     regs::fp_reg(),
@@ -1431,7 +1430,6 @@ impl MachInstEmit for Inst {
                 Inst::Mov { size, rd, rm }.emit(&[], sink, emit_info, state);
             }
             &Inst::MovToPReg { rd, rm } => {
-                allocs.next_fixed_nonallocatable(rd);
                 let rd: Writable<Reg> = Writable::from_reg(rd.into());
                 let rm = allocs.next(rm);
                 debug_assert!([
@@ -3454,7 +3452,7 @@ impl MachInstEmit for Inst {
             }
             &Inst::LoadAddr { rd, ref mem } => {
                 let rd = allocs.next_writable(rd);
-                let mem = mem.with_allocs(&mut allocs);
+                let mem = mem.clone();
                 let (mem_insts, mem) = mem_finalize(Some(sink), &mem, I8, state);
                 for inst in mem_insts.into_iter() {
                     inst.emit(&[], sink, emit_info, state);

cranelift/codegen/src/isa/aarch64/inst/mod.rs

@@ -397,7 +397,6 @@ impl Inst {
 // Instructions: get_regs
 
 fn memarg_operands(memarg: &mut AMode, collector: &mut impl OperandVisitor) {
-    // This should match `AMode::with_allocs()`.
     match memarg {
         AMode::Unscaled { rn, .. } | AMode::UnsignedOffset { rn, .. } => {
             collector.reg_use(rn);
@@ -421,7 +420,6 @@ fn memarg_operands(memarg: &mut AMode, collector: &mut impl OperandVisitor) {
 }
 
 fn pairmemarg_operands(pairmemarg: &mut PairAMode, collector: &mut impl OperandVisitor) {
-    // This should match `PairAMode::with_allocs()`.
     match pairmemarg {
         PairAMode::SignedOffset { reg, .. } => {
             collector.reg_use(reg);
@@ -1265,9 +1263,6 @@ impl Inst {
             }
         }
 
-        // N.B.: order of `allocs` consumption (via register
-        // pretty-printing or memarg.with_allocs()) needs to match the
-        // order in `aarch64_get_operands` above.
         match self {
             &Inst::Nop0 => "nop-zero-len".to_string(),
             &Inst::Nop4 => "nop".to_string(),
@@ -1416,7 +1411,7 @@ impl Inst {
                 };
 
                 let rd = pretty_print_ireg(rd.to_reg(), size, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
 
@@ -1443,7 +1438,7 @@ impl Inst {
                 };
 
                 let rd = pretty_print_ireg(rd, size, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
 
@@ -1454,7 +1449,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_ireg(rt, OperandSize::Size64, allocs);
                 let rt2 = pretty_print_ireg(rt2, OperandSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
                 format!("stp {}, {}, {}", rt, rt2, mem)
             }
@@ -1463,7 +1458,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_ireg(rt.to_reg(), OperandSize::Size64, allocs);
                 let rt2 = pretty_print_ireg(rt2.to_reg(), OperandSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
                 format!("ldp {}, {}, {}", rt, rt2, mem)
             }
@@ -1474,12 +1469,10 @@ impl Inst {
             }
             &Inst::MovFromPReg { rd, rm } => {
                 let rd = pretty_print_ireg(rd.to_reg(), OperandSize::Size64, allocs);
-                allocs.next_fixed_nonallocatable(rm);
                 let rm = show_ireg_sized(rm.into(), OperandSize::Size64);
                 format!("mov {}, {}", rd, rm)
             }
             &Inst::MovToPReg { rd, rm } => {
-                allocs.next_fixed_nonallocatable(rd);
                 let rd = show_ireg_sized(rd.into(), OperandSize::Size64);
                 let rm = pretty_print_ireg(rm, OperandSize::Size64, allocs);
                 format!("mov {}, {}", rd, rm)
@@ -1832,44 +1825,44 @@ impl Inst {
             }
             &Inst::FpuLoad32 { rd, ref mem, .. } => {
                 let rd = pretty_print_vreg_scalar(rd.to_reg(), ScalarSize::Size32, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}ldr {}, {}", mem_str, rd, mem)
             }
             &Inst::FpuLoad64 { rd, ref mem, .. } => {
                 let rd = pretty_print_vreg_scalar(rd.to_reg(), ScalarSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}ldr {}, {}", mem_str, rd, mem)
             }
             &Inst::FpuLoad128 { rd, ref mem, .. } => {
                 let rd = pretty_print_reg(rd.to_reg(), allocs);
                 let rd = "q".to_string() + &rd[1..];
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}ldr {}, {}", mem_str, rd, mem)
             }
             &Inst::FpuStore32 { rd, ref mem, .. } => {
                 let rd = pretty_print_vreg_scalar(rd, ScalarSize::Size32, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}str {}, {}", mem_str, rd, mem)
             }
             &Inst::FpuStore64 { rd, ref mem, .. } => {
                 let rd = pretty_print_vreg_scalar(rd, ScalarSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}str {}, {}", mem_str, rd, mem)
             }
             &Inst::FpuStore128 { rd, ref mem, .. } => {
                 let rd = pretty_print_reg(rd, allocs);
                 let rd = "q".to_string() + &rd[1..];
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let access_ty = self.mem_type().unwrap();
                 let (mem_str, mem) = mem_finalize_for_show(&mem, access_ty, state);
                 format!("{}str {}, {}", mem_str, rd, mem)
@@ -1879,7 +1872,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_vreg_scalar(rt.to_reg(), ScalarSize::Size64, allocs);
                 let rt2 = pretty_print_vreg_scalar(rt2.to_reg(), ScalarSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
 
                 format!("ldp {}, {}, {}", rt, rt2, mem)
@@ -1889,7 +1882,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_vreg_scalar(rt, ScalarSize::Size64, allocs);
                 let rt2 = pretty_print_vreg_scalar(rt2, ScalarSize::Size64, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
 
                 format!("stp {}, {}, {}", rt, rt2, mem)
@@ -1899,7 +1892,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_vreg_scalar(rt.to_reg(), ScalarSize::Size128, allocs);
                 let rt2 = pretty_print_vreg_scalar(rt2.to_reg(), ScalarSize::Size128, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
 
                 format!("ldp {}, {}, {}", rt, rt2, mem)
@@ -1909,7 +1902,7 @@ impl Inst {
             } => {
                 let rt = pretty_print_vreg_scalar(rt, ScalarSize::Size128, allocs);
                 let rt2 = pretty_print_vreg_scalar(rt2, ScalarSize::Size128, allocs);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let mem = mem.pretty_print_default();
 
                 format!("stp {}, {}, {}", rt, rt2, mem)
@@ -2781,7 +2774,7 @@ impl Inst {
                 // expansion stage (i.e., legalization, but without the slow edit-in-place
                 // of the existing legalization framework).
                 let rd = allocs.next_writable(rd);
-                let mem = mem.with_allocs(allocs);
+                let mem = mem.clone();
                 let (mem_insts, mem) = mem_finalize(None, &mem, I8, state);
                 let mut ret = String::new();
                 for inst in mem_insts.into_iter() {

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

@@ -113,10 +113,6 @@ pub enum AMode {
 }
 
 impl AMode {
-    pub(crate) fn with_allocs(self, _allocs: &mut AllocationConsumer) -> Self {
-        self
-    }
-
     /// Add the registers referenced by this AMode to `collector`.
     pub(crate) fn get_operands(&mut self, collector: &mut impl OperandVisitor) {
         match self {
@@ -176,10 +172,6 @@ impl AMode {
             | &AMode::NominalSPOffset(..) => None,
         }
     }
-
-    pub(crate) fn to_string_with_alloc(&self, allocs: &mut AllocationConsumer) -> String {
-        format!("{}", self.clone().with_allocs(allocs))
-    }
 }
 
 impl Display for AMode {

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

@@ -223,17 +223,13 @@ impl MachInstEmit for Inst {
 
     fn emit(
         &self,
-        allocs: &[Allocation],
+        _allocs: &[Allocation],
         sink: &mut MachBuffer<Inst>,
         emit_info: &Self::Info,
         state: &mut EmitState,
     ) {
-        // Transform this into a instruction with all the physical regs
-        let mut allocs = AllocationConsumer::new(allocs);
-        let inst = self.clone().allocate(&mut allocs);
-
         // Check if we need to update the vector state before emitting this instruction
-        if let Some(expected) = inst.expected_vstate() {
+        if let Some(expected) = self.expected_vstate() {
             if state.vstate != EmitVState::Known(expected.clone()) {
                 // Update the vector state.
                 Inst::VecSetState {
@@ -252,10 +248,10 @@ impl MachInstEmit for Inst {
         let mut start_off = sink.cur_offset();
 
         // First try to emit this as a compressed instruction
-        let res = inst.try_emit_compressed(sink, emit_info, state, &mut start_off);
+        let res = self.try_emit_compressed(sink, emit_info, state, &mut start_off);
         if res.is_none() {
             // If we can't lets emit it as a normal instruction
-            inst.emit_uncompressed(sink, emit_info, state, &mut start_off);
+            self.emit_uncompressed(sink, emit_info, state, &mut start_off);
         }
 
         let end_off = sink.cur_offset();
@@ -2605,10 +2601,6 @@ impl Inst {
             }
         };
     }
-
-    fn allocate(self, _allocs: &mut AllocationConsumer) -> Self {
-        self
-    }
 }
 
 fn emit_return_call_common_sequence(

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

@@ -934,9 +934,9 @@ impl Inst {
             reg_name(reg)
         };
 
-        let format_vec_amode = |amode: &VecAMode, allocs: &mut AllocationConsumer| -> String {
+        let format_vec_amode = |amode: &VecAMode, _allocs: &mut AllocationConsumer| -> String {
             match amode {
-                VecAMode::UnitStride { base } => base.to_string_with_alloc(allocs),
+                VecAMode::UnitStride { base } => base.to_string(),
             }
         };
 
@@ -1346,7 +1346,7 @@ impl Inst {
                 from,
                 flags: _flags,
             } => {
-                let base = from.to_string_with_alloc(allocs);
+                let base = from.to_string();
                 let rd = format_reg(rd.to_reg(), allocs);
                 format!("{} {},{}", op.op_name(), rd, base,)
             }
@@ -1356,7 +1356,7 @@ impl Inst {
                 op,
                 flags: _flags,
             } => {
-                let base = to.to_string_with_alloc(allocs);
+                let base = to.to_string();
                 let src = format_reg(src, allocs);
                 format!("{} {},{}", op.op_name(), src, base,)
             }
@@ -1497,7 +1497,7 @@ impl Inst {
                 format!("elf_tls_get_addr {rd},{}", name.display(None))
             }
             &MInst::LoadAddr { ref rd, ref mem } => {
-                let rs = mem.to_string_with_alloc(allocs);
+                let rs = mem.to_string();
                 let rd = format_reg(rd.to_reg(), allocs);
                 format!("load_addr {},{}", rd, rs)
             }

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

@@ -101,11 +101,6 @@ impl MemArg {
             MemArg::NominalSPOffset { .. } => MemFlags::trusted(),
         }
     }
-
-    /// Edit registers with allocations.
-    pub fn with_allocs(&self, _allocs: &mut AllocationConsumer) -> Self {
-        self.clone()
-    }
 }
 
 /// A memory argument for an instruction with two memory operands.
@@ -153,11 +148,6 @@ impl MemArgPair {
             _ => None,
         }
     }
-
-    /// Edit registers with allocations.
-    pub fn with_allocs(&self, _allocs: &mut AllocationConsumer) -> Self {
-        self.clone()
-    }
 }
 
 //=============================================================================