From a8aec2e0e64abce8182a232870d987a027e015f7 Mon Sep 17 00:00:00 2001 From: Anton Kirilov Date: Mon, 13 Sep 2021 11:40:01 +0100 Subject: [PATCH] Cranelift AArch64: Avoid invalid encodings for some vector instructions Copyright (c) 2021, Arm Limited. --- .../codegen/src/isa/aarch64/inst/emit.rs | 88 ++++++++++++++--- .../src/isa/aarch64/inst/emit_tests.rs | 99 +++++++++++++++++++ cranelift/codegen/src/isa/aarch64/inst/mod.rs | 1 + .../codegen/src/isa/aarch64/lower_inst.rs | 37 ++++++- 4 files changed, 205 insertions(+), 20 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index 74ec299bed..b0122cbd79 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -1674,15 +1674,27 @@ impl MachInstEmit for Inst { VecMisc2::Neg => (0b1, 0b01011, enc_size), VecMisc2::Abs => (0b0, 0b01011, enc_size), VecMisc2::Fabs => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b0, 0b01111, enc_size) } VecMisc2::Fneg => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b1, 0b01111, enc_size) } VecMisc2::Fsqrt => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b1, 0b11111, enc_size) } VecMisc2::Rev64 => { @@ -1690,11 +1702,19 @@ impl MachInstEmit for Inst { (0b0, 0b00000, enc_size) } VecMisc2::Fcvtzs => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b0, 0b11011, enc_size) } VecMisc2::Fcvtzu => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b1, 0b11011, enc_size) } VecMisc2::Scvtf => { @@ -1706,20 +1726,36 @@ impl MachInstEmit for Inst { (0b1, 0b11101, enc_size & 0b1) } VecMisc2::Frintn => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b0, 0b11000, enc_size & 0b01) } VecMisc2::Frintz => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); - (0b0, 0b11001, enc_size | 0b10) + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); + (0b0, 0b11001, enc_size) } VecMisc2::Frintm => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); (0b0, 0b11001, enc_size & 0b01) } VecMisc2::Frintp => { - debug_assert!(size == VectorSize::Size32x4 || size == VectorSize::Size64x2); - (0b0, 0b11000, enc_size | 0b10) + debug_assert!( + size == VectorSize::Size32x2 + || size == VectorSize::Size32x4 + || size == VectorSize::Size64x2 + ); + (0b0, 0b11000, enc_size) } VecMisc2::Cnt => { debug_assert!(size == VectorSize::Size8x8 || size == VectorSize::Size8x16); @@ -2281,11 +2317,31 @@ impl MachInstEmit for Inst { } VecALUOp::Sshl => (0b000_01110_00_1 | enc_size << 1, 0b010001), VecALUOp::Ushl => (0b001_01110_00_1 | enc_size << 1, 0b010001), - VecALUOp::Umin => (0b001_01110_00_1 | enc_size << 1, 0b011011), - VecALUOp::Smin => (0b000_01110_00_1 | enc_size << 1, 0b011011), - VecALUOp::Umax => (0b001_01110_00_1 | enc_size << 1, 0b011001), - VecALUOp::Smax => (0b000_01110_00_1 | enc_size << 1, 0b011001), - VecALUOp::Urhadd => (0b001_01110_00_1 | enc_size << 1, 0b000101), + VecALUOp::Umin => { + debug_assert_ne!(size, VectorSize::Size64x2); + + (0b001_01110_00_1 | enc_size << 1, 0b011011) + } + VecALUOp::Smin => { + debug_assert_ne!(size, VectorSize::Size64x2); + + (0b000_01110_00_1 | enc_size << 1, 0b011011) + } + VecALUOp::Umax => { + debug_assert_ne!(size, VectorSize::Size64x2); + + (0b001_01110_00_1 | enc_size << 1, 0b011001) + } + VecALUOp::Smax => { + debug_assert_ne!(size, VectorSize::Size64x2); + + (0b000_01110_00_1 | enc_size << 1, 0b011001) + } + VecALUOp::Urhadd => { + debug_assert_ne!(size, VectorSize::Size64x2); + + (0b001_01110_00_1 | enc_size << 1, 0b000101) + } VecALUOp::Fadd => (0b000_01110_00_1, 0b110101), VecALUOp::Fsub => (0b000_01110_10_1, 0b110101), VecALUOp::Fdiv => (0b001_01110_00_1, 0b111111), diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs index 9e45c6795c..fa3aa722d4 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs @@ -4122,6 +4122,17 @@ fn test_aarch64_binemit() { "fabs v15.4s, v16.4s", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Fabs, + rd: writable_vreg(3), + rn: vreg(22), + size: VectorSize::Size64x2, + }, + "C3FAE04E", + "fabs v3.2d, v22.2d", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Fneg, @@ -4133,6 +4144,28 @@ fn test_aarch64_binemit() { "fneg v31.4s, v0.4s", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Fneg, + rd: writable_vreg(11), + rn: vreg(6), + size: VectorSize::Size64x2, + }, + "CBF8E06E", + "fneg v11.2d, v6.2d", + )); + + insns.push(( + Inst::VecMisc { + op: VecMisc2::Fsqrt, + rd: writable_vreg(18), + rn: vreg(25), + size: VectorSize::Size32x2, + }, + "32FBA12E", + "fsqrt v18.2s, v25.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Fsqrt, @@ -4166,6 +4199,28 @@ fn test_aarch64_binemit() { "fcvtzs v4.4s, v22.4s", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Fcvtzs, + rd: writable_vreg(0), + rn: vreg(31), + size: VectorSize::Size64x2, + }, + "E0BBE14E", + "fcvtzs v0.2d, v31.2d", + )); + + insns.push(( + Inst::VecMisc { + op: VecMisc2::Fcvtzu, + rd: writable_vreg(4), + rn: vreg(26), + size: VectorSize::Size32x2, + }, + "44BBA12E", + "fcvtzu v4.2s, v26.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Fcvtzu, @@ -4199,6 +4254,17 @@ fn test_aarch64_binemit() { "ucvtf v10.2d, v19.2d", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Frintn, + rd: writable_vreg(20), + rn: vreg(7), + size: VectorSize::Size32x2, + }, + "F488210E", + "frintn v20.2s, v7.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Frintn, @@ -4221,6 +4287,17 @@ fn test_aarch64_binemit() { "frintn v12.2d, v17.2d", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Frintz, + rd: writable_vreg(1), + rn: vreg(30), + size: VectorSize::Size32x2, + }, + "C19BA10E", + "frintz v1.2s, v30.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Frintz, @@ -4243,6 +4320,17 @@ fn test_aarch64_binemit() { "frintz v12.2d, v17.2d", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Frintm, + rd: writable_vreg(15), + rn: vreg(7), + size: VectorSize::Size32x2, + }, + "EF98210E", + "frintm v15.2s, v7.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Frintm, @@ -4265,6 +4353,17 @@ fn test_aarch64_binemit() { "frintm v12.2d, v17.2d", )); + insns.push(( + Inst::VecMisc { + op: VecMisc2::Frintp, + rd: writable_vreg(3), + rn: vreg(4), + size: VectorSize::Size32x2, + }, + "8388A10E", + "frintp v3.2s, v4.2s", + )); + insns.push(( Inst::VecMisc { op: VecMisc2::Frintp, diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index ac4d958bb1..9e1aa34d28 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -846,6 +846,7 @@ pub enum Inst { }, /// Zero-extend a SIMD & FP scalar to the full width of a vector register. + /// 16-bit scalars require half-precision floating-point support (FEAT_FP16). FpuExtend { rd: Writable, rn: Reg, diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index 10c6807555..5b8e8b67f2 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -2387,6 +2387,13 @@ pub(crate) fn lower_insn_to_regs>( size, }); } else { + if size == VectorSize::Size32x2 { + return Err(CodegenError::Unsupported(format!( + "VallTrue: Unsupported type: {:?}", + src_ty + ))); + } + ctx.emit(Inst::VecLanes { op: VecLanesOp::Uminv, rd: tmp, @@ -2729,7 +2736,7 @@ pub(crate) fn lower_insn_to_regs>( Opcode::Imax | Opcode::Umax | Opcode::Umin | Opcode::Imin => { let ty = ty.unwrap(); - if !ty.is_vector() { + if !ty.is_vector() || ty.lane_bits() == 64 { return Err(CodegenError::Unsupported(format!( "{}: Unsupported type: {:?}", op, ty @@ -3264,11 +3271,19 @@ pub(crate) fn lower_insn_to_regs>( } Opcode::FcvtFromUint | Opcode::FcvtFromSint => { + let input_ty = ctx.input_ty(insn, 0); let ty = ty.unwrap(); let signed = op == Opcode::FcvtFromSint; let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap(); if ty.is_vector() { + if input_ty.lane_bits() != ty.lane_bits() { + return Err(CodegenError::Unsupported(format!( + "{}: Unsupported types: {:?} -> {:?}", + op, input_ty, ty + ))); + } + let op = if signed { VecMisc2::Scvtf } else { @@ -3283,7 +3298,6 @@ pub(crate) fn lower_insn_to_regs>( size: VectorSize::from_ty(ty), }); } else { - let input_ty = ctx.input_ty(insn, 0); let in_bits = ty_bits(input_ty); let out_bits = ty_bits(ty); let op = match (signed, in_bits, out_bits) { @@ -3315,12 +3329,20 @@ pub(crate) fn lower_insn_to_regs>( } Opcode::FcvtToUintSat | Opcode::FcvtToSintSat => { + let in_ty = ctx.input_ty(insn, 0); let ty = ty.unwrap(); let out_signed = op == Opcode::FcvtToSintSat; let rn = put_input_in_reg(ctx, inputs[0], NarrowValueMode::None); let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap(); if ty.is_vector() { + if in_ty.lane_bits() != ty.lane_bits() { + return Err(CodegenError::Unsupported(format!( + "{}: Unsupported types: {:?} -> {:?}", + op, in_ty, ty + ))); + } + let op = if out_signed { VecMisc2::Fcvtzs } else { @@ -3334,7 +3356,6 @@ pub(crate) fn lower_insn_to_regs>( size: VectorSize::from_ty(ty), }); } else { - let in_ty = ctx.input_ty(insn, 0); let in_bits = ty_bits(in_ty); let out_bits = ty_bits(ty); // FIMM Vtmp1, u32::MAX or u64::MAX or i32::MAX or i64::MAX @@ -3548,10 +3569,18 @@ pub(crate) fn lower_insn_to_regs>( }); } Opcode::AvgRound => { + let ty = ty.unwrap(); + + if ty.lane_bits() == 64 { + return Err(CodegenError::Unsupported(format!( + "AvgRound: Unsupported type: {:?}", + ty + ))); + } + let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap(); let rn = put_input_in_reg(ctx, inputs[0], NarrowValueMode::None); let rm = put_input_in_reg(ctx, inputs[1], NarrowValueMode::None); - let ty = ty.unwrap(); ctx.emit(Inst::VecRRR { alu_op: VecALUOp::Urhadd, rd,