diff --git a/compiler/gen/src/crane/build.rs b/compiler/gen/src/crane/build.rs index d6c278be9a..5349788285 100644 --- a/compiler/gen/src/crane/build.rs +++ b/compiler/gen/src/crane/build.rs @@ -83,8 +83,10 @@ pub fn build_expr<'a, B: Backend>( cond_layout, ret_layout, } => { + let cond_value = load_symbol(env, scope, builder, *cond); + let branch = Branch2 { - cond, + cond: cond_value, pass, fail, cond_layout, @@ -115,7 +117,7 @@ pub fn build_expr<'a, B: Backend>( let mut scope = im_rc::HashMap::clone(scope); let cfg = env.cfg; - let ptr_size = cfg.pointer_bytes() as u32; + let ptr_size = cfg.pointer_bytes() as u32; for (name, layout, expr) in stores.iter() { let stack_size = layout.stack_size(ptr_size); @@ -127,10 +129,8 @@ pub fn build_expr<'a, B: Backend>( let val = build_expr(env, &scope, module, builder, &expr, procs); let expr_type = type_from_layout(cfg, &layout); - let slot = builder.create_stack_slot(StackSlotData::new( - StackSlotKind::ExplicitSlot, - stack_size, - )); + let slot = builder + .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, stack_size)); builder.ins().stack_store(val, slot, Offset32::new(0)); @@ -175,37 +175,7 @@ pub fn build_expr<'a, B: Backend>( results[0] } - Load(name) => match scope.get(name) { - Some(ScopeEntry::Stack { expr_type, slot }) => { - builder - .ins() - .stack_load(*expr_type, *slot, Offset32::new(0)) - } - Some(ScopeEntry::Arg { param, .. }) => *param, - Some(ScopeEntry::Heap { expr_type, ptr }) => { - builder - .ins() - .load(*expr_type, MemFlags::new(), *ptr, Offset32::new(0)) - } - Some(ScopeEntry::Func { .. }) => { - panic!("TODO I don't yet know how to return fn pointers") - } - Some(ScopeEntry::ZeroSized) => { - // Create a slot - let slot = builder.create_stack_slot(StackSlotData::new( - StackSlotKind::ExplicitSlot, - 0 - )); - - builder - .ins() - .stack_addr(env.cfg.pointer_type(), slot, Offset32::new(0)) - } - None => panic!( - "Could not resolve lookup for {:?} because no ScopeEntry was found for {:?} in scope {:?}", - name, name, scope - ), - }, + Load(name) => load_symbol(env, scope, builder, *name), Struct(sorted_fields) => { let cfg = env.cfg; @@ -217,10 +187,8 @@ pub fn build_expr<'a, B: Backend>( } // Create a slot - let slot = builder.create_stack_slot(StackSlotData::new( - StackSlotKind::ExplicitSlot, - slot_size - )); + let slot = builder + .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, slot_size)); // Create instructions for storing each field's expression // NOTE assumes that all fields have the same width! @@ -238,7 +206,11 @@ pub fn build_expr<'a, B: Backend>( .ins() .stack_addr(cfg.pointer_type(), slot, Offset32::new(0)) } - Tag { tag_layout, arguments, .. } => { + Tag { + tag_layout, + arguments, + .. + } => { let cfg = env.cfg; let ptr_bytes = cfg.pointer_bytes() as u32; @@ -248,10 +220,8 @@ pub fn build_expr<'a, B: Backend>( let slot_size = tag_layout.stack_size(ptr_bytes); // Create a slot - let slot = builder.create_stack_slot(StackSlotData::new( - StackSlotKind::ExplicitSlot, - slot_size - )); + let slot = builder + .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, slot_size)); // Create instructions for storing each field's expression let mut offset = 0; @@ -260,10 +230,12 @@ pub fn build_expr<'a, B: Backend>( let val = build_expr(env, &scope, module, builder, field_expr, procs); let field_size = field_layout.stack_size(ptr_bytes); - let field_offset = i32::try_from(offset) - .expect("TODO handle field size conversion to i32"); + let field_offset = + i32::try_from(offset).expect("TODO handle field size conversion to i32"); - builder.ins().stack_store(val, slot, Offset32::new(field_offset)); + builder + .ins() + .stack_store(val, slot, Offset32::new(field_offset)); offset += field_size; } @@ -281,8 +253,6 @@ pub fn build_expr<'a, B: Backend>( let cfg = env.cfg; let mut offset = 0; - - for (field_index, field_layout) in field_layouts.iter().enumerate() { if *index == field_index as u64 { let offset = i32::try_from(offset) @@ -301,8 +271,10 @@ pub fn build_expr<'a, B: Backend>( offset += field_layout.stack_size(ptr_bytes); } - panic!("field access out of bounds: index {:?} in layouts {:?}", index, field_layouts) - + panic!( + "field access out of bounds: index {:?} in layouts {:?}", + index, field_layouts + ) } Str(str_literal) => { @@ -375,14 +347,18 @@ pub fn build_expr<'a, B: Backend>( // Store the length { - let length = builder.ins().iconst(env.ptr_sized_int(), elems.len() as i64); + let length = builder + .ins() + .iconst(env.ptr_sized_int(), elems.len() as i64); let offset = Offset32::new((Builtin::WRAPPER_LEN * ptr_bytes) as i32); builder.ins().stack_store(length, slot, offset); } // Return the pointer to the wrapper - builder.ins().stack_addr(cfg.pointer_type(), slot, Offset32::new(0)) + builder + .ins() + .stack_addr(cfg.pointer_type(), slot, Offset32::new(0)) } _ => { panic!("I don't yet know how to crane build {:?}", expr); @@ -390,8 +366,47 @@ pub fn build_expr<'a, B: Backend>( } } +fn load_symbol<'a>( + env: &mut Env<'a>, + scope: &Scope, + builder: &mut FunctionBuilder, + name: Symbol, +) -> Value { + match scope.get(&name) { + Some(ScopeEntry::Stack { expr_type, slot }) => { + builder + .ins() + .stack_load(*expr_type, *slot, Offset32::new(0)) + } + Some(ScopeEntry::Arg { param, .. }) => *param, + Some(ScopeEntry::Heap { expr_type, ptr }) => { + builder + .ins() + .load(*expr_type, MemFlags::new(), *ptr, Offset32::new(0)) + } + Some(ScopeEntry::Func { .. }) => { + panic!("TODO I don't yet know how to return fn pointers") + } + Some(ScopeEntry::ZeroSized) => { + // Create a slot + let slot = builder.create_stack_slot(StackSlotData::new( + StackSlotKind::ExplicitSlot, + 0 + )); + + builder + .ins() + .stack_addr(env.cfg.pointer_type(), slot, Offset32::new(0)) + } + None => panic!( + "Could not resolve lookup for {:?} because no ScopeEntry was found for {:?} in scope {:?}", + name, name, scope + ), + } +} + struct Branch2<'a> { - cond: &'a Expr<'a>, + cond: Value, cond_layout: &'a Layout<'a>, pass: &'a Expr<'a>, fail: &'a Expr<'a>, @@ -417,7 +432,7 @@ fn build_branch2<'a, B: Backend>( builder.declare_var(ret, ret_type); - let cond = build_expr(env, scope, module, builder, branch.cond, procs); + let cond = branch.cond; let pass_block = builder.create_block(); let fail_block = builder.create_block(); @@ -763,8 +778,10 @@ fn call_by_name<'a, B: Backend>( Symbol::BOOL_OR => { debug_assert!(args.len() == 2); + let cond = build_arg(&args[0], env, scope, module, builder, procs); + let branch2 = Branch2 { - cond: &args[0].0, + cond, cond_layout: &Layout::Builtin(Builtin::Bool), pass: &Expr::Bool(true), fail: &args[1].0, @@ -775,8 +792,10 @@ fn call_by_name<'a, B: Backend>( Symbol::BOOL_AND => { debug_assert!(args.len() == 2); + let cond = build_arg(&args[0], env, scope, module, builder, procs); + let branch2 = Branch2 { - cond: &args[0].0, + cond, cond_layout: &Layout::Builtin(Builtin::Bool), pass: &args[1].0, fail: &Expr::Bool(false), diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 60d4d41dbd..d5b03e06a9 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -46,7 +46,7 @@ pub fn build_expr<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, scope: &Scope<'a, 'ctx>, parent: FunctionValue<'ctx>, - expr: &Expr<'a>, + expr: &'a Expr<'a>, procs: &Procs<'a>, ) -> BasicValueEnum<'ctx> { use roc_mono::expr::Expr::*; @@ -193,12 +193,7 @@ pub fn build_expr<'a, 'ctx, 'env>( .left() .unwrap_or_else(|| panic!("LLVM error: Invalid call by pointer.")) } - Load(symbol) => match scope.get(symbol) { - Some((_, ptr)) => env - .builder - .build_load(*ptr, symbol.ident_string(&env.interns)), - None => panic!("Could not find a var for {:?} in scope {:?}", symbol, scope), - }, + Load(symbol) => load_symbol(env, scope, symbol), Str(str_literal) => { if str_literal.is_empty() { panic!("TODO build an empty string in LLVM"); @@ -514,6 +509,19 @@ pub fn build_expr<'a, 'ctx, 'env>( } } +fn load_symbol<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + scope: &Scope<'a, 'ctx>, + symbol: &Symbol, +) -> BasicValueEnum<'ctx> { + match scope.get(symbol) { + Some((_, ptr)) => env + .builder + .build_load(*ptr, symbol.ident_string(&env.interns)), + None => panic!("Could not find a var for {:?} in scope {:?}", symbol, scope), + } +} + /// Cast a struct to another struct of the same (or smaller?) size fn cast_struct_struct<'ctx>( builder: &Builder<'ctx>, @@ -563,7 +571,7 @@ fn extract_tag_discriminant<'a, 'ctx, 'env>( } struct Branch2<'a> { - cond: &'a Expr<'a>, + cond: &'a Symbol, pass: &'a Expr<'a>, fail: &'a Expr<'a>, ret_layout: Layout<'a>, @@ -579,7 +587,7 @@ fn build_branch2<'a, 'ctx, 'env>( let ret_layout = cond.ret_layout; let ret_type = basic_type_from_layout(env.arena, env.context, &ret_layout, env.ptr_bytes); - let cond_expr = build_expr(env, scope, parent, cond.cond, procs); + let cond_expr = load_symbol(env, scope, cond.cond); match cond_expr { IntValue(value) => { diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 4fcdcd5ad1..bb1d64fe8a 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -1105,16 +1105,32 @@ fn decide_to_branching<'a>( jumps, )); - let cond = boolean_all(env.arena, tests); + let condition = boolean_all(env.arena, tests); let cond_layout = Layout::Builtin(Builtin::Bool); - Expr::Cond { - cond: env.arena.alloc(cond), - cond_layout, - pass, - fail, - ret_layout, + if let Expr::Load(symbol) = condition { + Expr::Cond { + cond: symbol, + cond_layout, + pass, + fail, + ret_layout, + } + } else { + let cond_symbol = env.fresh_symbol(); + let stores = vec![(cond_symbol, cond_layout.clone(), condition)]; + + Expr::Store( + env.arena.alloc(stores), + env.arena.alloc(Expr::Cond { + cond: cond_symbol, + cond_layout, + pass, + fail, + ret_layout, + }), + ) } } FanOut { diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index a959b855ce..a7d1e96f3e 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -152,7 +152,7 @@ pub enum Expr<'a> { // The left-hand side of the conditional comparison and the right-hand side. // These are stored separately because there are different machine instructions // for e.g. "compare float and jump" vs. "compare integer and jump" - cond: &'a Expr<'a>, + cond: Symbol, cond_layout: Layout<'a>, // What to do if the condition either passes or fails pass: &'a Expr<'a>, @@ -646,13 +646,22 @@ fn from_can<'a>( for (loc_cond, loc_then) in branches.into_iter().rev() { let cond = from_can(env, loc_cond.value, procs, None); let then = from_can(env, loc_then.value, procs, None); - expr = Expr::Cond { - cond: env.arena.alloc(cond), + + let cond_symbol = env.fresh_symbol(); + + let cond_expr = Expr::Cond { + cond: cond_symbol, cond_layout: cond_layout.clone(), pass: env.arena.alloc(then), fail: env.arena.alloc(expr), ret_layout: ret_layout.clone(), }; + + expr = Expr::Store( + env.arena + .alloc(vec![(cond_symbol, Layout::Builtin(Builtin::Bool), cond)]), + env.arena.alloc(cond_expr), + ); } expr diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 20f2a200f6..c4c9b9a660 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -16,6 +16,7 @@ mod test_mono { use roc_module::symbol::{Interns, Symbol}; use roc_mono::expr::Expr::{self, *}; use roc_mono::expr::Procs; + use roc_mono::layout; use roc_mono::layout::{Builtin, Layout}; use roc_types::subs::Subs; @@ -160,13 +161,23 @@ mod test_mono { use self::Builtin::*; use Layout::Builtin; - Cond { - cond: &Expr::Bool(true), - cond_layout: Builtin(Bool), - pass: &Expr::Str("bar"), - fail: &Expr::Str("foo"), - ret_layout: Builtin(Str), - } + let home = test_home(); + let gen_symbol_0 = Interns::from_index(home, 0); + + Store( + &[( + gen_symbol_0, + Layout::Builtin(layout::Builtin::Bool), + Expr::Bool(true), + )], + &Cond { + cond: gen_symbol_0, + cond_layout: Builtin(Bool), + pass: &Expr::Str("bar"), + fail: &Expr::Str("foo"), + ret_layout: Builtin(Str), + }, + ) }, ) } @@ -186,19 +197,37 @@ mod test_mono { use self::Builtin::*; use Layout::Builtin; - Cond { - cond: &Expr::Bool(true), - cond_layout: Builtin(Bool), - pass: &Expr::Str("bar"), - fail: &Cond { - cond: &Expr::Bool(false), + let home = test_home(); + let gen_symbol_0 = Interns::from_index(home, 1); + let gen_symbol_1 = Interns::from_index(home, 0); + + Store( + &[( + gen_symbol_0, + Layout::Builtin(layout::Builtin::Bool), + Expr::Bool(true), + )], + &Cond { + cond: gen_symbol_0, cond_layout: Builtin(Bool), - pass: &Expr::Str("foo"), - fail: &Expr::Str("baz"), + pass: &Expr::Str("bar"), + fail: &Store( + &[( + gen_symbol_1, + Layout::Builtin(layout::Builtin::Bool), + Expr::Bool(false), + )], + &Cond { + cond: gen_symbol_1, + cond_layout: Builtin(Bool), + pass: &Expr::Str("foo"), + fail: &Expr::Str("baz"), + ret_layout: Builtin(Str), + }, + ), ret_layout: Builtin(Str), }, - ret_layout: Builtin(Str), - } + ) }, ) } @@ -218,19 +247,27 @@ mod test_mono { use Layout::Builtin; let home = test_home(); + let gen_symbol_0 = Interns::from_index(home, 1); let symbol_x = Interns::from_index(home, 0); Store( &[( symbol_x, Builtin(Str), - Cond { - cond: &Expr::Bool(true), - cond_layout: Builtin(Bool), - pass: &Expr::Str("bar"), - fail: &Expr::Str("foo"), - ret_layout: Builtin(Str), - }, + Store( + &[( + gen_symbol_0, + Layout::Builtin(layout::Builtin::Bool), + Expr::Bool(true), + )], + &Cond { + cond: gen_symbol_0, + cond_layout: Builtin(Bool), + pass: &Expr::Str("bar"), + fail: &Expr::Str("foo"), + ret_layout: Builtin(Str), + }, + ), )], &Load(symbol_x), ) diff --git a/compiler/mono/tests/test_opt.rs b/compiler/mono/tests/test_opt.rs index 448dae46d2..2b8bffc974 100644 --- a/compiler/mono/tests/test_opt.rs +++ b/compiler/mono/tests/test_opt.rs @@ -114,13 +114,12 @@ mod test_opt { } Cond { - cond, + cond: _, cond_layout: _, pass, fail, ret_layout: _, } => { - extract_named_calls_help(cond, calls, unexpected_calls); extract_named_calls_help(pass, calls, unexpected_calls); extract_named_calls_help(fail, calls, unexpected_calls); }