Boolean and/or in cranelift

compiler/gen/src/crane/build.rs

@@ -5,6 +5,7 @@ use bumpalo::Bump;
 use cranelift::frontend::Switch;
 use cranelift::prelude::{
     AbiParam, ExternalName, FloatCC, FunctionBuilder, FunctionBuilderContext, IntCC, MemFlags,
+    Variable,
 };
 use cranelift_codegen::ir::entities::{StackSlot, Value};
 use cranelift_codegen::ir::stackslot::{StackSlotData, StackSlotKind};
@@ -38,6 +39,7 @@ pub struct Env<'a> {
     pub cfg: TargetFrontendConfig,
     pub interns: Interns,
     pub malloc: FuncId,
+    pub variable_counter: &'a mut u32,
 }
 
 impl<'a> Env<'a> {
@@ -47,15 +49,23 @@ impl<'a> Env<'a> {
     pub fn ptr_sized_int(&self) -> Type {
         Type::int(self.cfg.pointer_bits() as u16).unwrap()
     }
+
+    /// Cranelift creates variables by index.
+    /// For nested conditionals, we need unique variables
+    pub fn fresh_variable(&mut self) -> Variable {
+        let result = cranelift::frontend::Variable::with_u32(*self.variable_counter);
+        *self.variable_counter += 1;
+        result
+    }
 }
 
 pub fn build_expr<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     scope: &Scope,
     module: &mut Module<B>,
     builder: &mut FunctionBuilder,
-    expr: &Expr<'a>,
+    expr: &'a Expr<'a>,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) -> Value {
     use roc_mono::expr::Expr::*;
 
@@ -459,18 +469,18 @@ struct Branch2<'a> {
 }
 
 fn build_branch2<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     scope: &Scope,
     module: &mut Module<B>,
     builder: &mut FunctionBuilder,
     branch: Branch2<'a>,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) -> Value {
     let ret_layout = branch.ret_layout;
     let ret_type = type_from_layout(env.cfg, &ret_layout);
     // Declare a variable which each branch will mutate to be the value of that branch.
     // At the end of the expression, we will evaluate to this.
-    let ret = cranelift::frontend::Variable::with_u32(0);
+    let ret = env.fresh_variable();
 
     // The block we'll jump to once the switch has completed.
     let ret_block = builder.create_block();
@@ -530,12 +540,12 @@ struct SwitchArgs<'a> {
 }
 
 fn build_switch<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     scope: &Scope,
     module: &mut Module<B>,
     builder: &mut FunctionBuilder,
     switch_args: SwitchArgs<'a>,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) -> Value {
     let mut switch = Switch::new();
     let SwitchArgs {
@@ -633,7 +643,7 @@ fn build_switch<'a, B: Backend>(
 }
 
 pub fn declare_proc<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     module: &mut Module<B>,
     symbol: Symbol,
     proc: &Proc<'a>,
@@ -667,14 +677,14 @@ pub fn declare_proc<'a, B: Backend>(
 // TODO trim down these arguments
 #[allow(clippy::too_many_arguments)]
 pub fn define_proc_body<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     ctx: &mut Context,
     module: &mut Module<B>,
     fn_id: FuncId,
     scope: &Scope,
     sig: Signature,
-    proc: Proc<'a>,
+    proc: &'a Proc<'a>,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) {
     let args = proc.args;
     let cfg = env.cfg;
@@ -721,11 +731,11 @@ pub fn define_proc_body<'a, B: Backend>(
 
 fn build_arg<'a, B: Backend>(
     (arg, _): &'a (Expr<'a>, Layout<'a>),
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     scope: &Scope,
     module: &mut Module<B>,
     builder: &mut FunctionBuilder,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) -> Value {
     build_expr(env, scope, module, builder, arg, procs)
 }
@@ -733,13 +743,13 @@ fn build_arg<'a, B: Backend>(
 #[inline(always)]
 #[allow(clippy::cognitive_complexity)]
 fn call_by_name<'a, B: Backend>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     symbol: Symbol,
     args: &'a [(Expr<'a>, Layout<'a>)],
     scope: &Scope,
     module: &mut Module<B>,
     builder: &mut FunctionBuilder,
