From 698dc9c4019144db206a78882dbe1ba6558488ce Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 28 Apr 2020 17:40:16 +0200 Subject: [PATCH] Fixes #1619: Properly bubble up errors when seeing an unexpected type during lowering. --- cranelift/codegen/src/isa/aarch64/inst/mod.rs | 15 ++++---- cranelift/codegen/src/isa/aarch64/mod.rs | 8 +++-- cranelift/codegen/src/machinst/compile.rs | 6 ++-- cranelift/codegen/src/machinst/lower.rs | 36 +++++++++---------- cranelift/codegen/src/machinst/mod.rs | 5 +-- cranelift/codegen/src/result.rs | 7 ++++ 6 files changed, 45 insertions(+), 32 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 98776a490c..79f6bb691f 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -7,7 +7,7 @@ use crate::binemit::CodeOffset; use crate::ir::types::{B1, B16, B32, B64, B8, F32, F64, FFLAGS, I16, I32, I64, I8, IFLAGS}; use crate::ir::{ExternalName, Opcode, SourceLoc, TrapCode, Type}; use crate::machinst::*; -use crate::settings; +use crate::{settings, CodegenError, CodegenResult}; use regalloc::Map as RegallocMap; use regalloc::{RealReg, RealRegUniverse, Reg, RegClass, SpillSlot, VirtualReg, Writable}; @@ -1787,12 +1787,15 @@ impl MachInst for Inst { None } - fn rc_for_type(ty: Type) -> RegClass { + fn rc_for_type(ty: Type) -> CodegenResult { match ty { - I8 | I16 | I32 | I64 | B1 | B8 | B16 | B32 | B64 => RegClass::I64, - F32 | F64 => RegClass::V128, - IFLAGS | FFLAGS => RegClass::I64, - _ => panic!("Unexpected SSA-value type: {}", ty), + I8 | I16 | I32 | I64 | B1 | B8 | B16 | B32 | B64 => Ok(RegClass::I64), + F32 | F64 => Ok(RegClass::V128), + IFLAGS | FFLAGS => Ok(RegClass::I64), + _ => Err(CodegenError::Unsupported(format!( + "Unexpected SSA-value type: {}", + ty + ))), } } diff --git a/cranelift/codegen/src/isa/aarch64/mod.rs b/cranelift/codegen/src/isa/aarch64/mod.rs index 879788e32a..530035f43c 100644 --- a/cranelift/codegen/src/isa/aarch64/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/mod.rs @@ -34,7 +34,11 @@ impl AArch64Backend { /// This performs lowering to VCode, register-allocates the code, computes block layout and /// finalizes branches. The result is ready for binary emission. - fn compile_vcode(&self, func: &Function, flags: settings::Flags) -> VCode { + fn compile_vcode( + &self, + func: &Function, + flags: settings::Flags, + ) -> CodegenResult> { let abi = Box::new(abi::AArch64ABIBody::new(func, flags)); compile::compile::(func, self, abi) } @@ -47,7 +51,7 @@ impl MachBackend for AArch64Backend { want_disasm: bool, ) -> CodegenResult { let flags = self.flags(); - let vcode = self.compile_vcode(func, flags.clone()); + let vcode = self.compile_vcode(func, flags.clone())?; let sections = vcode.emit(); let frame_size = vcode.frame_size(); diff --git a/cranelift/codegen/src/machinst/compile.rs b/cranelift/codegen/src/machinst/compile.rs index 42374aea61..3034822aa5 100644 --- a/cranelift/codegen/src/machinst/compile.rs +++ b/cranelift/codegen/src/machinst/compile.rs @@ -14,12 +14,12 @@ pub fn compile( f: &Function, b: &B, abi: Box>, -) -> VCode +) -> CodegenResult> where B::MInst: ShowWithRRU, { // This lowers the CL IR. - let mut vcode = Lower::new(f, abi).lower(b); + let mut vcode = Lower::new(f, abi)?.lower(b)?; let universe = &B::MInst::reg_universe(vcode.flags()); @@ -62,5 +62,5 @@ where vcode.show_rru(Some(universe)) ); - vcode + Ok(vcode) } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 5397ab0361..b90b08b3be 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -10,7 +10,7 @@ use crate::ir::{ MemFlags, Opcode, Signature, SourceLoc, Type, Value, ValueDef, }; use crate::machinst::{ABIBody, BlockIndex, VCode, VCodeBuilder, VCodeInst}; -use crate::num_uses::NumUses; +use crate::{num_uses::NumUses, CodegenResult}; use regalloc::{Reg, RegClass, Set, VirtualReg, Writable}; @@ -144,7 +144,7 @@ enum GenerateReturn { impl<'func, I: VCodeInst> Lower<'func, I> { /// Prepare a new lowering context for the given IR function. - pub fn new(f: &'func Function, abi: Box>) -> Lower<'func, I> { + pub fn new(f: &'func Function, abi: Box>) -> CodegenResult> { let mut vcode = VCodeBuilder::new(abi); let num_uses = NumUses::compute(f).take_uses(); @@ -164,7 +164,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { for param in f.dfg.block_params(bb) { let vreg = alloc_vreg( &mut value_regs, - I::rc_for_type(f.dfg.value_type(*param)), + I::rc_for_type(f.dfg.value_type(*param))?, *param, &mut next_vreg, ); @@ -174,7 +174,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { for result in f.dfg.inst_results(inst) { let vreg = alloc_vreg( &mut value_regs, - I::rc_for_type(f.dfg.value_type(*result)), + I::rc_for_type(f.dfg.value_type(*result))?, *result, &mut next_vreg, ); @@ -188,20 +188,20 @@ impl<'func, I: VCodeInst> Lower<'func, I> { for ret in &f.signature.returns { let v = next_vreg; next_vreg += 1; - let regclass = I::rc_for_type(ret.value_type); + let regclass = I::rc_for_type(ret.value_type)?; let vreg = Reg::new_virtual(regclass, v); retval_regs.push((vreg, ret.extension)); vcode.set_vreg_type(vreg.as_virtual_reg().unwrap(), ret.value_type); } - Lower { + Ok(Lower { f, vcode, num_uses, value_regs, retval_regs, next_vreg, - } + }) } fn gen_arg_setup(&mut self) { @@ -267,7 +267,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { } /// Lower the function. - pub fn lower>(mut self, backend: &B) -> VCode { + pub fn lower>(mut self, backend: &B) -> CodegenResult> { // Find all reachable blocks. let bbs = self.find_reachable_bbs(); @@ -421,13 +421,12 @@ impl<'func, I: VCodeInst> Lower<'func, I> { ); // Create a temporary for each block parameter. - let phi_classes: Vec<(Type, RegClass)> = self + let phi_classes: Vec = self .f .dfg .block_params(orig_block) .iter() .map(|p| self.f.dfg.value_type(*p)) - .map(|ty| (ty, I::rc_for_type(ty))) .collect(); // FIXME sewardj 2020Feb29: use SmallVec @@ -455,28 +454,27 @@ impl<'func, I: VCodeInst> Lower<'func, I> { if !Set::::from_vec(src_regs.clone()).intersects(&Set::::from_vec( dst_regs.iter().map(|r| r.to_reg()).collect(), )) { - for (dst_reg, (src_reg, (ty, _))) in + for (dst_reg, (src_reg, ty)) in dst_regs.iter().zip(src_regs.iter().zip(phi_classes)) { self.vcode.push(I::gen_move(*dst_reg, *src_reg, ty)); } } else { // There's some overlap, so play safe and copy via temps. - - let tmp_regs: Vec> = phi_classes - .iter() - .map(|&(ty, rc)| self.tmp(rc, ty)) // borrows `self` mutably. - .collect(); + let mut tmp_regs = Vec::with_capacity(phi_classes.len()); + for &ty in &phi_classes { + tmp_regs.push(self.tmp(I::rc_for_type(ty)?, ty)); + } debug!("phi_temps = {:?}", tmp_regs); debug_assert!(tmp_regs.len() == src_regs.len()); - for (tmp_reg, (src_reg, &(ty, _))) in + for (tmp_reg, (src_reg, &ty)) in tmp_regs.iter().zip(src_regs.iter().zip(phi_classes.iter())) { self.vcode.push(I::gen_move(*tmp_reg, *src_reg, ty)); } - for (dst_reg, (tmp_reg, &(ty, _))) in + for (dst_reg, (tmp_reg, &ty)) in dst_regs.iter().zip(tmp_regs.iter().zip(phi_classes.iter())) { self.vcode.push(I::gen_move(*dst_reg, tmp_reg.to_reg(), ty)); @@ -495,7 +493,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { } // Now that we've emitted all instructions into the VCodeBuilder, let's build the VCode. - self.vcode.build() + Ok(self.vcode.build()) } /// Reduce the use-count of an IR instruction. Use this when, e.g., isel incorporates the diff --git a/cranelift/codegen/src/machinst/mod.rs b/cranelift/codegen/src/machinst/mod.rs index 554b661300..d9608fe036 100644 --- a/cranelift/codegen/src/machinst/mod.rs +++ b/cranelift/codegen/src/machinst/mod.rs @@ -163,8 +163,9 @@ pub trait MachInst: Clone + Debug { /// (e.g., add directly from or directly to memory), like x86. fn maybe_direct_reload(&self, reg: VirtualReg, slot: SpillSlot) -> Option; - /// Determine a register class to store the given CraneLift type. - fn rc_for_type(ty: Type) -> RegClass; + /// Determine a register class to store the given Cranelift type. + /// May return an error if the type isn't supported by this backend. + fn rc_for_type(ty: Type) -> CodegenResult; /// Generate a jump to another target. Used during lowering of /// control flow. diff --git a/cranelift/codegen/src/result.rs b/cranelift/codegen/src/result.rs index 4ed2c8f70e..5ea91f8a05 100644 --- a/cranelift/codegen/src/result.rs +++ b/cranelift/codegen/src/result.rs @@ -1,6 +1,7 @@ //! Result and error types representing the outcome of compiling a function. use crate::verifier::VerifierErrors; +use std::string::String; use thiserror::Error; /// A compilation error. @@ -31,6 +32,12 @@ pub enum CodegenError { #[error("Code for function is too large")] CodeTooLarge, + /// Something is not supported by the code generator. This might be an indication that a + /// feature is used without explicitly enabling it, or that something is temporarily + /// unsupported by a given target backend. + #[error("Unsupported feature: {0}")] + Unsupported(String), + /// A failure to map Cranelift register representation to a DWARF register representation. #[cfg(feature = "unwind")] #[error("Register mapping error")]