From a412cce615de538086f6b14fd565d2be614a566e Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 9 Sep 2021 15:33:15 -0700 Subject: [PATCH] Infallible extractors, and some fixes to fallibility in return types (Option vs T). --- cranelift/isle/TODO | 14 ++--- cranelift/isle/isle_examples/test3.isle | 4 +- cranelift/isle/src/ast.rs | 5 ++ cranelift/isle/src/codegen.rs | 71 +++++++++++++++++++----- cranelift/isle/src/ir.rs | 72 ++++++++++++++++++++++--- cranelift/isle/src/parser.rs | 10 ++++ cranelift/isle/src/sema.rs | 9 ++++ 7 files changed, 156 insertions(+), 29 deletions(-) diff --git a/cranelift/isle/TODO b/cranelift/isle/TODO index b6e0acff40..0e7f3f3759 100644 --- a/cranelift/isle/TODO +++ b/cranelift/isle/TODO @@ -1,8 +1,10 @@ -- Optimizations - - Infallible patterns; optimize away control flow when possible. - - Don't do the closure-wrapping thing for expressions inside of patterns. +- Document the semantics of the DSL! -- Document semantics carefully, especially wrt extractors. +- Clean up and factor the codegen properly. -- Build out an initial set of bindings for Cranelift LowerCtx with extractors - for instruction info. +- Look into whether optimizations are possible: + - More in-depth fallibility analysis (avoid failure edges where possible) + +- Slightly nicer human-readable generated code + - Include full rule body (S-expression) in comment, not just line number + - Inline some expressions (no more `let val23 = 1234; ... f(val23);`) \ No newline at end of file diff --git a/cranelift/isle/isle_examples/test3.isle b/cranelift/isle/isle_examples/test3.isle index df4c7337cd..e13c21d0f8 100644 --- a/cranelift/isle/isle_examples/test3.isle +++ b/cranelift/isle/isle_examples/test3.isle @@ -10,10 +10,10 @@ (type u32 (primitive u32)) (decl Op (Opcode) Inst) -(extern extractor Op get_opcode) +(extern extractor infallible Op get_opcode) (decl InstInput (InstInput u32) Inst) -(extern extractor InstInput get_inst_input (out in)) +(extern extractor infallible InstInput get_inst_input (out in)) (decl Producer (Inst) InstInput) (extern extractor Producer get_input_producer) diff --git a/cranelift/isle/src/ast.rs b/cranelift/isle/src/ast.rs index 8742aaa166..a0d6258485 100644 --- a/cranelift/isle/src/ast.rs +++ b/cranelift/isle/src/ast.rs @@ -171,6 +171,11 @@ pub enum Extern { /// more precisely the term value that we are extracting, is /// an "input"). arg_polarity: Option>, + /// Infallibility: if an external extractor returns `(T1, T2, + /// ...)` rather than `Option<(T1, T2, ...)>`, and hence can + /// never fail, it is declared as such and allows for slightly + /// better code to be generated. + infallible: bool, }, /// An external constructor: `(constructor Term rustfunc)` form. Constructor { diff --git a/cranelift/isle/src/codegen.rs b/cranelift/isle/src/codegen.rs index 3e29005926..bb334e4f47 100644 --- a/cranelift/isle/src/codegen.rs +++ b/cranelift/isle/src/codegen.rs @@ -567,7 +567,7 @@ impl<'a> Codegen<'a> { ) -> Result<(), Error> { writeln!( code, - "{}fn {}(&mut self, {}) -> Option<({},)>;", + "{}fn {}(&mut self, {}) -> {}({},){};", indent, sig.func_name, sig.arg_tys @@ -576,11 +576,13 @@ impl<'a> Codegen<'a> { .map(|(i, &ty)| format!("arg{}: {}", i, self.type_name(ty, /* by_ref = */ true))) .collect::>() .join(", "), + if sig.infallible { "" } else { "Option<" }, sig.ret_tys .iter() .map(|&ty| self.type_name(ty, /* by_ref = */ false)) .collect::>() - .join(", ") + .join(", "), + if sig.infallible { "" } else { ">" }, )?; Ok(()) } @@ -822,7 +824,10 @@ impl<'a> Codegen<'a> { self.define_val(&output, ctx, /* is_ref = */ false, ty); } &ExprInst::Construct { - ref inputs, term, .. + ref inputs, + term, + infallible, + .. } => { let mut input_exprs = vec![]; for (input_value, _) in inputs { @@ -838,13 +843,15 @@ impl<'a> Codegen<'a> { let termdata = &self.termenv.terms[term.index()]; let sig = termdata.to_sig(self.typeenv).unwrap(); assert_eq!(input_exprs.len(), sig.arg_tys.len()); + let fallible_try = if infallible { "" } else { "?" }; writeln!( code, - "{}let {} = {}(ctx, {});", + "{}let {} = {}(ctx, {}){};", indent, outputname, sig.full_name, input_exprs.join(", "), + fallible_try, )?; self.define_val(&output, ctx, /* is_ref = */ false, termdata.ret_ty); } @@ -905,7 +912,6 @@ impl<'a> Codegen<'a> { _ => true, }; writeln!(code, "{}let {} = arg{};", indent, outputname, index)?; - writeln!(code, "{}{{", indent)?; self.define_val( &Value::Pattern { inst: id, @@ -961,6 +967,7 @@ impl<'a> Codegen<'a> { ref inputs, ref output_tys, term, + infallible, .. } => { let termdata = &self.termenv.terms[term.index()]; @@ -983,18 +990,51 @@ impl<'a> Codegen<'a> { }) .collect::>(); + if infallible { + writeln!( + code, + "{}let ({},) = {}(ctx, {});", + indent, + output_binders.join(", "), + sig.full_name, + input_values.join(", "), + )?; + Ok(true) + } else { + writeln!( + code, + "{}if let Some(({},)) = {}(ctx, {}) {{", + indent, + output_binders.join(", "), + sig.full_name, + input_values.join(", "), + )?; + Ok(false) + } + } + &PatternInst::Expr { + ref seq, output_ty, .. + } if seq.is_const_int().is_some() => { + let (ty, val) = seq.is_const_int().unwrap(); + assert_eq!(ty, output_ty); + + let output = Value::Pattern { + inst: id, + output: 0, + }; writeln!( code, - "{}if let Some(({},)) = {}(ctx, {}) {{", + "{}let {} = {};", indent, - output_binders.join(", "), - sig.full_name, - input_values.join(", "), + self.value_name(&output), + val )?; - - Ok(false) + self.define_val(&output, ctx, /* is_ref = */ false, ty); + Ok(true) } - &PatternInst::Expr { ref seq, output_ty, .. } => { + &PatternInst::Expr { + ref seq, output_ty, .. + } => { let closure_name = format!("closure{}", id.index()); writeln!(code, "{}let {} = || {{", indent, closure_name)?; let subindent = format!("{} ", indent); @@ -1124,9 +1164,12 @@ impl<'a> Codegen<'a> { let id = InstId(depth); let infallible = self.generate_pattern_inst(code, id, op, indent, ctx)?; + let i = if infallible { indent } else { &subindent[..] }; let sub_returned = - self.generate_body(code, depth + 1, node, &subindent, ctx)?; - writeln!(code, "{}}}", indent)?; + self.generate_body(code, depth + 1, node, i, ctx)?; + if !infallible { + writeln!(code, "{}}}", indent)?; + } if infallible && sub_returned { returned = true; break; diff --git a/cranelift/isle/src/ir.rs b/cranelift/isle/src/ir.rs index 19802b4f1e..a8f88dff6f 100644 --- a/cranelift/isle/src/ir.rs +++ b/cranelift/isle/src/ir.rs @@ -50,6 +50,7 @@ pub enum PatternInst { input_tys: Vec, output_tys: Vec, term: TermId, + infallible: bool, }, /// Evaluate an expression and provide the given value as the @@ -80,6 +81,7 @@ pub enum ExprInst { inputs: Vec<(Value, TypeId)>, ty: TypeId, term: TermId, + infallible: bool, }, /// Set the Nth return value. Produces no values. @@ -130,6 +132,34 @@ pub struct ExprSequence { pub pos: Pos, } +impl ExprSequence { + pub fn is_const_int(&self) -> Option<(TypeId, i64)> { + if self.insts.len() == 2 && matches!(&self.insts[1], &ExprInst::Return { .. }) { + match &self.insts[0] { + &ExprInst::ConstInt { ty, val } => Some((ty, val)), + _ => None, + } + } else { + None + } + } + + pub fn is_const_variant(&self) -> Option<(TypeId, VariantId)> { + if self.insts.len() == 2 && matches!(&self.insts[1], &ExprInst::Return { .. }) { + match &self.insts[0] { + &ExprInst::CreateVariant { + ref inputs, + ty, + variant, + } if inputs.len() == 0 => Some((ty, variant)), + _ => None, + } + } else { + None + } + } +} + #[derive(Clone, Copy, Debug)] enum ValueOrArgs { Value(Value), @@ -195,6 +225,7 @@ impl PatternSequence { input_tys: Vec, output_tys: Vec, term: TermId, + infallible: bool, ) -> Vec { let inst = InstId(self.insts.len()); let mut outs = vec![]; @@ -208,6 +239,7 @@ impl PatternSequence { input_tys, output_tys, term, + infallible, }); outs } @@ -320,7 +352,9 @@ impl PatternSequence { panic!("Should have been expanded away"); } &TermKind::ExternalExtractor { - ref arg_polarity, .. + ref arg_polarity, + infallible, + .. } => { // Evaluate all `input` args. let mut inputs = vec![]; @@ -359,8 +393,8 @@ impl PatternSequence { } // Invoke the extractor. - let arg_values = - self.add_extract(inputs, input_tys, output_tys, term); + let arg_values = self + .add_extract(inputs, input_tys, output_tys, term, infallible); for (pat, &val) in output_pats.iter().zip(arg_values.iter()) { self.gen_pattern( @@ -417,10 +451,21 @@ impl ExprSequence { Value::Expr { inst, output: 0 } } - fn add_construct(&mut self, inputs: &[(Value, TypeId)], ty: TypeId, term: TermId) -> Value { + fn add_construct( + &mut self, + inputs: &[(Value, TypeId)], + ty: TypeId, + term: TermId, + infallible: bool, + ) -> Value { let inst = InstId(self.insts.len()); let inputs = inputs.iter().cloned().collect(); - self.add_inst(ExprInst::Construct { inputs, ty, term }); + self.add_inst(ExprInst::Construct { + inputs, + ty, + term, + infallible, + }); Value::Expr { inst, output: 0 } } @@ -469,8 +514,21 @@ impl ExprSequence { &TermKind::EnumVariant { variant } => { self.add_create_variant(&arg_values_tys[..], ty, variant) } - &TermKind::InternalConstructor | &TermKind::ExternalConstructor { .. } => { - self.add_construct(&arg_values_tys[..], ty, term) + &TermKind::InternalConstructor => { + self.add_construct( + &arg_values_tys[..], + ty, + term, + /* infallible = */ true, + ) + } + &TermKind::ExternalConstructor { .. } => { + self.add_construct( + &arg_values_tys[..], + ty, + term, + /* infallible = */ false, + ) } _ => panic!("Should have been caught by typechecking"), } diff --git a/cranelift/isle/src/parser.rs b/cranelift/isle/src/parser.rs index 7ec4fdc0ea..c872dd28d5 100644 --- a/cranelift/isle/src/parser.rs +++ b/cranelift/isle/src/parser.rs @@ -256,8 +256,17 @@ impl<'a> Parser<'a> { }) } else if self.is_sym_str("extractor") { self.symbol()?; + + let infallible = if self.is_sym_str("infallible") { + self.symbol()?; + true + } else { + false + }; + let term = self.parse_ident()?; let func = self.parse_ident()?; + let arg_polarity = if self.is_lparen() { let mut pol = vec![]; self.lparen()?; @@ -284,6 +293,7 @@ impl<'a> Parser<'a> { func, pos: pos.unwrap(), arg_polarity, + infallible, }) } else { Err(self.error( diff --git a/cranelift/isle/src/sema.rs b/cranelift/isle/src/sema.rs index 72aa779c69..fe55369b70 100644 --- a/cranelift/isle/src/sema.rs +++ b/cranelift/isle/src/sema.rs @@ -120,6 +120,8 @@ pub enum TermKind { name: Sym, /// Which arguments of the extractor are inputs and which are outputs? arg_polarity: Vec, + /// Is the external extractor infallible? + infallible: bool, }, /// A term defined solely by an external constructor function. ExternalConstructor { @@ -138,6 +140,7 @@ pub struct ExternalSig { pub full_name: String, pub arg_tys: Vec, pub ret_tys: Vec, + pub infallible: bool, } impl Term { @@ -180,10 +183,12 @@ impl Term { full_name: format!("C::{}", tyenv.syms[name.index()]), arg_tys: self.arg_tys.clone(), ret_tys: vec![self.ret_ty], + infallible: true, }), &TermKind::ExternalExtractor { name, ref arg_polarity, + infallible, } => { let mut arg_tys = vec![]; let mut ret_tys = vec![]; @@ -203,6 +208,7 @@ impl Term { full_name: format!("C::{}", tyenv.syms[name.index()]), arg_tys, ret_tys, + infallible, }) } &TermKind::InternalConstructor { .. } => { @@ -212,6 +218,7 @@ impl Term { full_name: name, arg_tys: self.arg_tys.clone(), ret_tys: vec![self.ret_ty], + infallible: false, }) } _ => None, @@ -688,6 +695,7 @@ impl TermEnv { ref func, pos, ref arg_polarity, + infallible, }) => { let term_sym = tyenv.intern_mut(term); let func_sym = tyenv.intern_mut(func); @@ -717,6 +725,7 @@ impl TermEnv { termdata.kind = TermKind::ExternalExtractor { name: func_sym, arg_polarity, + infallible, }; } _ => {