Infallible extractors, and some fixes to fallibility in return types (Option<T> vs T).

cranelift/isle/TODO

@@ -1,8 +1,10 @@
-- Optimizations
+- Document the semantics of the DSL!
-  - Infallible patterns; optimize away control flow when possible.
-  - Don't do the closure-wrapping thing for expressions inside of patterns.
 
-- 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
+- Look into whether optimizations are possible:
-  for instruction info.
+  - 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);`)

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)

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<Vec<ArgPolarity>>,
+        /// 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 {

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::<Vec<_>>()
                 .join(", "),
+            if sig.infallible { "" } else { "Option<" },
             sig.ret_tys
                 .iter()
                 .map(|&ty| self.type_name(ty, /* by_ref = */ false))
                 .collect::<Vec<_>>()
-                .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::<Vec<_>>();
 
+                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(", "),
+                    self.value_name(&output),
-                    sig.full_name,
+                    val
-                    input_values.join(", "),
                 )?;
-
+                self.define_val(&output, ctx, /* is_ref = */ false, ty);
-                Ok(false)
+                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)?;
+                                    self.generate_body(code, depth + 1, node, i, ctx)?;
-                                writeln!(code, "{}}}", indent)?;
+                                if !infallible {
+                                    writeln!(code, "{}}}", indent)?;
+                                }
                                 if infallible && sub_returned {
                                     returned = true;
                                     break;

cranelift/isle/src/ir.rs

@@ -50,6 +50,7 @@ pub enum PatternInst {
         input_tys: Vec<TypeId>,
         output_tys: Vec<TypeId>,
         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<TypeId>,
         output_tys: Vec<TypeId>,
         term: TermId,
+        infallible: bool,
     ) -> Vec<Value> {
         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 =
+                                let arg_values = self
-                                    self.add_extract(inputs, input_tys, output_tys, term);
+                                    .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 { .. } => {
+                    &TermKind::InternalConstructor => {
-                        self.add_construct(&arg_values_tys[..], ty, term)
+                        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"),
                 }

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(

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<ArgPolarity>,
+        /// 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<TypeId>,
     pub ret_tys: Vec<TypeId>,
+    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,
                             };
                         }
                         _ => {