From e913cf36479ef7d1c7e5c26e52bc38463b3057b9 Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Fri, 9 Dec 2022 22:42:03 +0100 Subject: [PATCH] Remove IFLAGS/FFLAGS types (#5406) All instructions using the CPU flags types (IFLAGS/FFLAGS) were already removed. This patch completes the cleanup by removing all remaining instructions that define values of CPU flags types, as well as the types themselves. Specifically, the following features are removed: - The IFLAGS and FFLAGS types and the SpecialType category. - Special handling of IFLAGS and FFLAGS in machinst/isle.rs and machinst/lower.rs. - The ifcmp, ifcmp_imm, ffcmp, iadd_ifcin, iadd_ifcout, iadd_ifcarry, isub_ifbin, isub_ifbout, and isub_ifborrow instructions. - The writes_cpu_flags instruction property. - The flags verifier pass. - Flags handling in the interpreter. All of these features are currently unused; no functional change intended by this patch. This addresses https://github.com/bytecodealliance/wasmtime/issues/3249. --- .../codegen/meta/src/cdsl/instructions.rs | 6 - cranelift/codegen/meta/src/cdsl/operands.rs | 11 - cranelift/codegen/meta/src/cdsl/types.rs | 102 ---------- cranelift/codegen/meta/src/cdsl/typevar.rs | 45 +---- cranelift/codegen/meta/src/gen_inst.rs | 10 - cranelift/codegen/meta/src/gen_types.rs | 5 - .../codegen/meta/src/shared/instructions.rs | 189 ------------------ cranelift/codegen/meta/src/shared/types.rs | 43 ---- cranelift/codegen/src/ir/dfg.rs | 7 +- cranelift/codegen/src/ir/types.rs | 27 +-- cranelift/codegen/src/isa/aarch64/inst.isle | 8 - cranelift/codegen/src/isa/aarch64/inst/mod.rs | 3 +- cranelift/codegen/src/isa/aarch64/lower.isle | 30 --- .../codegen/src/isa/aarch64/lower_inst.rs | 18 +- cranelift/codegen/src/isa/riscv64/inst.isle | 4 - cranelift/codegen/src/isa/riscv64/inst/mod.rs | 4 +- cranelift/codegen/src/isa/riscv64/lower.isle | 13 -- cranelift/codegen/src/isa/s390x/inst/mod.rs | 3 - cranelift/codegen/src/isa/s390x/lower.isle | 53 +---- cranelift/codegen/src/isa/s390x/lower.rs | 14 +- cranelift/codegen/src/isa/x64/inst/mod.rs | 1 - cranelift/codegen/src/isa/x64/lower.isle | 46 ----- cranelift/codegen/src/isa/x64/lower.rs | 18 +- cranelift/codegen/src/legalizer/mod.rs | 7 +- cranelift/codegen/src/licm.rs | 1 - cranelift/codegen/src/machinst/isle.rs | 14 +- cranelift/codegen/src/machinst/lower.rs | 8 +- cranelift/codegen/src/simple_gvn.rs | 1 - cranelift/codegen/src/simple_preopt.rs | 1 - cranelift/codegen/src/timing.rs | 1 - cranelift/codegen/src/verifier/flags.rs | 161 --------------- cranelift/codegen/src/verifier/mod.rs | 5 - cranelift/docs/ir.md | 18 -- .../filetests/isa/s390x/arithmetic.clif | 101 ---------- .../isa/s390x/uadd_overflow_trap.clif | 11 + .../filetests/filetests/licm/reject.clif | 23 --- .../filetests/filetests/parser/ternary.clif | 31 +-- cranelift/fuzzgen/src/function_generator.rs | 1 - cranelift/interpreter/src/interpreter.rs | 38 ---- cranelift/interpreter/src/state.rs | 26 --- cranelift/interpreter/src/step.rs | 58 ------ cranelift/reader/src/lexer.rs | 8 +- 42 files changed, 55 insertions(+), 1119 deletions(-) delete mode 100644 cranelift/codegen/src/verifier/flags.rs diff --git a/cranelift/codegen/meta/src/cdsl/instructions.rs b/cranelift/codegen/meta/src/cdsl/instructions.rs index bb6af7f692..48e44c6f67 100644 --- a/cranelift/codegen/meta/src/cdsl/instructions.rs +++ b/cranelift/codegen/meta/src/cdsl/instructions.rs @@ -73,8 +73,6 @@ pub(crate) struct InstructionContent { pub can_trap: bool, /// Does this instruction have other side effects besides can_* flags? pub other_side_effects: bool, - /// Does this instruction write to CPU flags? - pub writes_cpu_flags: bool, } impl InstructionContent { @@ -240,9 +238,6 @@ impl InstructionBuilder { let polymorphic_info = verify_polymorphic(&operands_in, &operands_out, &self.format, &value_opnums); - // Infer from output operands whether an instruction clobbers CPU flags or not. - let writes_cpu_flags = operands_out.iter().any(|op| op.is_cpu_flags()); - let camel_name = camel_case(&self.name); Rc::new(InstructionContent { @@ -264,7 +259,6 @@ impl InstructionBuilder { can_store: self.can_store, can_trap: self.can_trap, other_side_effects: self.other_side_effects, - writes_cpu_flags, }) } } diff --git a/cranelift/codegen/meta/src/cdsl/operands.rs b/cranelift/codegen/meta/src/cdsl/operands.rs index c278617b85..8420c62158 100644 --- a/cranelift/codegen/meta/src/cdsl/operands.rs +++ b/cranelift/codegen/meta/src/cdsl/operands.rs @@ -87,17 +87,6 @@ impl Operand { _ => false, } } - - pub fn is_cpu_flags(&self) -> bool { - match &self.kind.fields { - OperandKindFields::TypeVar(type_var) - if type_var.name == "iflags" || type_var.name == "fflags" => - { - true - } - _ => false, - } - } } pub type EnumValues = HashMap<&'static str, &'static str>; diff --git a/cranelift/codegen/meta/src/cdsl/types.rs b/cranelift/codegen/meta/src/cdsl/types.rs index b8681e619a..661ed2c957 100644 --- a/cranelift/codegen/meta/src/cdsl/types.rs +++ b/cranelift/codegen/meta/src/cdsl/types.rs @@ -18,7 +18,6 @@ static RUST_NAME_PREFIX: &str = "ir::types::"; pub(crate) enum ValueType { Lane(LaneType), Reference(ReferenceType), - Special(SpecialType), Vector(VectorType), DynamicVector(DynamicVectorType), } @@ -29,11 +28,6 @@ impl ValueType { LaneTypeIterator::new() } - /// Iterate through all of the special types (neither lanes nor vectors). - pub fn all_special_types() -> SpecialTypeIterator { - SpecialTypeIterator::new() - } - pub fn all_reference_types() -> ReferenceTypeIterator { ReferenceTypeIterator::new() } @@ -43,7 +37,6 @@ impl ValueType { match *self { ValueType::Lane(l) => l.doc(), ValueType::Reference(r) => r.doc(), - ValueType::Special(s) => s.doc(), ValueType::Vector(ref v) => v.doc(), ValueType::DynamicVector(ref v) => v.doc(), } @@ -54,7 +47,6 @@ impl ValueType { match *self { ValueType::Lane(l) => l.lane_bits(), ValueType::Reference(r) => r.lane_bits(), - ValueType::Special(s) => s.lane_bits(), ValueType::Vector(ref v) => v.lane_bits(), ValueType::DynamicVector(ref v) => v.lane_bits(), } @@ -78,7 +70,6 @@ impl ValueType { match *self { ValueType::Lane(l) => l.number(), ValueType::Reference(r) => r.number(), - ValueType::Special(s) => s.number(), ValueType::Vector(ref v) => v.number(), ValueType::DynamicVector(ref v) => v.number(), } @@ -100,7 +91,6 @@ impl fmt::Display for ValueType { match *self { ValueType::Lane(l) => l.fmt(f), ValueType::Reference(r) => r.fmt(f), - ValueType::Special(s) => s.fmt(f), ValueType::Vector(ref v) => v.fmt(f), ValueType::DynamicVector(ref v) => v.fmt(f), } @@ -121,13 +111,6 @@ impl From for ValueType { } } -/// Create a ValueType from a given special type. -impl From for ValueType { - fn from(spec: SpecialType) -> Self { - ValueType::Special(spec) - } -} - /// Create a ValueType from a given vector type. impl From for ValueType { fn from(vector: VectorType) -> Self { @@ -436,91 +419,6 @@ impl fmt::Debug for DynamicVectorType { } } -/// A concrete scalar type that is neither a vector nor a lane type. -/// -/// Special types cannot be used to form vectors. -#[derive(Clone, Copy, PartialEq, Eq, Hash)] -pub(crate) enum SpecialType { - Flag(shared_types::Flag), -} - -impl SpecialType { - /// Return a string containing the documentation comment for this special type. - pub fn doc(self) -> String { - match self { - SpecialType::Flag(shared_types::Flag::IFlags) => String::from( - "CPU flags representing the result of an integer comparison. These flags - can be tested with an :type:`intcc` condition code.", - ), - SpecialType::Flag(shared_types::Flag::FFlags) => String::from( - "CPU flags representing the result of a floating point comparison. These - flags can be tested with a :type:`floatcc` condition code.", - ), - } - } - - /// Return the number of bits in a lane. - pub fn lane_bits(self) -> u64 { - match self { - SpecialType::Flag(_) => 0, - } - } - - /// Find the unique number associated with this special type. - pub fn number(self) -> u16 { - match self { - SpecialType::Flag(shared_types::Flag::IFlags) => 1, - SpecialType::Flag(shared_types::Flag::FFlags) => 2, - } - } -} - -impl fmt::Display for SpecialType { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - SpecialType::Flag(shared_types::Flag::IFlags) => write!(f, "iflags"), - SpecialType::Flag(shared_types::Flag::FFlags) => write!(f, "fflags"), - } - } -} - -impl fmt::Debug for SpecialType { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "{}", - match *self { - SpecialType::Flag(_) => format!("FlagsType({})", self), - } - ) - } -} - -impl From for SpecialType { - fn from(f: shared_types::Flag) -> Self { - SpecialType::Flag(f) - } -} - -pub(crate) struct SpecialTypeIterator { - flag_iter: shared_types::FlagIterator, -} - -impl SpecialTypeIterator { - fn new() -> Self { - Self { - flag_iter: shared_types::FlagIterator::new(), - } - } -} - -impl Iterator for SpecialTypeIterator { - type Item = SpecialType; - fn next(&mut self) -> Option { - self.flag_iter.next().map(SpecialType::from) - } -} - /// Reference type is scalar type, but not lane type. #[derive(Clone, Copy, PartialEq, Eq, Hash)] pub(crate) struct ReferenceType(pub shared_types::Reference); diff --git a/cranelift/codegen/meta/src/cdsl/typevar.rs b/cranelift/codegen/meta/src/cdsl/typevar.rs index 1c307fccdf..60cf685f23 100644 --- a/cranelift/codegen/meta/src/cdsl/typevar.rs +++ b/cranelift/codegen/meta/src/cdsl/typevar.rs @@ -6,7 +6,7 @@ use std::iter::FromIterator; use std::ops; use std::rc::Rc; -use crate::cdsl::types::{LaneType, ReferenceType, SpecialType, ValueType}; +use crate::cdsl::types::{LaneType, ReferenceType, ValueType}; const MAX_LANES: u16 = 256; const MAX_BITS: u16 = 128; @@ -57,9 +57,6 @@ impl TypeVar { let mut builder = TypeSetBuilder::new(); let (scalar_type, num_lanes) = match value_type { - ValueType::Special(special_type) => { - return TypeVar::new(name, doc, builder.specials(vec![special_type]).build()); - } ValueType::Reference(ReferenceType(reference_type)) => { let bits = reference_type as RangeBound; return TypeVar::new(name, doc, builder.refs(bits..bits).build()); @@ -156,7 +153,6 @@ impl TypeVar { let ts = self.get_typeset(); // Safety checks to avoid over/underflows. - assert!(ts.specials.is_empty(), "can't derive from special types"); match derived_func { DerivedFunc::HalfWidth => { assert!( @@ -364,9 +360,6 @@ pub(crate) struct TypeVarParent { /// - The permitted range of boolean types. /// /// The ranges are inclusive from smallest bit-width to largest bit-width. -/// -/// Finally, a type set can contain special types (derived from `SpecialType`) -/// which can't appear as lane types. type RangeBound = u16; type Range = ops::Range; @@ -385,7 +378,6 @@ pub(crate) struct TypeSet { pub ints: NumSet, pub floats: NumSet, pub refs: NumSet, - pub specials: Vec, } impl TypeSet { @@ -395,7 +387,6 @@ impl TypeSet { ints: NumSet, floats: NumSet, refs: NumSet, - specials: Vec, ) -> Self { Self { lanes, @@ -403,7 +394,6 @@ impl TypeSet { ints, floats, refs, - specials, } } @@ -411,7 +401,6 @@ impl TypeSet { pub fn size(&self) -> usize { self.lanes.len() * (self.ints.len() + self.floats.len() + self.refs.len()) + self.dynamic_lanes.len() * (self.ints.len() + self.floats.len() + self.refs.len()) - + self.specials.len() } /// Return the image of self across the derived function func. @@ -450,7 +439,6 @@ impl TypeSet { let mut copy = self.clone(); copy.ints = NumSet::from_iter(self.ints.iter().filter(|&&x| x > 8).map(|&x| x / 2)); copy.floats = NumSet::from_iter(self.floats.iter().filter(|&&x| x > 32).map(|&x| x / 2)); - copy.specials = Vec::new(); copy } @@ -464,7 +452,6 @@ impl TypeSet { .filter(|&&x| x < MAX_FLOAT_BITS) .map(|&x| x * 2), ); - copy.specials = Vec::new(); copy } @@ -472,7 +459,6 @@ impl TypeSet { fn half_vector(&self) -> TypeSet { let mut copy = self.clone(); copy.lanes = NumSet::from_iter(self.lanes.iter().filter(|&&x| x > 1).map(|&x| x / 2)); - copy.specials = Vec::new(); copy } @@ -485,7 +471,6 @@ impl TypeSet { .filter(|&&x| x < MAX_LANES) .map(|&x| x * 2), ); - copy.specials = Vec::new(); copy } @@ -497,7 +482,6 @@ impl TypeSet { .filter(|&&x| x < MAX_LANES) .map(|&x| x), ); - copy.specials = Vec::new(); copy.dynamic_lanes = NumSet::new(); copy } @@ -523,9 +507,6 @@ impl TypeSet { ret.push(LaneType::float_from_bits(bits).to_dynamic(num_lanes)); } } - for &special in &self.specials { - ret.push(special.into()); - } ret } @@ -572,12 +553,6 @@ impl fmt::Debug for TypeSet { Vec::from_iter(self.refs.iter().map(|x| x.to_string())).join(", ") )); } - if !self.specials.is_empty() { - subsets.push(format!( - "specials={{{}}}", - Vec::from_iter(self.specials.iter().map(|x| x.to_string())).join(", ") - )); - } write!(fmt, "{})", subsets.join(", "))?; Ok(()) @@ -591,7 +566,6 @@ pub(crate) struct TypeSetBuilder { includes_scalars: bool, simd_lanes: Interval, dynamic_simd_lanes: Interval, - specials: Vec, } impl TypeSetBuilder { @@ -603,7 +577,6 @@ impl TypeSetBuilder { includes_scalars: true, simd_lanes: Interval::None, dynamic_simd_lanes: Interval::None, - specials: Vec::new(), } } @@ -636,11 +609,6 @@ impl TypeSetBuilder { self.dynamic_simd_lanes = interval.into(); self } - pub fn specials(mut self, specials: Vec) -> Self { - assert!(self.specials.is_empty()); - self.specials = specials; - self - } pub fn build(self) -> TypeSet { let min_lanes = if self.includes_scalars { 1 } else { 2 }; @@ -651,7 +619,6 @@ impl TypeSetBuilder { range_to_set(self.ints.to_range(8..MAX_BITS, None)), range_to_set(self.floats.to_range(32..64, None)), range_to_set(self.refs.to_range(32..64, None)), - self.specials, ) } } @@ -721,13 +688,11 @@ fn test_typevar_builder() { assert_eq!(type_set.lanes, num_set![1]); assert!(type_set.floats.is_empty()); assert_eq!(type_set.ints, num_set![8, 16, 32, 64, 128]); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new().floats(Interval::All).build(); assert_eq!(type_set.lanes, num_set![1]); assert_eq!(type_set.floats, num_set![32, 64]); assert!(type_set.ints.is_empty()); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new() .floats(Interval::All) @@ -737,7 +702,6 @@ fn test_typevar_builder() { assert_eq!(type_set.lanes, num_set![2, 4, 8, 16, 32, 64, 128, 256]); assert_eq!(type_set.floats, num_set![32, 64]); assert!(type_set.ints.is_empty()); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new() .floats(Interval::All) @@ -747,7 +711,6 @@ fn test_typevar_builder() { assert_eq!(type_set.lanes, num_set![1, 2, 4, 8, 16, 32, 64, 128, 256]); assert_eq!(type_set.floats, num_set![32, 64]); assert!(type_set.ints.is_empty()); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new() .floats(Interval::All) @@ -758,7 +721,6 @@ fn test_typevar_builder() { assert_eq!(type_set.floats, num_set![32, 64]); assert!(type_set.dynamic_lanes.is_empty()); assert!(type_set.ints.is_empty()); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new() .ints(Interval::All) @@ -773,7 +735,6 @@ fn test_typevar_builder() { assert_eq!(type_set.ints, num_set![8, 16, 32, 64, 128]); assert_eq!(type_set.floats, num_set![32, 64]); assert_eq!(type_set.lanes, num_set![1]); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new() .floats(Interval::All) @@ -787,13 +748,11 @@ fn test_typevar_builder() { assert_eq!(type_set.floats, num_set![32, 64]); assert_eq!(type_set.lanes, num_set![1]); assert!(type_set.ints.is_empty()); - assert!(type_set.specials.is_empty()); let type_set = TypeSetBuilder::new().ints(16..64).build(); assert_eq!(type_set.lanes, num_set![1]); assert_eq!(type_set.ints, num_set![16, 32, 64]); assert!(type_set.floats.is_empty()); - assert!(type_set.specials.is_empty()); } #[test] @@ -979,7 +938,6 @@ fn test_typevar_singleton() { assert_eq!(typevar.name, "i32"); assert_eq!(typevar.type_set.ints, num_set![32]); assert!(typevar.type_set.floats.is_empty()); - assert!(typevar.type_set.specials.is_empty()); assert_eq!(typevar.type_set.lanes, num_set![1]); // Test f32x4. @@ -991,5 +949,4 @@ fn test_typevar_singleton() { assert!(typevar.type_set.ints.is_empty()); assert_eq!(typevar.type_set.floats, num_set![32]); assert_eq!(typevar.type_set.lanes, num_set![4]); - assert!(typevar.type_set.specials.is_empty()); } diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index 5e9f0abfef..1287a2c3ab 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -519,13 +519,6 @@ fn gen_opcodes(all_inst: &AllInstructions, fmt: &mut Formatter) { "Does this instruction have other side effects besides can_* flags?", fmt, ); - gen_bool_accessor( - all_inst, - |inst| inst.writes_cpu_flags, - "writes_cpu_flags", - "Does this instruction write to CPU flags?", - fmt, - ); }); fmt.line("}"); fmt.empty_line(); @@ -652,9 +645,6 @@ fn typeset_to_string(ts: &TypeSet) -> String { if !ts.floats.is_empty() { result += &format!(", floats={}", iterable_to_string(&ts.floats)); } - if !ts.specials.is_empty() { - result += &format!(", specials=[{}]", iterable_to_string(&ts.specials)); - } if !ts.refs.is_empty() { result += &format!(", refs={}", iterable_to_string(&ts.refs)); } diff --git a/cranelift/codegen/meta/src/gen_types.rs b/cranelift/codegen/meta/src/gen_types.rs index 0d27070df7..f83638fd7f 100644 --- a/cranelift/codegen/meta/src/gen_types.rs +++ b/cranelift/codegen/meta/src/gen_types.rs @@ -48,11 +48,6 @@ fn emit_dynamic_vectors(bits: u64, fmt: &mut srcgen::Formatter) { /// Emit types using the given formatter object. fn emit_types(fmt: &mut srcgen::Formatter) { - // Emit all of the special types, such as types for CPU flags. - for spec in cdsl_types::ValueType::all_special_types().map(cdsl_types::ValueType::from) { - emit_type(&spec, fmt); - } - // Emit all of the lane types, such integers, floats, and booleans. for ty in cdsl_types::ValueType::all_lane_types().map(cdsl_types::ValueType::from) { emit_type(&ty, fmt); diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 91a3099f50..3ff997c18c 100755 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -571,9 +571,6 @@ pub(crate) fn define( define_simd_arithmetic(&mut ig, formats, imm, entities); // Operand kind shorthands. - let iflags: &TypeVar = &ValueType::Special(types::Flag::IFlags.into()).into(); - let fflags: &TypeVar = &ValueType::Special(types::Flag::FFlags.into()).into(); - let i8: &TypeVar = &ValueType::from(LaneType::from(types::Int::I8)).into(); let f32_: &TypeVar = &ValueType::from(LaneType::from(types::Float::F32)).into(); let f64_: &TypeVar = &ValueType::from(LaneType::from(types::Float::F64)).into(); @@ -1671,40 +1668,6 @@ pub(crate) fn define( .operands_out(vec![a]), ); - let f = &Operand::new("f", iflags); - let x = &Operand::new("x", iB); - let y = &Operand::new("y", iB); - - ig.push( - Inst::new( - "ifcmp", - r#" - Compare scalar integers and return flags. - - Compare two scalar integer values and return integer CPU flags - representing the result. - "#, - &formats.binary, - ) - .operands_in(vec![x, y]) - .operands_out(vec![f]), - ); - - ig.push( - Inst::new( - "ifcmp_imm", - r#" - Compare scalar integer to a constant and return flags. - - Like `icmp_imm`, but returns integer CPU flags instead of testing - a specific condition code. - "#, - &formats.binary_imm64, - ) - .operands_in(vec![x, Y]) - .operands_out(vec![f]), - ); - let a = &Operand::new("a", Int); let x = &Operand::new("x", Int); let y = &Operand::new("y", Int); @@ -2043,11 +2006,6 @@ pub(crate) fn define( let b_in = &Operand::new("b_in", i8).with_doc("Input borrow flag"); let b_out = &Operand::new("b_out", i8).with_doc("Output borrow flag"); - let c_if_in = &Operand::new("c_in", iflags); - let c_if_out = &Operand::new("c_out", iflags); - let b_if_in = &Operand::new("b_in", iflags); - let b_if_out = &Operand::new("b_out", iflags); - ig.push( Inst::new( "iadd_cin", @@ -2069,27 +2027,6 @@ pub(crate) fn define( .operands_out(vec![a]), ); - ig.push( - Inst::new( - "iadd_ifcin", - r#" - Add integers with carry in. - - Same as `iadd` with an additional carry flag input. Computes: - - ```text - a = x + y + c_{in} \pmod 2^B - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.ternary, - ) - .operands_in(vec![x, y, c_if_in]) - .operands_out(vec![a]), - ); - ig.push( Inst::new( "iadd_cout", @@ -2112,28 +2049,6 @@ pub(crate) fn define( .operands_out(vec![a, c_out]), ); - ig.push( - Inst::new( - "iadd_ifcout", - r#" - Add integers with carry out. - - Same as `iadd` with an additional carry flag output. - - ```text - a &= x + y \pmod 2^B \\ - c_{out} &= x+y >= 2^B - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.binary, - ) - .operands_in(vec![x, y]) - .operands_out(vec![a, c_if_out]), - ); - ig.push( Inst::new( "iadd_carry", @@ -2156,28 +2071,6 @@ pub(crate) fn define( .operands_out(vec![a, c_out]), ); - ig.push( - Inst::new( - "iadd_ifcarry", - r#" - Add integers with carry in and out. - - Same as `iadd` with an additional carry flag input and output. - - ```text - a &= x + y + c_{in} \pmod 2^B \\ - c_{out} &= x + y + c_{in} >= 2^B - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.ternary, - ) - .operands_in(vec![x, y, c_if_in]) - .operands_out(vec![a, c_if_out]), - ); - { let code = &Operand::new("code", &imm.trapcode); @@ -2227,27 +2120,6 @@ pub(crate) fn define( .operands_out(vec![a]), ); - ig.push( - Inst::new( - "isub_ifbin", - r#" - Subtract integers with borrow in. - - Same as `isub` with an additional borrow flag input. Computes: - - ```text - a = x - (y + b_{in}) \pmod 2^B - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.ternary, - ) - .operands_in(vec![x, y, b_if_in]) - .operands_out(vec![a]), - ); - ig.push( Inst::new( "isub_bout", @@ -2270,28 +2142,6 @@ pub(crate) fn define( .operands_out(vec![a, b_out]), ); - ig.push( - Inst::new( - "isub_ifbout", - r#" - Subtract integers with borrow out. - - Same as `isub` with an additional borrow flag output. - - ```text - a &= x - y \pmod 2^B \\ - b_{out} &= x < y - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.binary, - ) - .operands_in(vec![x, y]) - .operands_out(vec![a, b_if_out]), - ); - ig.push( Inst::new( "isub_borrow", @@ -2314,28 +2164,6 @@ pub(crate) fn define( .operands_out(vec![a, b_out]), ); - ig.push( - Inst::new( - "isub_ifborrow", - r#" - Subtract integers with borrow in and out. - - Same as `isub` with an additional borrow flag input and output. - - ```text - a &= x - (y + b_{in}) \pmod 2^B \\ - b_{out} &= x < y + b_{in} - ``` - - Polymorphic over all scalar integer types, but does not support vector - types. - "#, - &formats.ternary, - ) - .operands_in(vec![x, y, b_if_in]) - .operands_out(vec![a, b_if_out]), - ); - let bits = &TypeVar::new( "bits", "Any integer, float, or vector type", @@ -2848,23 +2676,6 @@ pub(crate) fn define( .operands_out(vec![a]), ); - let f = &Operand::new("f", fflags); - - ig.push( - Inst::new( - "ffcmp", - r#" - Floating point comparison returning flags. - - Compares two numbers like `fcmp`, but returns floating point CPU - flags instead of testing a specific condition. - "#, - &formats.binary, - ) - .operands_in(vec![x, y]) - .operands_out(vec![f]), - ); - let x = &Operand::new("x", Float); let y = &Operand::new("y", Float); let z = &Operand::new("z", Float); diff --git a/cranelift/codegen/meta/src/shared/types.rs b/cranelift/codegen/meta/src/shared/types.rs index 85ff018538..33efd10801 100644 --- a/cranelift/codegen/meta/src/shared/types.rs +++ b/cranelift/codegen/meta/src/shared/types.rs @@ -72,41 +72,6 @@ impl Iterator for FloatIterator { } } -/// A type representing CPU flags. -/// -/// Flags can't be stored in memory. -#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] -pub(crate) enum Flag { - /// CPU flags from an integer comparison. - IFlags, - /// CPU flags from a floating point comparison. - FFlags, -} - -/// Iterator through the variants of the Flag enum. -pub(crate) struct FlagIterator { - index: u8, -} - -impl FlagIterator { - pub fn new() -> Self { - Self { index: 0 } - } -} - -impl Iterator for FlagIterator { - type Item = Flag; - fn next(&mut self) -> Option { - let res = match self.index { - 0 => Some(Flag::IFlags), - 1 => Some(Flag::FFlags), - _ => return None, - }; - self.index += 1; - res - } -} - #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] pub(crate) enum Reference { /// 32-bit reference. @@ -162,14 +127,6 @@ mod iter_tests { assert_eq!(float_iter.next(), None); } - #[test] - fn flag_iter_works() { - let mut flag_iter = FlagIterator::new(); - assert_eq!(flag_iter.next(), Some(Flag::IFlags)); - assert_eq!(flag_iter.next(), Some(Flag::FFlags)); - assert_eq!(flag_iter.next(), None); - } - #[test] fn reference_iter_works() { let mut reference_iter = ReferenceIterator::new(); diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index 877fc2cb83..6776e3a2cf 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -1511,6 +1511,7 @@ mod tests { #[test] fn aliases() { + use crate::ir::condcodes::IntCC; use crate::ir::InstBuilder; let mut func = Function::new(); @@ -1525,7 +1526,7 @@ mod tests { assert_eq!(pos.func.dfg.resolve_aliases(v1), v1); let arg0 = pos.func.dfg.append_block_param(block0, types::I32); - let (s, c) = pos.ins().iadd_ifcout(v1, arg0); + let (s, c) = pos.ins().iadd_cout(v1, arg0); let iadd = match pos.func.dfg.value_def(s) { ValueDef::Result(i, 0) => i, _ => panic!(), @@ -1535,9 +1536,9 @@ mod tests { pos.func.dfg.clear_results(iadd); pos.func.dfg.attach_result(iadd, s); - // Replace `iadd_ifcout` with a normal `iadd` and an `ifcmp`. + // Replace `iadd_cout` with a normal `iadd` and an `icmp`. pos.func.dfg.replace(iadd).iadd(v1, arg0); - let c2 = pos.ins().ifcmp(s, v1); + let c2 = pos.ins().icmp(IntCC::Equal, s, v1); pos.func.dfg.change_to_alias(c, c2); assert_eq!(pos.func.dfg.resolve_aliases(c2), c2); diff --git a/cranelift/codegen/src/ir/types.rs b/cranelift/codegen/src/ir/types.rs index 3c5e3b4bc6..90021d9f6b 100644 --- a/cranelift/codegen/src/ir/types.rs +++ b/cranelift/codegen/src/ir/types.rs @@ -244,14 +244,6 @@ impl Type { } } - /// Is this a CPU flags type? - pub fn is_flags(self) -> bool { - match self { - IFLAGS | FFLAGS => true, - _ => false, - } - } - /// Is this a ref type? pub fn is_ref(self) -> bool { match self { @@ -453,12 +445,10 @@ impl Display for Type { } else if self.is_ref() { write!(f, "r{}", self.lane_bits()) } else { - f.write_str(match *self { - IFLAGS => "iflags", - FFLAGS => "fflags", + match *self { INVALID => panic!("INVALID encountered"), _ => panic!("Unknown Type(0x{:x})", self.0), - }) + } } } } @@ -478,8 +468,6 @@ impl Debug for Type { } else { match *self { INVALID => write!(f, "types::INVALID"), - IFLAGS => write!(f, "types::IFLAGS"), - FFLAGS => write!(f, "types::FFLAGS"), _ => write!(f, "Type(0x{:x})", self.0), } } @@ -501,10 +489,6 @@ mod tests { fn basic_scalars() { assert_eq!(INVALID, INVALID.lane_type()); assert_eq!(0, INVALID.bits()); - assert_eq!(IFLAGS, IFLAGS.lane_type()); - assert_eq!(0, IFLAGS.bits()); - assert_eq!(FFLAGS, FFLAGS.lane_type()); - assert_eq!(0, FFLAGS.bits()); assert_eq!(I8, I8.lane_type()); assert_eq!(I16, I16.lane_type()); assert_eq!(I32, I32.lane_type()); @@ -518,8 +502,6 @@ mod tests { assert_eq!(R64, R64.lane_type()); assert_eq!(INVALID.lane_bits(), 0); - assert_eq!(IFLAGS.lane_bits(), 0); - assert_eq!(FFLAGS.lane_bits(), 0); assert_eq!(I8.lane_bits(), 8); assert_eq!(I16.lane_bits(), 16); assert_eq!(I32.lane_bits(), 32); @@ -535,7 +517,6 @@ mod tests { fn typevar_functions() { assert_eq!(INVALID.half_width(), None); assert_eq!(INVALID.half_width(), None); - assert_eq!(FFLAGS.half_width(), None); assert_eq!(I8.half_width(), None); assert_eq!(I16.half_width(), Some(I8)); assert_eq!(I32.half_width(), Some(I16)); @@ -546,8 +527,6 @@ mod tests { assert_eq!(F64.half_width(), Some(F32)); assert_eq!(INVALID.double_width(), None); - assert_eq!(IFLAGS.double_width(), None); - assert_eq!(FFLAGS.double_width(), None); assert_eq!(I8.double_width(), Some(I16)); assert_eq!(I16.double_width(), Some(I32)); assert_eq!(I32.double_width(), Some(I64)); @@ -614,8 +593,6 @@ mod tests { #[test] fn format_scalars() { - assert_eq!(IFLAGS.to_string(), "iflags"); - assert_eq!(FFLAGS.to_string(), "fflags"); assert_eq!(I8.to_string(), "i8"); assert_eq!(I16.to_string(), "i16"); assert_eq!(I32.to_string(), "i32"); diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 4e281a9b09..33ef41c9f2 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -1940,14 +1940,6 @@ (MInst.AluRRR (ALUOp.AddS) (operand_size ty) dst src1 src2) dst))) -;; Helper for emitting `adds` instructions, setting flags in ambient -;; state. Used only for `iadd_ifcout`. -(decl add_with_flags (Type Reg Reg) Reg) -(rule (add_with_flags ty src1 src2) - (let ((dst WritableReg (temp_writable_reg $I64)) - (_ Unit (emit (MInst.AluRRR (ALUOp.AddS) (operand_size ty) dst src1 src2)))) - dst)) - ;; Helper for emitting `adc` instructions. (decl adc_paired (Type Reg Reg) ConsumesFlags) (rule (adc_paired ty src1 src2) diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index b54f016a88..c9c2f037f9 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -1,7 +1,7 @@ //! This module defines aarch64-specific machine instruction types. use crate::binemit::{Addend, CodeOffset, Reloc}; -use crate::ir::types::{F32, F64, FFLAGS, I128, I16, I32, I64, I8, I8X16, IFLAGS, R32, R64}; +use crate::ir::types::{F32, F64, I128, I16, I32, I64, I8, I8X16, R32, R64}; use crate::ir::{types, ExternalName, MemFlags, Opcode, Type}; use crate::isa::CallConv; use crate::machinst::*; @@ -1280,7 +1280,6 @@ impl MachInst for Inst { Ok((&[RegClass::Float], &[I8X16])) } _ if ty.is_dynamic_vector() => Ok((&[RegClass::Float], &[I8X16])), - IFLAGS | FFLAGS => Ok((&[RegClass::Int], &[I64])), _ => Err(CodegenError::Unsupported(format!( "Unexpected SSA-value type: {}", ty diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index 2bb40d1694..2a867c437b 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -2332,36 +2332,6 @@ (lower_msb Reg (lsr_imm $I64 lower_msb (imm_shift_from_u8 63)))) (add_shift $I64 lower_msb upper_msb (lshl_from_u64 $I64 1)))) -;;; Rules for `iadd_ifcout` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -;; This is a two-output instruction that is needed for the -;; legalizer's explicit heap-check sequence, among possible other -;; uses. Its second output is a flags output only ever meant to -;; check for overflow using the -;; `backend.unsigned_add_overflow_condition()` condition. -;; -;; Note that the CLIF validation will ensure that no flag-setting -;; operation comes between this IaddIfcout and its use (e.g., a -;; Trapif). Thus, we can rely on implicit communication through the -;; processor flags rather than explicitly generating flags into a -;; register. We simply use the variant of the add instruction that -;; sets flags (`adds`) here. -;; -;; Note that the second output (the flags) need not be generated, -;; because flags are never materialized into a register; the only -;; instructions that can use a value of type `iflags` or `fflags` -;; will look directly for the flags-producing instruction (which can -;; always be found, by construction) and merge it. -;; -;; Now handle the iadd as above, except use an AddS opcode that sets -;; flags. - -(rule (lower (has_type (ty_int ty) - (iadd_ifcout a b))) - (output_pair - (add_with_flags ty a b) - (invalid_reg))) - ;;; Rules for `iadd_cout` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; For values smaller than a register, we do a normal `add` with both arguments diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index f99f946418..92674f4409 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -167,14 +167,6 @@ pub(crate) fn lower_insn_to_regs( Opcode::Return => implemented_in_isle(ctx), - Opcode::Ifcmp | Opcode::Ffcmp => { - // An Ifcmp/Ffcmp must always be seen as a use of a brif/brff or trueif/trueff - // instruction. This will always be the case as long as the IR uses an Ifcmp/Ffcmp from - // the same block, or a dominating block. In other words, it cannot pass through a BB - // param (phi). The flags pass of the verifier will ensure this. - panic!("Should never reach ifcmp as isel root!"); - } - Opcode::Icmp => implemented_in_isle(ctx), Opcode::Fcmp => implemented_in_isle(ctx), @@ -253,8 +245,6 @@ pub(crate) fn lower_insn_to_regs( Opcode::FcvtToUintSat | Opcode::FcvtToSintSat => implemented_in_isle(ctx), - Opcode::IaddIfcout => implemented_in_isle(ctx), - Opcode::UaddOverflowTrap => implemented_in_isle(ctx), Opcode::IaddCout => implemented_in_isle(ctx), @@ -267,15 +257,10 @@ pub(crate) fn lower_insn_to_regs( | Opcode::SremImm | Opcode::IrsubImm | Opcode::IaddCin - | Opcode::IaddIfcin | Opcode::IaddCarry - | Opcode::IaddIfcarry | Opcode::IsubBin - | Opcode::IsubIfbin | Opcode::IsubBout - | Opcode::IsubIfbout | Opcode::IsubBorrow - | Opcode::IsubIfborrow | Opcode::BandImm | Opcode::BorImm | Opcode::BxorImm @@ -284,8 +269,7 @@ pub(crate) fn lower_insn_to_regs( | Opcode::IshlImm | Opcode::UshrImm | Opcode::SshrImm - | Opcode::IcmpImm - | Opcode::IfcmpImm => { + | Opcode::IcmpImm => { panic!("ALU+imm and ALU+carry ops should not appear here!"); } diff --git a/cranelift/codegen/src/isa/riscv64/inst.isle b/cranelift/codegen/src/isa/riscv64/inst.isle index 20461db7c3..c6b9e1b794 100644 --- a/cranelift/codegen/src/isa/riscv64/inst.isle +++ b/cranelift/codegen/src/isa/riscv64/inst.isle @@ -2037,10 +2037,6 @@ (a2 Reg (alu_rr_imm12 (AluOPRRI.Addi) a (imm12_const 1)))) (gen_select_reg (IntCC.SignedLessThan) r (zero_reg) a2 r))) -(decl output_ifcout (Reg) InstOutput) -(rule (output_ifcout reg) - (output_pair reg (value_regs_invalid))) - (decl gen_trapff (FloatCC Reg Reg Type TrapCode) InstOutput) (rule (gen_trapff cc a b ty trap_code) diff --git a/cranelift/codegen/src/isa/riscv64/inst/mod.rs b/cranelift/codegen/src/isa/riscv64/inst/mod.rs index dfcf467185..ac1cc08e7b 100644 --- a/cranelift/codegen/src/isa/riscv64/inst/mod.rs +++ b/cranelift/codegen/src/isa/riscv64/inst/mod.rs @@ -6,7 +6,7 @@ use crate::binemit::{Addend, CodeOffset, Reloc}; pub use crate::ir::condcodes::IntCC; -use crate::ir::types::{F32, F64, FFLAGS, I128, I16, I32, I64, I8, IFLAGS, R32, R64}; +use crate::ir::types::{F32, F64, I128, I16, I32, I64, I8, R32, R64}; pub use crate::ir::{ExternalName, MemFlags, Opcode, SourceLoc, Type, ValueLabel}; use crate::isa::CallConv; @@ -778,8 +778,6 @@ impl MachInst for Inst { F32 => Ok((&[RegClass::Float], &[F32])), F64 => Ok((&[RegClass::Float], &[F64])), I128 => Ok((&[RegClass::Int, RegClass::Int], &[I64, I64])), - IFLAGS => Ok((&[RegClass::Int], &[IFLAGS])), - FFLAGS => Ok((&[RegClass::Int], &[FFLAGS])), _ => Err(CodegenError::Unsupported(format!( "Unexpected SSA-value type: {}", ty diff --git a/cranelift/codegen/src/isa/riscv64/lower.isle b/cranelift/codegen/src/isa/riscv64/lower.isle index af89d9bbe3..634ea2e81b 100644 --- a/cranelift/codegen/src/isa/riscv64/lower.isle +++ b/cranelift/codegen/src/isa/riscv64/lower.isle @@ -43,11 +43,6 @@ (high Reg (alu_add high_tmp carry))) (value_regs low high))) -;;; Rules for `iadd_ifcout` ;;;;;;;;;;;;; -(rule - (lower (has_type (fits_in_64 ty) (iadd_ifcout x y))) - (output_ifcout (alu_add x y))) - ;;; Rules for `uadd_overflow_trap` ;;;;;;;;;;;;; (rule (lower (has_type (fits_in_64 ty) (uadd_overflow_trap x y tc))) @@ -747,14 +742,6 @@ (rule (lower (icmp cc x @ (value_type ty) y)) (lower_icmp cc x y ty)) -;; special for `iadd_ifcout` first out. -(rule 2 - (lower (icmp cc (iadd_ifcout a @ (value_type ty) b) y)) - (lower_icmp cc (alu_add a b) y ty)) - -(rule 1 - (lower (icmp cc x (iadd_ifcout a @ (value_type ty) b))) - (lower_icmp cc x (alu_add a b) ty)) (decl gen_fcmp (FloatCC Value Value Type) Reg) (rule diff --git a/cranelift/codegen/src/isa/s390x/inst/mod.rs b/cranelift/codegen/src/isa/s390x/inst/mod.rs index 049abbd7ff..455065806b 100644 --- a/cranelift/codegen/src/isa/s390x/inst/mod.rs +++ b/cranelift/codegen/src/isa/s390x/inst/mod.rs @@ -1244,9 +1244,6 @@ impl MachInst for Inst { types::F64 => Ok((&[RegClass::Float], &[types::F64])), types::I128 => Ok((&[RegClass::Float], &[types::I128])), _ if ty.is_vector() && ty.bits() == 128 => Ok((&[RegClass::Float], &[types::I8X16])), - // FIXME: We don't really have IFLAGS, but need to allow it here - // for now to support the SelectifSpectreGuard instruction. - types::IFLAGS => Ok((&[RegClass::Int], &[types::I64])), _ => Err(CodegenError::Unsupported(format!( "Unexpected SSA-value type: {}", ty diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index a3bb10701b..9873e935cb 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -192,47 +192,6 @@ (vec_unpacks_low ty y)))) -;;;; Rules for `iadd_ifcout` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -;; N.B.: the second output of `iadd_ifcout` is meant to be the `iflags` value -;; containing the carry result, but we do not support the `iflags` mechanism. -;; However, the only actual use case is where `iadd_ifcout` feeds into `trapif`, -;; which is implemented by explicitly matching on the flags producer. So we can -;; get away with just using an invalid second output, and the reg-renaming code -;; does the right thing, for now. -(decl output_ifcout (Reg) InstOutput) -(rule (output_ifcout reg) - (output_pair reg (value_regs_invalid))) - -;; Add two registers. -(rule 0 (lower (has_type (fits_in_64 ty) (iadd_ifcout x y))) - (output_ifcout (add_logical_reg ty x y))) - -;; Add a register and a zero-extended register. -(rule 4 (lower (has_type (fits_in_64 ty) (iadd_ifcout x (zext32_value y)))) - (output_ifcout (add_logical_reg_zext32 ty x y))) -(rule 8 (lower (has_type (fits_in_64 ty) (iadd_ifcout (zext32_value x) y))) - (output_ifcout (add_logical_reg_zext32 ty y x))) - -;; Add a register and an immediate. -(rule 3 (lower (has_type (fits_in_64 ty) (iadd_ifcout x (u32_from_value y)))) - (output_ifcout (add_logical_zimm32 ty x y))) -(rule 7 (lower (has_type (fits_in_64 ty) (iadd_ifcout (u32_from_value x) y))) - (output_ifcout (add_logical_zimm32 ty y x))) - -;; Add a register and memory (32/64-bit types). -(rule 2 (lower (has_type (fits_in_64 ty) (iadd_ifcout x (sinkable_load_32_64 y)))) - (output_ifcout (add_logical_mem ty x (sink_load y)))) -(rule 6 (lower (has_type (fits_in_64 ty) (iadd_ifcout (sinkable_load_32_64 x) y))) - (output_ifcout (add_logical_mem ty y (sink_load x)))) - -;; Add a register and zero-extended memory. -(rule 1 (lower (has_type (fits_in_64 ty) (iadd_ifcout x (sinkable_uload32 y)))) - (output_ifcout (add_logical_mem_zext32 ty x (sink_uload32 y)))) -(rule 5 (lower (has_type (fits_in_64 ty) (iadd_ifcout (sinkable_uload32 x) y))) - (output_ifcout (add_logical_mem_zext32 ty y (sink_uload32 x)))) - - ;;;; Rules for `iabs` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Absolute value of a register. @@ -3742,14 +3701,12 @@ ;;;; Rules for `select_spectre_guard` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; We do not support the `iflags` mechanism on our platform. However, common -;; code will unconditionally emit certain patterns using `iflags` which we -;; need to handle somehow. Note that only those specific patterns are -;; recognized by the code below, other uses will fail to lower. - +;; We need to guarantee a conditional move instruction. But on this platform +;; this is already the best way to implement select in general, so the +;; implementation of `select_spectre_guard` is identical to `select`. (rule (lower (has_type ty (select_spectre_guard - (icmp int_cc x y) val_true val_false))) - (select_bool_reg ty (icmp_val $false int_cc x y) + val_cond val_true val_false))) + (select_bool_reg ty (value_nonzero val_cond) (put_in_reg val_true) (put_in_reg val_false))) diff --git a/cranelift/codegen/src/isa/s390x/lower.rs b/cranelift/codegen/src/isa/s390x/lower.rs index 79765d0175..7c122713f9 100644 --- a/cranelift/codegen/src/isa/s390x/lower.rs +++ b/cranelift/codegen/src/isa/s390x/lower.rs @@ -51,7 +51,6 @@ impl LowerBackend for S390xBackend { | Opcode::Isplit | Opcode::Iconcat | Opcode::Iadd - | Opcode::IaddIfcout | Opcode::Isub | Opcode::UaddSat | Opcode::SaddSat @@ -177,6 +176,7 @@ impl LowerBackend for S390xBackend { | Opcode::Trapnz | Opcode::ResumableTrapnz | Opcode::Debugtrap + | Opcode::UaddOverflowTrap | Opcode::Call | Opcode::CallIndirect | Opcode::Return @@ -221,9 +221,6 @@ impl LowerBackend for S390xBackend { Opcode::GlobalValue => { panic!("global_value should have been removed by legalization!"); } - Opcode::Ifcmp | Opcode::Ffcmp => { - panic!("Flags opcode should not be encountered."); - } Opcode::Jump | Opcode::Brz | Opcode::Brnz | Opcode::BrTable => { panic!("Branch opcode reached non-branch lowering logic!"); } @@ -235,17 +232,11 @@ impl LowerBackend for S390xBackend { | Opcode::SremImm | Opcode::IrsubImm | Opcode::IaddCin - | Opcode::IaddIfcin | Opcode::IaddCout | Opcode::IaddCarry - | Opcode::IaddIfcarry - | Opcode::UaddOverflowTrap | Opcode::IsubBin - | Opcode::IsubIfbin | Opcode::IsubBout - | Opcode::IsubIfbout | Opcode::IsubBorrow - | Opcode::IsubIfborrow | Opcode::BandImm | Opcode::BorImm | Opcode::BxorImm @@ -254,8 +245,7 @@ impl LowerBackend for S390xBackend { | Opcode::IshlImm | Opcode::UshrImm | Opcode::SshrImm - | Opcode::IcmpImm - | Opcode::IfcmpImm => { + | Opcode::IcmpImm => { panic!("ALU+imm and ALU+carry ops should not appear here!"); } } diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index eb7307e3db..a510789446 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -2318,7 +2318,6 @@ impl MachInst for Inst { assert!(ty.bits() <= 128); Ok((&[RegClass::Float], &[types::I8X16])) } - types::IFLAGS | types::FFLAGS => Ok((&[RegClass::Int], &[types::I64])), _ => Err(CodegenError::Unsupported(format!( "Unexpected SSA-value type: {}", ty diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 3954ec5fbf..d6a6dbe225 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -135,49 +135,6 @@ (uadd_sat x y))) (x64_paddusw x y)) -;;;; Rules for `iadd_ifcout` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -;; N.B.: the second output of `iadd_ifcout` is meant to be the -;; `iflags` value containing the carry result. However, we plan to -;; replace this with a bool carry flag, and all consumers of `iflags` -;; remain in the handwritten pattern-matching code and explicitly -;; match on the flags producer. So we can get away with just -;; using an invalid second output, and the reg-renaming code does the -;; right thing, for now. For safety, we assert elsewhere that no one -;; actually uses the register assigned to the SSA `iflags`-typed -;; `Value`. - -(decl output_ifcout (Reg) InstOutput) -(rule (output_ifcout reg) - (output_pair reg (value_regs_invalid))) - -;; Add two registers. -(rule 0 (lower (has_type (fits_in_64 ty) - (iadd_ifcout x y))) - (output_ifcout (x64_add ty x y))) - -;; Add a register and an immediate. - -(rule 1 (lower (has_type (fits_in_64 ty) - (iadd_ifcout x (simm32_from_value y)))) - (output_ifcout (x64_add ty x y))) - -(rule 2 (lower (has_type (fits_in_64 ty) - (iadd_ifcout (simm32_from_value x) y))) - (output_ifcout (x64_add ty y x))) - -;; Add a register and memory. - -(rule 3 (lower (has_type (fits_in_64 ty) - (iadd_ifcout x (sinkable_load y)))) - (output_ifcout (x64_add ty x (sink_load_to_gpr_mem_imm y)))) - -(rule 4 (lower (has_type (fits_in_64 ty) - (iadd_ifcout (sinkable_load x) y))) - (output_ifcout (x64_add ty y (sink_load_to_gpr_mem_imm x)))) - -;; (No `iadd_ifcout` for `i128`.) - ;;;; Rules for `isub` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; `i64` and smaller. @@ -2180,9 +2137,6 @@ (rule (lower (has_type $I64 (uextend src @ (has_type $I32 (iadd _ _))))) src) -(rule (lower (has_type $I64 - (uextend src @ (has_type $I32 (iadd_ifcout _ _))))) - src) (rule (lower (has_type $I64 (uextend src @ (has_type $I32 (isub _ _))))) src) diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index a2bfb41430..7224b64baf 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -332,7 +332,6 @@ fn lower_insn_to_regs( | Opcode::Null | Opcode::Iadd | Opcode::IaddCout - | Opcode::IaddIfcout | Opcode::SaddSat | Opcode::UaddSat | Opcode::Isub @@ -360,6 +359,7 @@ fn lower_insn_to_regs( | Opcode::Ineg | Opcode::Trap | Opcode::ResumableTrap + | Opcode::UaddOverflowTrap | Opcode::Clz | Opcode::Ctz | Opcode::Popcnt @@ -500,13 +500,6 @@ fn lower_insn_to_regs( unimplemented!("Vector split/concat ops not implemented."); } - // Opcodes that should be removed by legalization. These should - // eventually be removed if/when we replace in-situ legalization with - // something better. - Opcode::Ifcmp | Opcode::Ffcmp => { - panic!("Should never reach ifcmp/ffcmp as isel root!"); - } - Opcode::IaddImm | Opcode::ImulImm | Opcode::UdivImm @@ -515,16 +508,10 @@ fn lower_insn_to_regs( | Opcode::SremImm | Opcode::IrsubImm | Opcode::IaddCin - | Opcode::IaddIfcin | Opcode::IaddCarry - | Opcode::IaddIfcarry | Opcode::IsubBin - | Opcode::IsubIfbin | Opcode::IsubBout - | Opcode::IsubIfbout | Opcode::IsubBorrow - | Opcode::IsubIfborrow - | Opcode::UaddOverflowTrap | Opcode::BandImm | Opcode::BorImm | Opcode::BxorImm @@ -533,8 +520,7 @@ fn lower_insn_to_regs( | Opcode::IshlImm | Opcode::UshrImm | Opcode::SshrImm - | Opcode::IcmpImm - | Opcode::IfcmpImm => { + | Opcode::IcmpImm => { panic!("ALU+imm and ALU+carry ops should not appear here!"); } diff --git a/cranelift/codegen/src/legalizer/mod.rs b/cranelift/codegen/src/legalizer/mod.rs index 32f3331214..e82189cec1 100644 --- a/cranelift/codegen/src/legalizer/mod.rs +++ b/cranelift/codegen/src/legalizer/mod.rs @@ -172,8 +172,7 @@ pub fn simple_legalize(func: &mut ir::Function, cfg: &mut ControlFlowGraph, isa: | ir::Opcode::IrsubImm | ir::Opcode::ImulImm | ir::Opcode::SdivImm - | ir::Opcode::SremImm - | ir::Opcode::IfcmpImm => true, + | ir::Opcode::SremImm => true, _ => false, }; @@ -229,10 +228,6 @@ pub fn simple_legalize(func: &mut ir::Function, cfg: &mut ControlFlowGraph, isa: ir::Opcode::UremImm => { replace.urem(arg, imm); } - // comparisons - ir::Opcode::IfcmpImm => { - replace.ifcmp(arg, imm); - } _ => prev_pos = pos.position(), }; } diff --git a/cranelift/codegen/src/licm.rs b/cranelift/codegen/src/licm.rs index 1d1e340d0c..6b4d65f3ef 100644 --- a/cranelift/codegen/src/licm.rs +++ b/cranelift/codegen/src/licm.rs @@ -142,7 +142,6 @@ fn trivially_unsafe_for_licm(opcode: Opcode) -> bool { || opcode.is_return() || opcode.can_trap() || opcode.other_side_effects() - || opcode.writes_cpu_flags() } fn is_unsafe_load(inst_data: &InstructionData) -> bool { diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index da1391d347..f46cde696f 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -1,4 +1,4 @@ -use crate::ir::{types, Inst, Value, ValueList}; +use crate::ir::{Inst, Value, ValueList}; use crate::machinst::{get_output_reg, InsnOutput}; use alloc::boxed::Box; use alloc::vec::Vec; @@ -775,16 +775,8 @@ where for i in 0..outputs.len() { let regs = temp_regs[i]; let dsts = get_output_reg(isle_ctx.lower_ctx, outputs[i]); - let ty = isle_ctx - .lower_ctx - .output_ty(outputs[i].insn, outputs[i].output); - if ty == types::IFLAGS || ty == types::FFLAGS { - // Flags values do not occupy any registers. - assert!(regs.len() == 0); - } else { - for (dst, temp) in dsts.regs().iter().zip(regs.regs().iter()) { - isle_ctx.lower_ctx.set_vreg_alias(dst.to_reg(), *temp); - } + for (dst, temp) in dsts.regs().iter().zip(regs.regs().iter()) { + isle_ctx.lower_ctx.set_vreg_alias(dst.to_reg(), *temp); } } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 2b6480f280..ea2680b887 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -9,7 +9,6 @@ use crate::entity::SecondaryMap; use crate::fx::{FxHashMap, FxHashSet}; use crate::inst_predicates::{has_lowering_side_effect, is_constant_64bit}; use crate::ir::{ - types::{FFLAGS, IFLAGS}, ArgumentPurpose, Block, Constant, ConstantData, DataFlowGraph, ExternalName, Function, GlobalValue, GlobalValueData, Immediate, Inst, InstructionData, MemFlags, Opcode, RelSourceLoc, Type, Value, ValueDef, ValueLabelAssignments, ValueLabelStart, @@ -1246,12 +1245,6 @@ impl<'func, I: VCodeInst> Lower<'func, I> { let val = self.f.dfg.resolve_aliases(val); trace!("put_value_in_regs: val {}", val); - // Assert that the value is not `iflags`/`fflags`-typed; these - // cannot be reified into normal registers. TODO(#3249) - // eventually remove the `iflags` type altogether! - let ty = self.f.dfg.value_type(val); - assert!(ty != IFLAGS && ty != FFLAGS); - if let Some(inst) = self.f.dfg.value_def(val).inst() { assert!(!self.inst_sunk.contains(&inst)); } @@ -1268,6 +1261,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { .inst() .and_then(|inst| self.get_constant(inst)) { + let ty = self.f.dfg.value_type(val); let regs = self.alloc_tmp(ty); trace!(" -> regs {:?}", regs); assert!(regs.is_valid()); diff --git a/cranelift/codegen/src/simple_gvn.rs b/cranelift/codegen/src/simple_gvn.rs index 87dc4d1a96..0606297e3b 100644 --- a/cranelift/codegen/src/simple_gvn.rs +++ b/cranelift/codegen/src/simple_gvn.rs @@ -18,7 +18,6 @@ fn trivially_unsafe_for_gvn(opcode: Opcode) -> bool { || opcode.can_trap() || opcode.other_side_effects() || opcode.can_store() - || opcode.writes_cpu_flags() } /// Test that, if the specified instruction is a load, it doesn't have the `readonly` memflag. diff --git a/cranelift/codegen/src/simple_preopt.rs b/cranelift/codegen/src/simple_preopt.rs index 08c3660213..2c51aee330 100644 --- a/cranelift/codegen/src/simple_preopt.rs +++ b/cranelift/codegen/src/simple_preopt.rs @@ -699,7 +699,6 @@ mod simplify { imm = imm.wrapping_neg(); Opcode::IaddImm } - Opcode::Ifcmp => Opcode::IfcmpImm, _ => return, }; let ty = pos.func.dfg.ctrl_typevar(inst); diff --git a/cranelift/codegen/src/timing.rs b/cranelift/codegen/src/timing.rs index dcefde2da4..65412c5df5 100644 --- a/cranelift/codegen/src/timing.rs +++ b/cranelift/codegen/src/timing.rs @@ -49,7 +49,6 @@ define_passes! { wasm_translate_function: "Translate WASM function", verifier: "Verify Cranelift IR", - verify_flags: "Verify CPU flags", compile: "Compilation passes", try_incremental_cache: "Try loading from incremental cache", diff --git a/cranelift/codegen/src/verifier/flags.rs b/cranelift/codegen/src/verifier/flags.rs deleted file mode 100644 index 5e67e3ae77..0000000000 --- a/cranelift/codegen/src/verifier/flags.rs +++ /dev/null @@ -1,161 +0,0 @@ -//! Verify CPU flags values. - -use crate::entity::{EntitySet, SecondaryMap}; -use crate::flowgraph::{BlockPredecessor, ControlFlowGraph}; -use crate::ir; -use crate::ir::instructions::BranchInfo; -use crate::packed_option::PackedOption; -use crate::timing; -use crate::verifier::{VerifierErrors, VerifierStepResult}; - -/// Verify that CPU flags are used correctly. -/// -/// The value types `iflags` and `fflags` represent CPU flags which usually live in a -/// special-purpose register, so they can't be used as freely as other value types that can live in -/// any register. -/// -/// We verify the following conditions: -/// -/// - At most one flags value can be live at a time. -/// - A flags value can not be live across an instruction that clobbers the flags. -/// -/// -pub fn verify_flags( - func: &ir::Function, - cfg: &ControlFlowGraph, - errors: &mut VerifierErrors, -) -> VerifierStepResult<()> { - let _tt = timing::verify_flags(); - let mut verifier = FlagsVerifier { - func, - cfg, - livein: SecondaryMap::new(), - }; - verifier.check(errors) -} - -struct FlagsVerifier<'a> { - func: &'a ir::Function, - cfg: &'a ControlFlowGraph, - - /// The single live-in flags value (if any) for each block. - livein: SecondaryMap>, -} - -impl<'a> FlagsVerifier<'a> { - fn check(&mut self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { - // List of blocks that need to be processed. blocks may be re-added to this list when we detect - // that one of their successor blocks needs a live-in flags value. - let mut worklist = EntitySet::with_capacity(self.func.layout.block_capacity()); - for block in self.func.layout.blocks() { - worklist.insert(block); - } - - while let Some(block) = worklist.pop() { - if let Some(value) = self.visit_block(block, errors)? { - // The block has live-in flags. Check if the value changed. - match self.livein[block].expand() { - // Revisit any predecessor blocks the first time we see a live-in for `block`. - None => { - self.livein[block] = value.into(); - for BlockPredecessor { block: pred, .. } in self.cfg.pred_iter(block) { - worklist.insert(pred); - } - } - Some(old) if old != value => { - return errors.fatal(( - block, - format!("conflicting live-in CPU flags: {} and {}", old, value), - )); - } - x => assert_eq!(x, Some(value)), - } - } else { - // Existing live-in flags should never be able to disappear. - assert_eq!(self.livein[block].expand(), None); - } - } - - Ok(()) - } - - /// Check flags usage in `block` and return the live-in flags value, if any. - fn visit_block( - &self, - block: ir::Block, - errors: &mut VerifierErrors, - ) -> VerifierStepResult> { - // The single currently live flags value. - let mut live_val = None; - - // Visit instructions backwards so we can track liveness accurately. - for inst in self.func.layout.block_insts(block).rev() { - // Check if `inst` interferes with existing live flags. - if let Some(live) = live_val { - for &res in self.func.dfg.inst_results(inst) { - if res == live { - // We've reached the def of `live_flags`, so it is no longer live above. - live_val = None; - } else if self.func.dfg.value_type(res).is_flags() { - errors - .report((inst, format!("{} clobbers live CPU flags in {}", res, live))); - return Err(()); - } - } - } - - // Now look for live ranges of CPU flags that end here. - for &arg in self.func.dfg.inst_args(inst) { - if self.func.dfg.value_type(arg).is_flags() { - merge(&mut live_val, arg, inst, errors)?; - } - } - - // Include live-in flags to successor blocks. - match self.func.dfg.analyze_branch(inst) { - BranchInfo::NotABranch => {} - BranchInfo::SingleDest(dest, _) => { - if let Some(val) = self.livein[dest].expand() { - merge(&mut live_val, val, inst, errors)?; - } - } - BranchInfo::Table(jt, dest) => { - if let Some(dest) = dest { - if let Some(val) = self.livein[dest].expand() { - merge(&mut live_val, val, inst, errors)?; - } - } - for dest in self.func.jump_tables[jt].iter() { - if let Some(val) = self.livein[*dest].expand() { - merge(&mut live_val, val, inst, errors)?; - } - } - } - } - } - - // Return the required live-in flags value. - Ok(live_val) - } -} - -// Merge live flags values, or return an error on conflicting values. -fn merge( - a: &mut Option, - b: ir::Value, - inst: ir::Inst, - errors: &mut VerifierErrors, -) -> VerifierStepResult<()> { - if let Some(va) = *a { - if b != va { - return errors.fatal(( - inst, - format!("conflicting live CPU flags: {} and {}", va, b), - )); - } - } else { - *a = Some(b); - } - - Ok(()) -} diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index b19523b7ef..162d909c63 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -57,7 +57,6 @@ //! - Swizzle and shuffle instructions take a variable number of lane arguments. The number //! of arguments must match the destination type, and the lane indexes must be in range. -use self::flags::verify_flags; use crate::dbg::DisplayList; use crate::dominator_tree::DominatorTree; use crate::entity::SparseSet; @@ -81,8 +80,6 @@ use alloc::vec::Vec; use core::cmp::Ordering; use core::fmt::{self, Display, Formatter}; -mod flags; - /// A verifier error. #[derive(Debug, PartialEq, Eq, Clone)] pub struct VerifierError { @@ -1799,8 +1796,6 @@ impl<'a> Verifier<'a> { self.encodable_as_bb(block, errors)?; } - verify_flags(self.func, &self.expected_cfg, errors)?; - if !errors.is_empty() { log::warn!( "Found verifier errors in function:\n{}", diff --git a/cranelift/docs/ir.md b/cranelift/docs/ir.md index 66d787295a..6ad6d5fc73 100644 --- a/cranelift/docs/ir.md +++ b/cranelift/docs/ir.md @@ -179,24 +179,6 @@ instructions are encoded as follows: - f32 - f64 -### CPU flags types - -Some target ISAs use CPU flags to represent the result of a comparison. These -CPU flags are represented as two value types depending on the type of values -compared. - -Since some ISAs don't have CPU flags, these value types should not be used -until the legalization phase of compilation where the code is adapted to fit -the target ISA. Use instructions like `icmp` instead. - -The CPU flags types are also restricted such that two flags values can not be -live at the same time. After legalization, some instruction encodings will -clobber the flags, and flags values are not allowed to be live across such -instructions either. The verifier enforces these rules. - -- iflags -- fflags - ### SIMD vector types A SIMD vector type represents a vector of values from one of the scalar types diff --git a/cranelift/filetests/filetests/isa/s390x/arithmetic.clif b/cranelift/filetests/filetests/isa/s390x/arithmetic.clif index 8e0cdf3cca..ff65baa7e6 100644 --- a/cranelift/filetests/filetests/isa/s390x/arithmetic.clif +++ b/cranelift/filetests/filetests/isa/s390x/arithmetic.clif @@ -231,107 +231,6 @@ block0(v0: i8, v1: i64): ; ar %r2, %r3 ; br %r14 -function %iadd_i64(i64, i64) -> i64 { -block0(v0: i64, v1: i64): - v2, v3 = iadd_ifcout.i64 v0, v1 - return v2 -} - -; block0: -; algr %r2, %r3 -; br %r14 - -function %iadd_i64_ext32(i64, i32) -> i64 { -block0(v0: i64, v1: i32): - v2 = uextend.i64 v1 - v3, v4 = iadd_ifcout.i64 v0, v2 - return v3 -} - -; block0: -; algfr %r2, %r3 -; br %r14 - -function %iadd_i64_imm32(i64) -> i64 { -block0(v0: i64): - v1 = iconst.i64 32768 - v2, v3 = iadd_ifcout.i64 v0, v1 - return v2 -} - -; block0: -; algfi %r2, 32768 -; br %r14 - -function %iadd_i64_mem(i64, i64) -> i64 { -block0(v0: i64, v1: i64): - v2 = load.i64 v1 - v3, v4 = iadd_ifcout.i64 v0, v2 - return v3 -} - -; block0: -; lg %r3, 0(%r3) -; algr %r2, %r3 -; br %r14 - -function %iadd_i64_mem_ext32(i64, i64) -> i64 { -block0(v0: i64, v1: i64): - v2 = uload32.i64 v1 - v3, v4 = iadd_ifcout.i64 v0, v2 - return v3 -} - -; block0: -; llgf %r3, 0(%r3) -; algr %r2, %r3 -; br %r14 - -function %iadd_i32(i32, i32) -> i32 { -block0(v0: i32, v1: i32): - v2, v3 = iadd_ifcout.i32 v0, v1 - return v2 -} - -; block0: -; alr %r2, %r3 -; br %r14 - -function %iadd_i32_imm(i32) -> i32 { -block0(v0: i32): - v1 = iconst.i32 32768 - v2, v3 = iadd_ifcout.i32 v0, v1 - return v2 -} - -; block0: -; alfi %r2, 32768 -; br %r14 - -function %iadd_i32_mem(i32, i64) -> i32 { -block0(v0: i32, v1: i64): - v2 = load.i32 v1 - v3, v4 = iadd_ifcout.i32 v0, v2 - return v3 -} - -; block0: -; l %r3, 0(%r3) -; alr %r2, %r3 -; br %r14 - -function %iadd_i32_memoff(i32, i64) -> i32 { -block0(v0: i32, v1: i64): - v2 = load.i32 v1+4096 - v3, v4 = iadd_ifcout.i32 v0, v2 - return v3 -} - -; block0: -; ly %r3, 4096(%r3) -; alr %r2, %r3 -; br %r14 - function %isub_i128(i128, i128) -> i128 { block0(v0: i128, v1: i128): v2 = isub.i128 v0, v1 diff --git a/cranelift/filetests/filetests/isa/s390x/uadd_overflow_trap.clif b/cranelift/filetests/filetests/isa/s390x/uadd_overflow_trap.clif index 12c2a83143..e0c9ddd841 100644 --- a/cranelift/filetests/filetests/isa/s390x/uadd_overflow_trap.clif +++ b/cranelift/filetests/filetests/isa/s390x/uadd_overflow_trap.clif @@ -71,3 +71,14 @@ block0(v0: i64, v1: i64): ; jle 6 ; trap ; br %r14 +function %f5(i64, i32) -> i64 { +block0(v0: i64, v1: i32): + v2 = uextend.i64 v1 + v3 = uadd_overflow_trap v0, v2, user0 + return v3 +} + +; block0: +; algfr %r2, %r3 +; jle 6 ; trap +; br %r14 diff --git a/cranelift/filetests/filetests/licm/reject.clif b/cranelift/filetests/filetests/licm/reject.clif index 378a9003d1..4b3d64664c 100644 --- a/cranelift/filetests/filetests/licm/reject.clif +++ b/cranelift/filetests/filetests/licm/reject.clif @@ -20,29 +20,6 @@ block3(v6: i32): } -function %cpu_flags(i32, i32) -> i32 { -block0(v0: i32, v1: i32): - jump block1(v0, v1) - -block1(v2: i32, v3: i32): - v4 = ifcmp.i32 v0, v1 - v5 = selectif.i32 eq v4, v2, v3 -; check: block1(v2: i32, v3: i32): -; check: ifcmp.i32 v0, v1 -; check: v5 = selectif.i32 eq v4, v2, v3 - v8 = iconst.i32 1 - brz v1, block3(v1) - jump block2 - -block2: - v9 = isub v1, v8 - v10 = iadd v1, v8 - jump block1(v9, v10) - -block3(v6: i32): - return v6 -} - function %spill(i32, i32) -> i32 { block0(v0: i32, v1: i32): v2 = spill.i32 v0 diff --git a/cranelift/filetests/filetests/parser/ternary.clif b/cranelift/filetests/filetests/parser/ternary.clif index b148850198..5f86eb6686 100644 --- a/cranelift/filetests/filetests/parser/ternary.clif +++ b/cranelift/filetests/filetests/parser/ternary.clif @@ -1,24 +1,31 @@ test cat test verifier +function %select(i32, i32, i32) -> i32 { +block1(v1: i32, v2: i32, v3: i32): + v10 = select v1, v2, v3 + ;check: v10 = select v1, v2, v3 + return v10 +} + function %add_i96(i32, i32, i32, i32, i32, i32) -> i32, i32, i32 { block1(v1: i32, v2: i32, v3: i32, v4: i32, v5: i32, v6: i32): - v10, v11 = iadd_ifcout v1, v4 - ;check: v10, v11 = iadd_ifcout v1, v4 - v20, v21 = iadd_ifcarry v2, v5, v11 - ; check: v20, v21 = iadd_ifcarry v2, v5, v11 - v30 = iadd_ifcin v3, v6, v21 - ; check: v30 = iadd_ifcin v3, v6, v21 + v10, v11 = iadd_cout v1, v4 + ;check: v10, v11 = iadd_cout v1, v4 + v20, v21 = iadd_carry v2, v5, v11 + ; check: v20, v21 = iadd_carry v2, v5, v11 + v30 = iadd_cin v3, v6, v21 + ; check: v30 = iadd_cin v3, v6, v21 return v10, v20, v30 } function %sub_i96(i32, i32, i32, i32, i32, i32) -> i32, i32, i32 { block1(v1: i32, v2: i32, v3: i32, v4: i32, v5: i32, v6: i32): - v10, v11 = isub_ifbout v1, v4 - ;check: v10, v11 = isub_ifbout v1, v4 - v20, v21 = isub_ifborrow v2, v5, v11 - ; check: v20, v21 = isub_ifborrow v2, v5, v11 - v30 = isub_ifbin v3, v6, v21 - ; check: v30 = isub_ifbin v3, v6, v21 + v10, v11 = isub_bout v1, v4 + ;check: v10, v11 = isub_bout v1, v4 + v20, v21 = isub_borrow v2, v5, v11 + ; check: v20, v21 = isub_borrow v2, v5, v11 + v30 = isub_bin v3, v6, v21 + ; check: v30 = isub_bin v3, v6, v21 return v10, v20, v30 } diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 4872b4e8b1..61d22b991a 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -1144,7 +1144,6 @@ where fn generate_type(&mut self) -> Result { // TODO: It would be nice if we could get these directly from cranelift let scalars = [ - // IFLAGS, FFLAGS, I8, I16, I32, I64, I128, F32, F64, // R32, R64, ]; diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index cc58cf29ba..022b82770c 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -10,14 +10,12 @@ use crate::state::{MemoryError, State}; use crate::step::{step, ControlFlow, StepError}; use crate::value::{Value, ValueError}; use cranelift_codegen::data_value::DataValue; -use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; use cranelift_codegen::ir::{ ArgumentPurpose, Block, FuncRef, Function, GlobalValue, GlobalValueData, Heap, LibCall, StackSlot, TrapCode, Type, Value as ValueRef, }; use log::trace; use smallvec::SmallVec; -use std::collections::HashSet; use std::convert::{TryFrom, TryInto}; use std::fmt::Debug; use std::iter; @@ -204,8 +202,6 @@ pub struct InterpreterState<'a> { pub frame_offset: usize, pub stack: Vec, pub heaps: Vec, - pub iflags: HashSet, - pub fflags: HashSet, pub pinned_reg: DataValue, } @@ -218,8 +214,6 @@ impl Default for InterpreterState<'_> { frame_offset: 0, stack: Vec::with_capacity(1024), heaps: Vec::new(), - iflags: HashSet::new(), - fflags: HashSet::new(), pinned_reg: DataValue::U64(0), } } @@ -349,27 +343,6 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { self.current_frame_mut().set(name, value) } - fn has_iflag(&self, flag: IntCC) -> bool { - self.iflags.contains(&flag) - } - - fn has_fflag(&self, flag: FloatCC) -> bool { - self.fflags.contains(&flag) - } - - fn set_iflag(&mut self, flag: IntCC) { - self.iflags.insert(flag); - } - - fn set_fflag(&mut self, flag: FloatCC) { - self.fflags.insert(flag); - } - - fn clear_flags(&mut self) { - self.iflags.clear(); - self.fflags.clear() - } - fn stack_address( &self, size: AddressSize, @@ -737,17 +710,6 @@ mod tests { assert_eq!(result, vec![DataValue::I32(0)]) } - #[test] - fn state_flags() { - let mut state = InterpreterState::default(); - let flag = IntCC::UnsignedLessThan; - assert!(!state.has_iflag(flag)); - state.set_iflag(flag); - assert!(state.has_iflag(flag)); - state.clear_flags(); - assert!(!state.has_iflag(flag)); - } - #[test] fn fuel() { let code = "function %test() -> i8 { diff --git a/cranelift/interpreter/src/state.rs b/cranelift/interpreter/src/state.rs index 48fe3ba6f0..868266d7b7 100644 --- a/cranelift/interpreter/src/state.rs +++ b/cranelift/interpreter/src/state.rs @@ -3,7 +3,6 @@ use crate::address::{Address, AddressSize}; use crate::interpreter::LibCallHandler; use cranelift_codegen::data_value::DataValue; -use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; use cranelift_codegen::ir::{FuncRef, Function, GlobalValue, Heap, StackSlot, Type, Value}; use cranelift_entity::PrimaryMap; use smallvec::SmallVec; @@ -51,17 +50,6 @@ pub trait State<'a, V> { Ok(values) } - /// Check if an [IntCC] flag has been set. - fn has_iflag(&self, flag: IntCC) -> bool; - /// Set an [IntCC] flag. - fn set_iflag(&mut self, flag: IntCC); - /// Check if a [FloatCC] flag has been set. - fn has_fflag(&self, flag: FloatCC) -> bool; - /// Set a [FloatCC] flag. - fn set_fflag(&mut self, flag: FloatCC); - /// Clear all [IntCC] and [FloatCC] flags. - fn clear_flags(&mut self); - /// Computes the stack address for this stack slot, including an offset. fn stack_address( &self, @@ -152,20 +140,6 @@ where None } - fn has_iflag(&self, _flag: IntCC) -> bool { - false - } - - fn has_fflag(&self, _flag: FloatCC) -> bool { - false - } - - fn set_iflag(&mut self, _flag: IntCC) {} - - fn set_fflag(&mut self, _flag: FloatCC) {} - - fn clear_flags(&mut self) {} - fn stack_address( &self, _size: AddressSize, diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 12c268322c..5bb2b1e5a4 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -622,32 +622,6 @@ where &arg(0)?, &imm_as_ctrl_ty()?, )?), - Opcode::Ifcmp | Opcode::IfcmpImm => { - let arg0 = arg(0)?; - let arg1 = match inst.opcode() { - Opcode::Ifcmp => arg(1)?, - Opcode::IfcmpImm => imm_as_ctrl_ty()?, - _ => unreachable!(), - }; - state.clear_flags(); - for f in &[ - IntCC::Equal, - IntCC::NotEqual, - IntCC::SignedLessThan, - IntCC::SignedGreaterThanOrEqual, - IntCC::SignedGreaterThan, - IntCC::SignedLessThanOrEqual, - IntCC::UnsignedLessThan, - IntCC::UnsignedGreaterThanOrEqual, - IntCC::UnsignedGreaterThan, - IntCC::UnsignedLessThanOrEqual, - ] { - if icmp(ctrl_ty, *f, &arg0, &arg1)?.into_bool()? { - state.set_iflag(*f); - } - } - ControlFlow::Continue - } Opcode::Smin => { if ctrl_ty.is_vector() { let icmp = icmp(ctrl_ty, IntCC::SignedGreaterThan, &arg(1)?, &arg(0)?)?; @@ -795,13 +769,11 @@ where Value::add(Value::add(arg(0)?, arg(1)?)?, Value::int(1, ctrl_ty)?)?, Value::add(arg(0)?, arg(1)?)?, ), - Opcode::IaddIfcin => unimplemented!("IaddIfcin"), Opcode::IaddCout => { let carry = arg(0)?.checked_add(arg(1)?)?.is_none(); let sum = arg(0)?.add(arg(1)?)?; assign_multiple(&[sum, Value::bool(carry, false, types::I8)?]) } - Opcode::IaddIfcout => unimplemented!("IaddIfcout"), Opcode::IaddCarry => { let mut sum = Value::add(arg(0)?, arg(1)?)?; let mut carry = arg(0)?.checked_add(arg(1)?)?.is_none(); @@ -813,7 +785,6 @@ where assign_multiple(&[sum, Value::bool(carry, false, types::I8)?]) } - Opcode::IaddIfcarry => unimplemented!("IaddIfcarry"), Opcode::UaddOverflowTrap => { let sum = Value::add(arg(0)?, arg(1)?)?; let carry = Value::lt(&sum, &arg(0)?)? && Value::lt(&sum, &arg(1)?)?; @@ -828,13 +799,11 @@ where Value::sub(arg(0)?, Value::add(arg(1)?, Value::int(1, ctrl_ty)?)?)?, Value::sub(arg(0)?, arg(1)?)?, ), - Opcode::IsubIfbin => unimplemented!("IsubIfbin"), Opcode::IsubBout => { let sum = Value::sub(arg(0)?, arg(1)?)?; let borrow = Value::lt(&arg(0)?, &arg(1)?)?; assign_multiple(&[sum, Value::bool(borrow, false, types::I8)?]) } - Opcode::IsubIfbout => unimplemented!("IsubIfbout"), Opcode::IsubBorrow => { let rhs = if Value::into_bool(arg(2)?)? { Value::add(arg(1)?, Value::int(1, ctrl_ty)?)? @@ -845,7 +814,6 @@ where let sum = Value::sub(arg(0)?, rhs)?; assign_multiple(&[sum, Value::bool(borrow, false, types::I8)?]) } - Opcode::IsubIfborrow => unimplemented!("IsubIfborrow"), Opcode::Band => binary(Value::and, arg(0)?, arg(1)?)?, Opcode::Bor => binary(Value::or, arg(0)?, arg(1)?)?, Opcode::Bxor => binary(Value::xor, arg(0)?, arg(1)?)?, @@ -910,32 +878,6 @@ where ctrl_ty, )?) } - Opcode::Ffcmp => { - let arg0 = arg(0)?; - let arg1 = arg(1)?; - state.clear_flags(); - for f in &[ - FloatCC::Ordered, - FloatCC::Unordered, - FloatCC::Equal, - FloatCC::NotEqual, - FloatCC::OrderedNotEqual, - FloatCC::UnorderedOrEqual, - FloatCC::LessThan, - FloatCC::LessThanOrEqual, - FloatCC::GreaterThan, - FloatCC::GreaterThanOrEqual, - FloatCC::UnorderedOrLessThan, - FloatCC::UnorderedOrLessThanOrEqual, - FloatCC::UnorderedOrGreaterThan, - FloatCC::UnorderedOrGreaterThanOrEqual, - ] { - if fcmp(*f, &arg0, &arg1)? { - state.set_fflag(*f); - } - } - ControlFlow::Continue - } Opcode::Fadd => binary(Value::add, arg(0)?, arg(1)?)?, Opcode::Fsub => binary(Value::sub, arg(0)?, arg(1)?)?, Opcode::Fmul => binary(Value::mul, arg(0)?, arg(1)?)?, diff --git a/cranelift/reader/src/lexer.rs b/cranelift/reader/src/lexer.rs index 6a23dd32b9..cbf847399d 100644 --- a/cranelift/reader/src/lexer.rs +++ b/cranelift/reader/src/lexer.rs @@ -329,8 +329,6 @@ impl<'a> Lexer<'a> { .or_else(|| Self::value_type(text, prefix, number)) }) .unwrap_or_else(|| match text { - "iflags" => Token::Type(types::IFLAGS), - "fflags" => Token::Type(types::FFLAGS), "cold" => Token::Cold, _ => Token::Identifier(text), }), @@ -622,8 +620,7 @@ mod tests { fn lex_identifiers() { let mut lex = Lexer::new( "v0 v00 vx01 block1234567890 block5234567890 v1x vx1 vxvx4 \ - function0 function i8 i32x4 f32x5 \ - iflags fflags iflagss", + function0 function i8 i32x4 f32x5", ); assert_eq!( lex.next(), @@ -644,9 +641,6 @@ mod tests { assert_eq!(lex.next(), token(Token::Type(types::I8), 1)); assert_eq!(lex.next(), token(Token::Type(types::I32X4), 1)); assert_eq!(lex.next(), token(Token::Identifier("f32x5"), 1)); - assert_eq!(lex.next(), token(Token::Type(types::IFLAGS), 1)); - assert_eq!(lex.next(), token(Token::Type(types::FFLAGS), 1)); - assert_eq!(lex.next(), token(Token::Identifier("iflagss"), 1)); assert_eq!(lex.next(), None); }