From bacc7a9c6b14933c6c89259d46ad2897ca234bcb Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 13 Feb 2021 02:03:36 +0100 Subject: [PATCH 1/2] fuse RC operations on records/closures --- compiler/mono/src/expand_rc.rs | 100 +++++++++++++++++++++++++++------ compiler/mono/src/ir.rs | 6 +- compiler/mono/src/layout.rs | 4 +- 3 files changed, 90 insertions(+), 20 deletions(-) diff --git a/compiler/mono/src/expand_rc.rs b/compiler/mono/src/expand_rc.rs index 851430eaf4..34e36bde0c 100644 --- a/compiler/mono/src/expand_rc.rs +++ b/compiler/mono/src/expand_rc.rs @@ -157,6 +157,16 @@ impl<'a, 'i> Env<'a, 'i> { } } + fn insert_struct_info(&mut self, symbol: Symbol, fields: &'a [Layout<'a>]) { + self.constructor_map.insert(symbol, 0); + self.layout_map.insert(symbol, Layout::Struct(fields)); + } + + fn remove_struct_info(&mut self, symbol: Symbol) { + self.constructor_map.remove(&symbol); + self.layout_map.remove(&symbol); + } + pub fn unique_symbol(&mut self) -> Symbol { let ident_id = self.ident_ids.gen_unique(); @@ -213,6 +223,10 @@ fn layout_for_constructor<'a>( } } } + Struct(fields) => { + debug_assert_eq!(constructor, 0); + HasFields(fields) + } _ => unreachable!(), } } @@ -322,7 +336,39 @@ enum ConstructorLayout { Unknown, } -pub fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a Stmt<'a> { +pub fn expand_and_cancel_proc<'a>( + env: &mut Env<'a, '_>, + stmt: &'a Stmt<'a>, + arguments: &'a [(Layout<'a>, Symbol)], +) -> &'a Stmt<'a> { + let mut introduced = Vec::new_in(env.arena); + + for (layout, symbol) in arguments { + match layout { + Layout::Struct(fields) => { + env.insert_struct_info(*symbol, fields); + + introduced.push(*symbol); + } + Layout::Closure(_, closure_layout, _) => { + if let Layout::Struct(fields) = closure_layout.layout { + env.insert_struct_info(*symbol, fields); + } + } + _ => {} + } + } + + let result = expand_and_cancel(env, stmt); + + for symbol in introduced { + env.remove_struct_info(symbol); + } + + result +} + +fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a Stmt<'a> { use Stmt::*; let mut deferred_default = Deferred { @@ -351,7 +397,7 @@ pub fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a S // prevent long chains of `Let`s from blowing the stack let mut literal_stack = Vec::new_in(env.arena); - while !matches!(&expr, Expr::AccessAtIndex { .. } ) { + while !matches!(&expr, Expr::AccessAtIndex { .. } | Expr::Struct(_)) { if let Stmt::Let(symbol1, expr1, layout1, cont1) = cont { literal_stack.push((symbol, expr.clone(), layout.clone())); @@ -366,25 +412,43 @@ pub fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a S let new_cont; - if let Expr::AccessAtIndex { - structure, index, .. - } = &expr - { - let entry = env - .alias_map - .entry(*structure) - .or_insert_with(MutMap::default); + match &expr { + Expr::AccessAtIndex { + structure, index, .. + } => { + let entry = env + .alias_map + .entry(*structure) + .or_insert_with(MutMap::default); - entry.insert(*index, symbol); + entry.insert(*index, symbol); - new_cont = expand_and_cancel(env, cont); + new_cont = expand_and_cancel(env, cont); - // make sure to remove the alias, so other branches don't use it by accident - env.alias_map - .get_mut(structure) - .and_then(|map| map.remove(index)); - } else { - new_cont = expand_and_cancel(env, cont); + // make sure to remove the alias, so other branches don't use it by accident + env.alias_map + .get_mut(structure) + .and_then(|map| map.remove(index)); + } + Expr::Struct(_) => match layout { + Layout::Struct(fields) => { + env.insert_struct_info(symbol, fields); + + new_cont = expand_and_cancel(env, cont); + + env.remove_struct_info(symbol); + } + Layout::Closure(_, _closure_layout, _) => { + // TODO figure out if we can do better with closures + new_cont = expand_and_cancel(env, cont); + } + _ => { + unreachable!("struct must have struct layout, but got {:?}", layout); + } + }, + _ => { + new_cont = expand_and_cancel(env, cont); + } } let stmt = Let(symbol, expr.clone(), layout.clone(), new_cont); diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index bdfdc12dd9..54b1c16798 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -183,7 +183,11 @@ impl<'a> Proc<'a> { }; for (_, proc) in procs.iter_mut() { - let b = expand_rc::expand_and_cancel(&mut env, arena.alloc(proc.body.clone())); + let b = expand_rc::expand_and_cancel_proc( + &mut env, + arena.alloc(proc.body.clone()), + proc.args, + ); proc.body = b.clone(); } } diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 8c222164ec..6cfe7a9b72 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -70,7 +70,9 @@ pub struct ClosureLayout<'a> { /// the vec is likely to be small, so linear search is fine captured: &'a [(TagName, &'a [Layout<'a>])], - layout: &'a Layout<'a>, + /// use with care; there is some stuff happening here re. unwrapping + /// one-element records that might cause issues + pub layout: &'a Layout<'a>, } impl<'a> ClosureLayout<'a> { From 0f53665afa75b968b56d6c225716bfc67e418079 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 13 Feb 2021 02:37:41 +0100 Subject: [PATCH 2/2] fix tests --- compiler/mono/src/expand_rc.rs | 13 ++++--------- compiler/mono/tests/test_mono.rs | 3 +-- compiler/reporting/tests/test_reporting.rs | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/compiler/mono/src/expand_rc.rs b/compiler/mono/src/expand_rc.rs index 34e36bde0c..c05f232eb7 100644 --- a/compiler/mono/src/expand_rc.rs +++ b/compiler/mono/src/expand_rc.rs @@ -430,22 +430,17 @@ fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a Stmt< .get_mut(structure) .and_then(|map| map.remove(index)); } - Expr::Struct(_) => match layout { - Layout::Struct(fields) => { + Expr::Struct(_) => { + if let Layout::Struct(fields) = layout { env.insert_struct_info(symbol, fields); new_cont = expand_and_cancel(env, cont); env.remove_struct_info(symbol); - } - Layout::Closure(_, _closure_layout, _) => { - // TODO figure out if we can do better with closures + } else { new_cont = expand_and_cancel(env, cont); } - _ => { - unreachable!("struct must have struct layout, but got {:?}", layout); - } - }, + } _ => { new_cont = expand_and_cancel(env, cont); } diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 184836cc33..32b6a4d1c6 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -949,8 +949,7 @@ mod test_mono { let Test.5 = 3.14f64; let Test.3 = Struct {Test.4, Test.5}; let Test.1 = Index 0 Test.3; - inc Test.1; - dec Test.3; + decref Test.3; ret Test.1; "# ), diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 7b50c9e5a8..80b455d3b2 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -4697,7 +4697,7 @@ mod test_reporting { I was expecting to see a private tag name. - Hint: Private tag names start with a `@` symbol followed by an + Hint: Private tag names start with an `@` symbol followed by an uppercase letter, like @UID or @SecretKey. "# ),