From a05bf2bf4285a1ba4153746a1a8689c7d28b120d Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Tue, 12 Oct 2021 14:37:36 +0200 Subject: [PATCH] Remove instructions necessary for the old regalloc --- .../codegen/meta/src/shared/instructions.rs | 150 ------------------ .../codegen/src/isa/aarch64/lower_inst.rs | 4 - cranelift/codegen/src/isa/s390x/lower.rs | 16 -- cranelift/codegen/src/isa/x64/lower.rs | 31 +--- cranelift/codegen/src/peepmatic.rs | 25 --- cranelift/codegen/src/preopt.peepmatic | 3 - cranelift/codegen/src/simple_preopt.rs | 9 -- cranelift/codegen/src/verifier/mod.rs | 56 +------ .../filetests/simple_gvn/reject.clif | 15 -- cranelift/interpreter/src/step.rs | 9 -- .../peepmatic/crates/fuzzing/src/interp.rs | 9 -- .../peepmatic/crates/test-operator/src/lib.rs | 8 - 12 files changed, 3 insertions(+), 332 deletions(-) diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 0703b19b96..23dbbaea0a 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1798,156 +1798,6 @@ pub(crate) fn define( .operands_out(vec![a]), ); - ig.push( - Inst::new( - "spill", - r#" - Spill a register value to a stack slot. - - This instruction behaves exactly like `copy`, but the result - value is assigned to a spill slot. - "#, - &formats.unary, - ) - .operands_in(vec![x]) - .operands_out(vec![a]) - .can_store(true), - ); - - ig.push( - Inst::new( - "fill", - r#" - Load a register value from a stack slot. - - This instruction behaves exactly like `copy`, but creates a new - SSA value for the spilled input value. - "#, - &formats.unary, - ) - .operands_in(vec![x]) - .operands_out(vec![a]) - .can_load(true), - ); - - ig.push( - Inst::new( - "fill_nop", - r#" - This is identical to `fill`, except it has no encoding, since it is a no-op. - - This instruction is created only during late-stage redundant-reload removal, after all - registers and stack slots have been assigned. It is used to replace `fill`s that have - been identified as redundant. - "#, - &formats.unary, - ) - .operands_in(vec![x]) - .operands_out(vec![a]) - .can_load(true), - ); - - ig.push( - Inst::new( - "copy_nop", - r#" - Stack-slot-to-the-same-stack-slot copy, which is guaranteed to turn - into a no-op. This instruction is for use only within Cranelift itself. - - This instruction copies its input, preserving the value type. - "#, - &formats.unary, - ) - .operands_in(vec![x]) - .operands_out(vec![a]), - ); - - let delta = &Operand::new("delta", Int); - - ig.push( - Inst::new( - "adjust_sp_down", - r#" - Subtracts ``delta`` offset value from the stack pointer register. - - This instruction is used to adjust the stack pointer by a dynamic amount. - "#, - &formats.unary, - ) - .operands_in(vec![delta]) - .other_side_effects(true), - ); - - let Offset = &Operand::new("Offset", &imm.imm64).with_doc("Offset from current stack pointer"); - - ig.push( - Inst::new( - "adjust_sp_up_imm", - r#" - Adds ``Offset`` immediate offset value to the stack pointer register. - - This instruction is used to adjust the stack pointer, primarily in function - prologues and epilogues. ``Offset`` is constrained to the size of a signed - 32-bit integer. - "#, - &formats.unary_imm, - ) - .operands_in(vec![Offset]) - .other_side_effects(true), - ); - - let Offset = &Operand::new("Offset", &imm.imm64).with_doc("Offset from current stack pointer"); - - ig.push( - Inst::new( - "adjust_sp_down_imm", - r#" - Subtracts ``Offset`` immediate offset value from the stack pointer - register. - - This instruction is used to adjust the stack pointer, primarily in function - prologues and epilogues. ``Offset`` is constrained to the size of a signed - 32-bit integer. - "#, - &formats.unary_imm, - ) - .operands_in(vec![Offset]) - .other_side_effects(true), - ); - - let f = &Operand::new("f", iflags); - - ig.push( - Inst::new( - "ifcmp_sp", - r#" - Compare ``addr`` with the stack pointer and set the CPU flags. - - This is like `ifcmp` where ``addr`` is the LHS operand and the stack - pointer is the RHS. - "#, - &formats.unary, - ) - .operands_in(vec![addr]) - .operands_out(vec![f]), - ); - - let N = - &Operand::new("args", &entities.varargs).with_doc("Variable number of args for StackMap"); - - ig.push( - Inst::new( - "safepoint", - r#" - This instruction will provide live reference values at a point in - the function. It can only be used by the compiler. - "#, - &formats.multiary, - ) - .operands_in(vec![N]) - .other_side_effects(true), - ); - let x = &Operand::new("x", TxN).with_doc("Vector to split"); let lo = &Operand::new("lo", &TxN.half_vector()).with_doc("Low-numbered lanes of `x`"); let hi = &Operand::new("hi", &TxN.half_vector()).with_doc("High-numbered lanes of `x`"); diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index dedc37eda8..8fa9115d79 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -2072,10 +2072,6 @@ pub(crate) fn lower_insn_to_regs>( }); } - Opcode::Safepoint => { - panic!("safepoint instructions not used by new backend's safepoints!"); - } - Opcode::Trapz | Opcode::Trapnz | Opcode::ResumableTrapnz => { panic!("trapz / trapnz / resumable_trapnz should have been removed by legalization!"); } diff --git a/cranelift/codegen/src/isa/s390x/lower.rs b/cranelift/codegen/src/isa/s390x/lower.rs index 379b31879d..fe96631ccb 100644 --- a/cranelift/codegen/src/isa/s390x/lower.rs +++ b/cranelift/codegen/src/isa/s390x/lower.rs @@ -2888,17 +2888,6 @@ fn lower_insn_to_regs>( Opcode::Isplit | Opcode::Iconcat => unimplemented!("Wide integer ops not implemented."), - Opcode::Spill - | Opcode::Fill - | Opcode::FillNop - | Opcode::CopyNop - | Opcode::AdjustSpDown - | Opcode::AdjustSpUpImm - | Opcode::AdjustSpDownImm - | Opcode::IfcmpSp => { - panic!("Unused opcode should not be encountered."); - } - Opcode::Ifcmp | Opcode::Ffcmp | Opcode::Trapff @@ -2918,11 +2907,6 @@ fn lower_insn_to_regs>( panic!("Branch opcode reached non-branch lowering logic!"); } - - Opcode::Safepoint => { - panic!("safepoint instructions not used by new backend's safepoints!"); - } - Opcode::IaddImm | Opcode::ImulImm | Opcode::UdivImm diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index 458d412899..78cbeb6272 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -6858,19 +6858,7 @@ fn lower_insn_to_regs>( panic!("table_addr should have been removed by legalization!"); } - Opcode::Safepoint => { - panic!("safepoint instructions not used by new backend's safepoints!"); - } - - Opcode::Spill - | Opcode::Fill - | Opcode::FillNop - | Opcode::CopyNop - | Opcode::AdjustSpDown - | Opcode::AdjustSpUpImm - | Opcode::AdjustSpDownImm - | Opcode::IfcmpSp - | Opcode::Copy => { + Opcode::Copy => { panic!("Unused opcode should not be encountered."); } @@ -7085,23 +7073,6 @@ impl LowerBackend for X64Backend { let cond_code = emit_cmp(ctx, ifcmp, cond_code); let cc = CC::from_intcc(cond_code); ctx.emit(Inst::jmp_cond(cc, taken, not_taken)); - } else if let Some(ifcmp_sp) = matches_input(ctx, flag_input, Opcode::IfcmpSp) { - let operand = put_input_in_reg( - ctx, - InsnInput { - insn: ifcmp_sp, - input: 0, - }, - ); - let ty = ctx.input_ty(ifcmp_sp, 0); - ctx.emit(Inst::cmp_rmi_r( - OperandSize::from_ty(ty), - RegMemImm::reg(regs::rsp()), - operand, - )); - let cond_code = ctx.data(branches[0]).cond_code().unwrap(); - let cc = CC::from_intcc(cond_code); - ctx.emit(Inst::jmp_cond(cc, taken, not_taken)); } else { // Should be disallowed by flags checks in verifier. unimplemented!("Brif with non-ifcmp input"); diff --git a/cranelift/codegen/src/peepmatic.rs b/cranelift/codegen/src/peepmatic.rs index a7e37c9ba0..7281596eaf 100644 --- a/cranelift/codegen/src/peepmatic.rs +++ b/cranelift/codegen/src/peepmatic.rs @@ -26,14 +26,6 @@ use std::sync::atomic::{AtomicPtr, Ordering}; peepmatic_traits::define_parse_and_typing_rules_for_operator! { Opcode { - adjust_sp_down => AdjustSpDown { - parameters(iNN); - result(void); - } - adjust_sp_down_imm => AdjustSpDownImm { - immediates(iNN); - result(void); - } band => Band { parameters(iNN, iNN); result(iNN); @@ -888,10 +880,6 @@ unsafe impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa { } InstructionData::Unary { - opcode: opcode @ Opcode::AdjustSpDown, - arg, - } - | InstructionData::Unary { opcode: opcode @ Opcode::Bint, arg, } @@ -920,10 +908,6 @@ unsafe impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa { } InstructionData::UnaryImm { - opcode: opcode @ Opcode::AdjustSpDownImm, - imm, - } - | InstructionData::UnaryImm { opcode: opcode @ Opcode::Iconst, imm, } => { @@ -949,15 +933,6 @@ unsafe impl<'a, 'b> InstructionSet<'b> for &'a dyn TargetIsa { let root = root.resolve_inst(&pos.func.dfg).unwrap(); match operator { - Opcode::AdjustSpDown => { - let a = part_to_value(pos, root, a).unwrap(); - pos.ins().adjust_sp_down(a).into() - } - Opcode::AdjustSpDownImm => { - let c = a.unwrap_constant(); - let imm = Imm64::try_from(c).unwrap(); - pos.ins().adjust_sp_down_imm(imm).into() - } Opcode::Bconst => { let c = a.unwrap_constant(); let val = const_to_value(pos.ins(), c, root); diff --git a/cranelift/codegen/src/preopt.peepmatic b/cranelift/codegen/src/preopt.peepmatic index cd988ff7d9..b032089492 100644 --- a/cranelift/codegen/src/preopt.peepmatic +++ b/cranelift/codegen/src/preopt.peepmatic @@ -82,9 +82,6 @@ (fits-in-native-word $C)) (irsub_imm $C $x)) -;; Unary instructions whose operand is constant. -(=> (adjust_sp_down $C) (adjust_sp_down_imm $C)) - ;; Fold `(binop_imm $C1 (binop_imm $C2 $x))` into `(binop_imm $(binop $C2 $C1) $x)`. (=> (iadd_imm $C1 (iadd_imm $C2 $x)) (iadd_imm $(iadd $C1 $C2) $x)) (=> (imul_imm $C1 (imul_imm $C2 $x)) (imul_imm $(imul $C1 $C2) $x)) diff --git a/cranelift/codegen/src/simple_preopt.rs b/cranelift/codegen/src/simple_preopt.rs index 6903d18f6c..0beda58c1a 100644 --- a/cranelift/codegen/src/simple_preopt.rs +++ b/cranelift/codegen/src/simple_preopt.rs @@ -809,15 +809,6 @@ mod simplify { } } - InstructionData::Unary { opcode, arg } => { - if let Opcode::AdjustSpDown = opcode { - if let Some(imm) = resolve_imm64_value(&pos.func.dfg, arg) { - // Note this works for both positive and negative immediate values. - pos.func.dfg.replace(inst).adjust_sp_down_imm(imm); - } - } - } - InstructionData::BinaryImm64 { opcode, arg, imm } => { let ty = pos.func.dfg.ctrl_typevar(inst); diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 777c817a7b..f63c45b924 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -65,8 +65,8 @@ use crate::ir; use crate::ir::entities::AnyEntity; use crate::ir::instructions::{BranchInfo, CallInfo, InstructionFormat, ResolvedConstraint}; use crate::ir::{ - types, ArgumentPurpose, Block, Constant, FuncRef, Function, GlobalValue, Inst, InstructionData, - JumpTable, Opcode, SigRef, StackSlot, Type, Value, ValueDef, ValueList, + types, ArgumentPurpose, Block, Constant, FuncRef, Function, GlobalValue, Inst, JumpTable, + Opcode, SigRef, StackSlot, Type, Value, ValueDef, ValueList, }; use crate::isa::TargetIsa; use crate::iterators::IteratorExtras; @@ -1216,9 +1216,6 @@ impl<'a> Verifier<'a> { let _ = self.typecheck_return(inst, errors); let _ = self.typecheck_special(inst, ctrl_type, errors); - // Misuses of copy_nop instructions are fatal - self.typecheck_copy_nop(inst, errors)?; - Ok(()) } @@ -1550,36 +1547,6 @@ impl<'a> Verifier<'a> { Ok(()) } - fn typecheck_copy_nop( - &self, - inst: Inst, - errors: &mut VerifierErrors, - ) -> VerifierStepResult<()> { - if let InstructionData::Unary { - opcode: Opcode::CopyNop, - arg, - } = self.func.dfg[inst] - { - let dst_vals = self.func.dfg.inst_results(inst); - if dst_vals.len() != 1 { - return errors.fatal(( - inst, - self.context(inst), - "copy_nop must produce exactly one result", - )); - } - let dst_val = dst_vals[0]; - if self.func.dfg.value_type(dst_val) != self.func.dfg.value_type(arg) { - return errors.fatal(( - inst, - self.context(inst), - "copy_nop src and dst types must be the same", - )); - } - } - Ok(()) - } - fn cfg_integrity( &self, cfg: &ControlFlowGraph, @@ -1701,24 +1668,6 @@ impl<'a> Verifier<'a> { } } - fn verify_safepoint_unused( - &self, - inst: Inst, - errors: &mut VerifierErrors, - ) -> VerifierStepResult<()> { - if let Some(isa) = self.isa { - if !isa.flags().enable_safepoints() && self.func.dfg[inst].opcode() == Opcode::Safepoint - { - return errors.fatal(( - inst, - self.context(inst), - "safepoint instruction cannot be used when it is not enabled.", - )); - } - } - Ok(()) - } - fn typecheck_function_signature(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { self.func .signature @@ -1782,7 +1731,6 @@ impl<'a> Verifier<'a> { for inst in self.func.layout.block_insts(block) { self.block_integrity(block, inst, errors)?; self.instruction_integrity(inst, errors)?; - self.verify_safepoint_unused(inst, errors)?; self.typecheck(inst, errors)?; self.immediate_constraints(inst, errors)?; } diff --git a/cranelift/filetests/filetests/simple_gvn/reject.clif b/cranelift/filetests/filetests/simple_gvn/reject.clif index bb01fe5839..3a5c1e0ee3 100644 --- a/cranelift/filetests/filetests/simple_gvn/reject.clif +++ b/cranelift/filetests/filetests/simple_gvn/reject.clif @@ -29,18 +29,3 @@ block0: ; check: v5 = trueif eq v4 return v6 } - -function %spill() -> i32 { -block0: - v0 = iconst.i32 7 - v1 = spill v0 - v2 = fill v1 - v3 = spill v0 - v4 = fill v1 - v5 = bor v2, v4 -; check: v1 = spill v0 -; check: v2 = fill v1 -; check: v3 = spill v0 -; check: v4 = fill v1 - return v5 -} diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 986dca0e50..60ab37dc67 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -448,15 +448,6 @@ where assign(Value::or(mask_a, mask_b)?) } Opcode::Copy => assign(arg(0)?), - Opcode::Spill => unimplemented!("Spill"), - Opcode::Fill => unimplemented!("Fill"), - Opcode::FillNop => assign(arg(0)?), - Opcode::CopyNop => unimplemented!("CopyNop"), - Opcode::AdjustSpDown => unimplemented!("AdjustSpDown"), - Opcode::AdjustSpUpImm => unimplemented!("AdjustSpUpImm"), - Opcode::AdjustSpDownImm => unimplemented!("AdjustSpDownImm"), - Opcode::IfcmpSp => unimplemented!("IfcmpSp"), - Opcode::Safepoint => unimplemented!("Safepoint"), Opcode::Icmp => assign(icmp( ctrl_ty, inst.cond_code().unwrap(), diff --git a/cranelift/peepmatic/crates/fuzzing/src/interp.rs b/cranelift/peepmatic/crates/fuzzing/src/interp.rs index 3cd7aa9280..b2d5355fc4 100644 --- a/cranelift/peepmatic/crates/fuzzing/src/interp.rs +++ b/cranelift/peepmatic/crates/fuzzing/src/interp.rs @@ -324,15 +324,6 @@ mod tests { ); } - #[test] - fn regression_8() { - interp( - b" - (=> (adjust_sp_down $C) (adjust_sp_down_imm $C)) - ", - ); - } - #[test] fn regression_9() { interp( diff --git a/cranelift/peepmatic/crates/test-operator/src/lib.rs b/cranelift/peepmatic/crates/test-operator/src/lib.rs index cff0ad5876..b3dff3c9eb 100644 --- a/cranelift/peepmatic/crates/test-operator/src/lib.rs +++ b/cranelift/peepmatic/crates/test-operator/src/lib.rs @@ -6,14 +6,6 @@ peepmatic_traits::define_operator! { /// A `TOperator` type for use inside tests. TestOperator { - adjust_sp_down => AdjustSpDown { - parameters(iNN); - result(void); - } - adjust_sp_down_imm => AdjustSpDownImm { - immediates(iNN); - result(void); - } band => Band { parameters(iNN, iNN); result(iNN);