From 38656cce35a78d2bedcdf54008a51fc22cfbbd67 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 3 Sep 2019 18:14:58 +0200 Subject: [PATCH] [meta] Simplify and comment instruction building a bit; --- cranelift/codegen/meta/src/cdsl/formats.rs | 4 +- .../codegen/meta/src/cdsl/instructions.rs | 136 +++++++++--------- 2 files changed, 67 insertions(+), 73 deletions(-) diff --git a/cranelift/codegen/meta/src/cdsl/formats.rs b/cranelift/codegen/meta/src/cdsl/formats.rs index a3249b4c6e..a0dcbb67ae 100644 --- a/cranelift/codegen/meta/src/cdsl/formats.rs +++ b/cranelift/codegen/meta/src/cdsl/formats.rs @@ -189,7 +189,9 @@ impl FormatRegistry { if operand.is_value() { num_values += 1; } - has_varargs = has_varargs || operand.is_varargs(); + if !has_varargs { + has_varargs = operand.is_varargs(); + } if let Some(imm_key) = operand.kind.imm_key() { imm_keys.push(imm_key); } diff --git a/cranelift/codegen/meta/src/cdsl/instructions.rs b/cranelift/codegen/meta/src/cdsl/instructions.rs index f31e530771..da59beb2b0 100644 --- a/cranelift/codegen/meta/src/cdsl/instructions.rs +++ b/cranelift/codegen/meta/src/cdsl/instructions.rs @@ -113,9 +113,12 @@ pub struct InstructionContent { /// polymorphic, set otherwise. pub polymorphic_info: Option, + /// Indices in operands_in of input operands that are values. pub value_opnums: Vec, - pub value_results: Vec, + /// Indices in operands_in of input operands that are immediates or entities. pub imm_opnums: Vec, + /// Indices in operands_out of output operands that are values. + pub value_results: Vec, /// True for instructions that terminate the EBB. pub is_terminator: bool, @@ -332,8 +335,6 @@ impl InstructionBuilder { let operands_in = self.operands_in.unwrap_or_else(Vec::new); let operands_out = self.operands_out.unwrap_or_else(Vec::new); - let format_index = format_registry.lookup(&operands_in); - let mut value_opnums = Vec::new(); let mut imm_opnums = Vec::new(); for (i, op) in operands_in.iter().enumerate() { @@ -346,13 +347,13 @@ impl InstructionBuilder { } } - let mut value_results = Vec::new(); - for (i, op) in operands_out.iter().enumerate() { - if op.is_value() { - value_results.push(i); - } - } + let value_results = operands_out + .iter() + .enumerate() + .filter_map(|(i, op)| if op.is_value() { Some(i) } else { None }) + .collect(); + let format_index = format_registry.lookup(&operands_in); let format = format_registry.get(format_index); let polymorphic_info = verify_polymorphic(&operands_in, &operands_out, &format, &value_opnums); @@ -461,12 +462,8 @@ fn verify_polymorphic( } // Verify the use of type variables. - let mut use_typevar_operand = false; - let mut ctrl_typevar = None; - let mut other_typevars = None; - let mut maybe_error_message = None; - let tv_op = format.typevar_operand; + let mut maybe_error_message = None; if let Some(tv_op) = tv_op { if tv_op < value_opnums.len() { let op_num = value_opnums[tv_op]; @@ -475,11 +472,13 @@ fn verify_polymorphic( if (free_typevar.is_some() && tv == &free_typevar.unwrap()) || tv.singleton_type().is_some() { - match verify_ctrl_typevar(tv, &value_opnums, &operands_in, &operands_out) { - Ok(typevars) => { - other_typevars = Some(typevars); - ctrl_typevar = Some(tv.clone()); - use_typevar_operand = true; + match is_ctrl_typevar_candidate(tv, &operands_in, &operands_out) { + Ok(other_typevars) => { + return Some(PolymorphicInfo { + use_typevar_operand: true, + ctrl_typevar: tv.clone(), + other_typevars, + }); } Err(error_message) => { maybe_error_message = Some(error_message); @@ -489,33 +488,32 @@ fn verify_polymorphic( } }; - if !use_typevar_operand { - if operands_out.len() == 0 { - match maybe_error_message { - Some(msg) => panic!(msg), - None => panic!("typevar_operand must be a free type variable"), - } + // If we reached here, it means the type variable indicated as the typevar operand couldn't + // control every other input and output type variable. We need to look at the result type + // variables. + if operands_out.len() == 0 { + // No result means no other possible type variable, so it's a type inference failure. + match maybe_error_message { + Some(msg) => panic!(msg), + None => panic!("typevar_operand must be a free type variable"), } - - let tv = operands_out[0].type_var().unwrap(); - let free_typevar = tv.free_typevar(); - if free_typevar.is_some() && tv != &free_typevar.unwrap() { - panic!("first result must be a free type variable"); - } - - other_typevars = - Some(verify_ctrl_typevar(tv, &value_opnums, &operands_in, &operands_out).unwrap()); - ctrl_typevar = Some(tv.clone()); } - // rustc is not capable to determine this statically, so enforce it with options. - assert!(ctrl_typevar.is_some()); - assert!(other_typevars.is_some()); + // Otherwise, try to infer the controlling type variable by looking at the first result. + let tv = operands_out[0].type_var().unwrap(); + let free_typevar = tv.free_typevar(); + if free_typevar.is_some() && tv != &free_typevar.unwrap() { + panic!("first result must be a free type variable"); + } + + // At this point, if the next unwrap() fails, it means the output type couldn't be used as a + // controlling type variable either; panicking is the right behavior. + let other_typevars = is_ctrl_typevar_candidate(tv, &operands_in, &operands_out).unwrap(); Some(PolymorphicInfo { - use_typevar_operand, - ctrl_typevar: ctrl_typevar.unwrap(), - other_typevars: other_typevars.unwrap(), + use_typevar_operand: false, + ctrl_typevar: tv.clone(), + other_typevars, }) } @@ -527,57 +525,51 @@ fn verify_polymorphic( /// /// All polymorphic results must be derived from `ctrl_typevar`. /// -/// Return a vector of other type variables used, or panics. -fn verify_ctrl_typevar( +/// Return a vector of other type variables used, or a string explaining what went wrong. +fn is_ctrl_typevar_candidate( ctrl_typevar: &TypeVar, - value_opnums: &Vec, operands_in: &Vec, operands_out: &Vec, ) -> Result, String> { let mut other_typevars = Vec::new(); // Check value inputs. - for &op_num in value_opnums { - let typ = operands_in[op_num].type_var(); + for input in operands_in { + if !input.is_value() { + continue; + } - let tv = if let Some(typ) = typ { - typ.free_typevar() - } else { - None - }; + let typ = input.type_var().unwrap(); + let free_typevar = typ.free_typevar(); // Non-polymorphic or derived from ctrl_typevar is OK. - let tv = match tv { - Some(tv) => { - if &tv == ctrl_typevar { - continue; - } - tv - } - None => continue, - }; + if free_typevar.is_none() { + continue; + } + let free_typevar = free_typevar.unwrap(); + if &free_typevar == ctrl_typevar { + continue; + } // No other derived typevars allowed. - if typ.is_some() && typ.unwrap() != &tv { + if typ != &free_typevar { return Err(format!( - "{:?}: type variable {} must be derived from {:?}", - operands_in[op_num], - typ.unwrap().name, - ctrl_typevar + "{:?}: type variable {} must be derived from {:?} while it is derived from {:?}", + input, typ.name, ctrl_typevar, free_typevar )); } // Other free type variables can only be used once each. for other_tv in &other_typevars { - if &tv == other_tv { + if &free_typevar == other_tv { return Err(format!( - "type variable {} can't be used more than once", - tv.name + "non-controlling type variable {} can't be used more than once", + free_typevar.name )); } } - other_typevars.push(tv); + other_typevars.push(free_typevar); } // Check outputs. @@ -587,10 +579,10 @@ fn verify_ctrl_typevar( } let typ = result.type_var().unwrap(); - let tv = typ.free_typevar(); + let free_typevar = typ.free_typevar(); - // Non-polymorphic or derived form ctrl_typevar is OK. - if tv.is_none() || &tv.unwrap() == ctrl_typevar { + // Non-polymorphic or derived from ctrl_typevar is OK. + if free_typevar.is_none() || &free_typevar.unwrap() == ctrl_typevar { continue; }