From d42c8692bc1a1014dac279ba553d8ba91f2f985b Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 25 Jun 2021 11:28:19 -0700 Subject: [PATCH] x64 backend: be explicit about unimplemented opcodes. As discussed in #3035, most backends have explicit `unimplemented!(...)` match-arms for opcode lowering cases that are not yet implemented; this allows the backend maintainer to easily see what is not yet implemented, and avoiding a catch-all wildcard arm is less error-prone as opcodes are added in the future. However, the x64 backend was the exception: as @akirilov-arm pointed out, it had a wildcard match arm. This fixes the issue by explicitly listing all opcodes the x64 backend does not yet implement. As per our tests, these opcodes are not used or need by Wasm lowering; but, it is good to know that they exist, so that we can eventually either support or remove them. This was a good exercise for me as I wasn't aware of a few of these in particular: e.g., aarch64 supports `bmask` while x64 does not, and there isn't a good reason why x64 shouldn't, especially if others hope to use Cranelift as a SIMD-capable general codegen in the future. The `unimplemented!()` cases are separate from `panic!()` ones: my convention here was to split out those that are logically just *missing* from those that should be *impossible*, mostly due to expected removal by legalization before we reach the lowering step. --- cranelift/codegen/src/isa/x64/lower.rs | 142 ++++++++++++++++++++++++- 1 file changed, 140 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index c9f52e71d282..7ef1a8d42435 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -5971,6 +5971,43 @@ fn lower_insn_to_regs>( } }, + // Unimplemented opcodes below. These are not currently used by Wasm + // lowering or other known embeddings, but should be either supported or + // removed eventually. + Opcode::Uload8x8Complex + | Opcode::Sload8x8Complex + | Opcode::Uload16x4Complex + | Opcode::Sload16x4Complex + | Opcode::Uload32x2Complex + | Opcode::Sload32x2Complex => { + unimplemented!("Vector load {:?} not implemented", op); + } + + Opcode::Cls => unimplemented!("Cls not supported"), + + Opcode::Fma => unimplemented!("Fma not supported"), + + Opcode::BorNot | Opcode::BxorNot => { + unimplemented!("or-not / xor-not opcodes not implemented"); + } + + Opcode::Bmask => unimplemented!("Bmask not implemented"), + + Opcode::Trueif | Opcode::Trueff => unimplemented!("trueif / trueff not implemented"), + + Opcode::ConstAddr => unimplemented!("ConstAddr not implemented"), + + Opcode::Vsplit | Opcode::Vconcat => { + 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 @@ -5996,10 +6033,111 @@ fn lower_insn_to_regs>( | Opcode::RotrImm | Opcode::IshlImm | Opcode::UshrImm - | Opcode::SshrImm => { + | Opcode::SshrImm + | Opcode::IcmpImm + | Opcode::IfcmpImm => { panic!("ALU+imm and ALU+carry ops should not appear here!"); } - _ => unimplemented!("unimplemented lowering for opcode {:?}", op), + + Opcode::StackLoad | Opcode::StackStore => { + panic!("Direct stack memory access not supported; should have been legalized"); + } + + Opcode::GlobalValue => { + panic!("global_value should have been removed by legalization!"); + } + + Opcode::HeapAddr => { + panic!("heap_addr should have been removed by legalization!"); + } + + Opcode::TableAddr => { + 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::Regmove + | Opcode::CopySpecial + | Opcode::CopyToSsa + | Opcode::CopyNop + | Opcode::AdjustSpDown + | Opcode::AdjustSpUpImm + | Opcode::AdjustSpDownImm + | Opcode::IfcmpSp + | Opcode::Regspill + | Opcode::Regfill + | Opcode::Copy + | Opcode::DummySargT => { + panic!("Unused opcode should not be encountered."); + } + + Opcode::JumpTableEntry | Opcode::JumpTableBase => { + panic!("Should not appear: we handle BrTable directly"); + } + + Opcode::Trapz | Opcode::Trapnz | Opcode::ResumableTrapnz => { + panic!("trapz / trapnz / resumable_trapnz should have been removed by legalization!"); + } + + Opcode::Jump + | Opcode::Fallthrough + | Opcode::Brz + | Opcode::Brnz + | Opcode::BrIcmp + | Opcode::Brif + | Opcode::Brff + | Opcode::IndirectJumpTableBr + | Opcode::BrTable => { + panic!("Branch opcode reached non-branch lowering logic!"); + } + + Opcode::X86Udivmodx + | Opcode::X86Sdivmodx + | Opcode::X86Umulx + | Opcode::X86Smulx + | Opcode::X86Cvtt2si + | Opcode::X86Fmin + | Opcode::X86Fmax + | Opcode::X86Push + | Opcode::X86Pop + | Opcode::X86Bsr + | Opcode::X86Bsf + | Opcode::X86Pblendw + | Opcode::X86Pshufd + | Opcode::X86Pshufb + | Opcode::X86Pextr + | Opcode::X86Pinsr + | Opcode::X86Insertps + | Opcode::X86Movsd + | Opcode::X86Movlhps + | Opcode::X86Palignr + | Opcode::X86Psll + | Opcode::X86Psrl + | Opcode::X86Psra + | Opcode::X86Ptest + | Opcode::X86Pmaxs + | Opcode::X86Pmaxu + | Opcode::X86Pmins + | Opcode::X86Pminu + | Opcode::X86Pmullq + | Opcode::X86Pmuludq + | Opcode::X86Punpckh + | Opcode::X86Punpckl + | Opcode::X86Vcvtudq2ps + | Opcode::X86ElfTlsGetAddr + | Opcode::X86MachoTlsGetAddr => { + panic!("x86-specific opcode in supposedly arch-neutral IR!"); + } + + Opcode::Nop => { + // Nothing. + } } Ok(())