-    procs: &Procs<'a>,
+    procs: &'a Procs<'a>,
 ) -> Value {
     match symbol {
         Symbol::INT_ADD | Symbol::NUM_ADD => {
@@ -810,6 +820,30 @@ fn call_by_name<'a, B: Backend>(
 
             builder.ins().fcmp(FloatCC::Equal, a, b)
         }
+        Symbol::BOOL_OR => {
+            debug_assert!(args.len() == 2);
+
+            let branch2 = Branch2 {
+                cond: &args[0].0,
+                cond_layout: &Layout::Builtin(Builtin::Bool),
+                pass: &Expr::Bool(true),
+                fail: &args[1].0,
+                ret_layout: &Layout::Builtin(Builtin::Bool),
+            };
+            build_branch2(env, scope, module, builder, branch2, procs)
+        }
+        Symbol::BOOL_AND => {
+            debug_assert!(args.len() == 2);
+
+            let branch2 = Branch2 {
+                cond: &args[0].0,
+                cond_layout: &Layout::Builtin(Builtin::Bool),
+                pass: &args[1].0,
+                fail: &Expr::Bool(false),
+                ret_layout: &Layout::Builtin(Builtin::Bool),
+            };
+            build_branch2(env, scope, module, builder, branch2, procs)
+        }
         Symbol::LIST_GET_UNSAFE => {
             debug_assert!(args.len() == 2);
 
@@ -868,14 +902,10 @@ fn call_by_name<'a, B: Backend>(
                 Layout::Builtin(Builtin::List(elem_layout)) => {
                     let wrapper_ptr = clone_list(env, builder, module, wrapper_ptr, elem_layout);
 
-                    list_set_in_place(
+                    let arg1 = build_arg(&args[1], env, scope, module, builder, procs);
-                        env,
+                    let arg2 = build_arg(&args[2], env, scope, module, builder, procs);
-                        wrapper_ptr,
+
-                        build_arg(&args[1], env, scope, module, builder, procs),
+                    list_set_in_place(env, wrapper_ptr, arg1, arg2, elem_layout, builder);
-                        build_arg(&args[2], env, scope, module, builder, procs),
-                        elem_layout,
-                        builder,
-                    );
 
                     wrapper_ptr
                 }
@@ -892,14 +922,11 @@ fn call_by_name<'a, B: Backend>(
             let list_val = build_expr(env, scope, module, builder, list_expr, procs);
 
             match list_layout {
-                Layout::Builtin(Builtin::List(elem_layout)) => list_set_in_place(
+                Layout::Builtin(Builtin::List(elem_layout)) => {
-                    env,
+                    let arg1 = build_arg(&args[1], env, scope, module, builder, procs);
-                    list_val,
+                    let arg2 = build_arg(&args[2], env, scope, module, builder, procs);
-                    build_arg(&args[1], env, scope, module, builder, procs),
+                    list_set_in_place(env, list_val, arg1, arg2, elem_layout, builder)
-                    build_arg(&args[2], env, scope, module, builder, procs),
+                }
-                    elem_layout,
-                    builder,
-                ),
                 _ => {
                     unreachable!("Invalid List layout for List.set: {:?}", list_layout);
                 }
@@ -949,7 +976,7 @@ fn call_malloc<B: Backend>(
 }
 
 fn list_set_in_place<'a>(
-    env: &Env<'a>,
+    env: &mut Env<'a>,
     wrapper_ptr: Value,
     elem_index: Value,
     elem: Value,

compiler/gen/src/llvm/build.rs

@@ -35,6 +35,7 @@ pub struct Env<'a, 'ctx, 'env> {
     pub pointer_bytes: u32,
 }
 
+#[allow(clippy::cognitive_complexity)]
 pub fn build_expr<'a, 'ctx, 'env>(
     env: &Env<'a, 'ctx, 'env>,
     scope: &Scope<'a, 'ctx>,
@@ -816,12 +817,14 @@ fn build_basic_phi2<'a, 'ctx, 'env>(
 
     // build then block
     builder.position_at_end(then_block);
+    let then_val = pass;
     builder.build_unconditional_branch(cont_block);
 
     let then_block = builder.get_insert_block().unwrap();
 
     // build else block
     builder.position_at_end(else_block);
+    let else_val = fail;
     builder.build_unconditional_branch(cont_block);
 
     let else_block = builder.get_insert_block().unwrap();
@@ -831,7 +834,7 @@ fn build_basic_phi2<'a, 'ctx, 'env>(
 
     let phi = builder.build_phi(ret_type, "branch");
 
-    phi.add_incoming(&[(&pass, then_block), (&fail, else_block)]);
+    phi.add_incoming(&[(&then_val, then_block), (&else_val, else_block)]);
 
     phi.as_basic_value()
 }

compiler/gen/tests/test_gen.rs

@@ -70,7 +70,9 @@ mod test_gen {
                 arena: &arena,
                 interns,
                 cfg,
-                malloc
+                malloc,
+                variable_counter: &mut 0
+
             };
             let mut ident_ids = env.interns.all_ident_ids.remove(&home).unwrap();
 
@@ -87,7 +89,7 @@ mod test_gen {
             // can look up their Funcs in scope later when calling each other by value.
             for (name, opt_proc) in procs.as_map().into_iter() {
                 if let Some(proc) = opt_proc {
-                    let (func_id, sig) = declare_proc(&env, &mut module, name, &proc);
+                    let (func_id, sig) = declare_proc(&mut env, &mut module, name, &proc);
 
                     declared.push((proc.clone(), sig.clone(), func_id));
 
@@ -97,13 +99,13 @@ mod test_gen {
 
             for (proc, sig, fn_id) in declared {
                 define_proc_body(
-                    &env,
+                    &mut env,
                     &mut ctx,
                     &mut module,
                     fn_id,
                     &scope,
                     sig,
-                    proc,
+                    arena.alloc(proc),
                     &procs,
                 );
 
@@ -137,7 +139,7 @@ mod test_gen {
                 builder.append_block_params_for_function_params(block);
 
                 let main_body =
-                    roc_gen::crane::build::build_expr(&env, &scope, &mut module, &mut builder, &mono_expr, &procs);
+                    roc_gen::crane::build::build_expr(&mut env, &scope, &mut module, &mut builder, &mono_expr, &procs);
 
                 builder.ins().return_(&[main_body]);
                 // TODO re-enable this once Switch stops making unsealed blocks, e.g.

compiler/mono/src/decision_tree.rs

@@ -1,6 +1,7 @@
 use crate::expr::Env;
 use crate::expr::Expr;
 use crate::expr::Pattern;
+use bumpalo::Bump;
 use roc_collections::all::{MutMap, MutSet};
 use roc_module::ident::TagName;
 use roc_module::symbol::Symbol;
@@ -793,13 +794,13 @@ fn decide_to_branching<'a>(
                             is_unwrapped: union.alternatives.len() == 1,
                         };
 
-                        let cond = env.arena.alloc(Expr::CallByName(
+                        let cond = Expr::CallByName(
                             Symbol::INT_EQ_I64,
                             env.arena.alloc([
                                 (lhs, Layout::Builtin(Builtin::Int64)),
                                 (rhs, Layout::Builtin(Builtin::Int64)),
                             ]),
-                        ));
+                        );
 
                         tests.push(cond);
                     }
@@ -813,13 +814,13 @@ fn decide_to_branching<'a>(
                             env.arena.alloc([Layout::Builtin(Builtin::Int64)]),
                         );
 
-                        let cond = env.arena.alloc(Expr::CallByName(
+                        let cond = Expr::CallByName(
                             Symbol::INT_EQ_I64,
                             env.arena.alloc([
                                 (lhs, Layout::Builtin(Builtin::Int64)),
                                 (rhs, Layout::Builtin(Builtin::Int64)),
                             ]),
-                        ));
+                        );
 
                         tests.push(cond);
                     }
@@ -836,13 +837,13 @@ fn decide_to_branching<'a>(
                             env.arena.alloc([Layout::Builtin(Builtin::Float64)]),
                         );
 
-                        let cond = env.arena.alloc(Expr::CallByName(
+                        let cond = Expr::CallByName(
                             Symbol::FLOAT_EQ,
                             env.arena.alloc([
                                 (lhs, Layout::Builtin(Builtin::Float64)),
                                 (rhs, Layout::Builtin(Builtin::Float64)),
                             ]),
-                        ));
+                        );
 
                         tests.push(cond);
                     }
@@ -861,13 +862,13 @@ fn decide_to_branching<'a>(
                             env.arena.alloc([Layout::Builtin(Builtin::Byte)]),
                         );
 
-                        let cond = env.arena.alloc(Expr::CallByName(
+                        let cond = Expr::CallByName(
                             Symbol::INT_EQ_I8,
                             env.arena.alloc([
                                 (lhs, Layout::Builtin(Builtin::Byte)),
                                 (rhs, Layout::Builtin(Builtin::Byte)),
                             ]),
-                        ));
+                        );
 
                         tests.push(cond);
                     }
@@ -893,14 +894,12 @@ fn decide_to_branching<'a>(
                 jumps,
             ));
 
-            // TODO take the boolean and of all the tests
+            let cond = boolean_all(env.arena, tests);
-            debug_assert!(tests.len() == 1);
-            let cond = tests.remove(0);
 
             let cond_layout = Layout::Builtin(Builtin::Bool);
 
             Expr::Cond {
-                cond,
+                cond: env.arena.alloc(cond),
                 cond_layout,
                 pass,
                 fail,
@@ -965,6 +964,22 @@ fn decide_to_branching<'a>(
     }
 }
 
+fn boolean_all<'a>(arena: &'a Bump, tests: Vec<Expr<'a>>) -> Expr<'a> {
+    let mut expr = Expr::Bool(true);
+
+    for test in tests.into_iter().rev() {
+        expr = Expr::CallByName(
+            Symbol::BOOL_AND,
+            arena.alloc([
+                (test, Layout::Builtin(Builtin::Bool)),
+                (expr, Layout::Builtin(Builtin::Bool)),
+            ]),
+        );
+    }
+
+    expr
+}
+
 /// TREE TO DECIDER
 ///
 /// Decision trees may have some redundancies, so we convert them to a Decider