From 1f2da3d0c7c54e430d346700776b642f1d62eacc Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Sat, 14 Nov 2020 12:22:31 +0100 Subject: [PATCH 01/37] refactor repl for readability --- cli/src/repl.rs | 691 +-------------------------------- cli/src/repl/debug.rs | 109 ++++++ cli/src/repl/gen.rs | 267 +++++++++++++ compiler/gen/src/llvm/build.rs | 2 - 4 files changed, 384 insertions(+), 685 deletions(-) create mode 100644 cli/src/repl/debug.rs create mode 100644 cli/src/repl/gen.rs diff --git a/cli/src/repl.rs b/cli/src/repl.rs index a7f4fc3e21..b965b10147 100644 --- a/cli/src/repl.rs +++ b/cli/src/repl.rs @@ -1,32 +1,7 @@ -use bumpalo::Bump; -use inkwell::context::Context; -use roc_build::link::module_to_dylib; -use roc_builtins::unique::uniq_stdlib; -use roc_can::constraint::Constraint; -use roc_can::expected::Expected; -use roc_can::expr::{canonicalize_expr, Expr, Output}; -use roc_can::operator; -use roc_can::scope::Scope; -use roc_collections::all::{ImMap, ImSet, MutMap, MutSet, SendMap, SendSet}; -use roc_constrain::expr::constrain_expr; -use roc_constrain::module::{constrain_imported_values, Import}; -use roc_fmt::annotation::{Formattable, Newlines, Parens}; -use roc_gen::llvm::build::{build_proc, build_proc_header, OptLevel}; -use roc_module::ident::Ident; -use roc_module::symbol::{IdentIds, Interns, ModuleId, ModuleIds, Symbol}; -use roc_parse::ast::{self, Attempting}; -use roc_parse::blankspace::space0_before; -use roc_parse::parser::{loc, Fail, FailReason, Parser, State}; -use roc_problem::can::Problem; -use roc_region::all::{Located, Region}; -use roc_solve::solve; -use roc_types::pretty_print::{content_to_string, name_all_type_vars}; -use roc_types::subs::{Content, Subs, VarStore, Variable}; -use roc_types::types::Type; -use std::hash::Hash; +use gen::{gen, ReplOutput}; +use roc_gen::llvm::build::OptLevel; +use roc_parse::parser::{Fail, FailReason}; use std::io::{self, Write}; -use std::path::{Path, PathBuf}; -use std::str::from_utf8_unchecked; use target_lexicon::Triple; pub const WELCOME_MESSAGE: &str = "\n The rockin’ \u{001b}[36mroc repl\u{001b}[0m\n\u{001b}[35m────────────────────────\u{001b}[0m\n\n"; @@ -35,6 +10,7 @@ pub const PROMPT: &str = "\n\u{001b}[36m»\u{001b}[0m "; pub const ELLIPSIS: &str = "\u{001b}[36m…\u{001b}[0m "; mod eval; +mod gen; pub fn main() -> io::Result<()> { use std::io::BufRead; @@ -75,7 +51,7 @@ pub fn main() -> io::Result<()> { } else if prev_line_blank { // After two blank lines in a row, give up and try parsing it // even though it's going to fail. This way you don't get stuck. - match print_output(pending_src.as_str()) { + match eval_and_format(pending_src.as_str()) { Ok(output) => { println!("{}", output); } @@ -97,12 +73,12 @@ pub fn main() -> io::Result<()> { } _ => { let result = if pending_src.is_empty() { - print_output(line) + eval_and_format(line) } else { pending_src.push('\n'); pending_src.push_str(line); - print_output(pending_src.as_str()) + eval_and_format(pending_src.as_str()) }; match result { @@ -141,7 +117,7 @@ fn report_parse_error(fail: Fail) { println!("TODO Gracefully report parse error in repl: {:?}", fail); } -fn print_output(src: &str) -> Result { +fn eval_and_format(src: &str) -> Result { gen(src.as_bytes(), Triple::host(), OptLevel::Normal).map(|output| match output { ReplOutput::NoProblems { expr, expr_type } => { format!("\n{} \u{001b}[35m:\u{001b}[0m {}", expr, expr_type) @@ -149,654 +125,3 @@ fn print_output(src: &str) -> Result { ReplOutput::Problems(lines) => format!("\n{}\n", lines.join("\n\n")), }) } - -pub fn repl_home() -> ModuleId { - ModuleIds::default().get_or_insert(&"REPL".into()) -} - -fn promote_expr_to_module(src: &str) -> String { - let mut buffer = String::from("app Repl provides [ replOutput ] imports []\n\nreplOutput =\n"); - - for line in src.lines() { - // indent the body! - buffer.push_str(" "); - buffer.push_str(line); - buffer.push('\n'); - } - - buffer -} - -fn gen(src: &[u8], target: Triple, opt_level: OptLevel) -> Result { - use roc_reporting::report::{ - can_problem, mono_problem, type_problem, RocDocAllocator, DEFAULT_PALETTE, - }; - - let arena = Bump::new(); - - // SAFETY: we've already verified that this is valid UTF-8 during parsing. - let src_str: &str = unsafe { from_utf8_unchecked(src) }; - - let stdlib = roc_builtins::std::standard_stdlib(); - let stdlib_mode = stdlib.mode; - let filename = PathBuf::from("REPL.roc"); - let src_dir = Path::new("fake/test/path"); - - let module_src = promote_expr_to_module(src_str); - - let exposed_types = MutMap::default(); - let loaded = roc_load::file::load_and_monomorphize_from_str( - &arena, - filename, - &module_src, - stdlib, - src_dir, - exposed_types, - ); - - let mut loaded = loaded.expect("failed to load module"); - - use roc_load::file::MonomorphizedModule; - let MonomorphizedModule { - mut procedures, - interns, - exposed_to_host, - mut subs, - module_id: home, - sources, - .. - } = loaded; - - let mut lines = Vec::new(); - - for (home, (module_path, src)) in sources { - let can_problems = loaded.can_problems.remove(&home).unwrap_or_default(); - let type_problems = loaded.type_problems.remove(&home).unwrap_or_default(); - let mono_problems = loaded.mono_problems.remove(&home).unwrap_or_default(); - - let error_count = can_problems.len() + type_problems.len() + mono_problems.len(); - - if error_count == 0 { - continue; - } - - let src_lines: Vec<&str> = src.split('\n').collect(); - let palette = DEFAULT_PALETTE; - - // Report parsing and canonicalization problems - let alloc = RocDocAllocator::new(&src_lines, home, &interns); - - for problem in can_problems.into_iter() { - let report = can_problem(&alloc, module_path.clone(), problem); - let mut buf = String::new(); - - report.render_color_terminal(&mut buf, &alloc, &palette); - - lines.push(buf); - } - - for problem in type_problems { - let report = type_problem(&alloc, module_path.clone(), problem); - let mut buf = String::new(); - - report.render_color_terminal(&mut buf, &alloc, &palette); - - lines.push(buf); - } - - for problem in mono_problems { - let report = mono_problem(&alloc, module_path.clone(), problem); - let mut buf = String::new(); - - report.render_color_terminal(&mut buf, &alloc, &palette); - - lines.push(buf); - } - } - - if !lines.is_empty() { - Ok(ReplOutput::Problems(lines)) - } else { - let context = Context::create(); - let module = arena.alloc(roc_gen::llvm::build::module_from_builtins(&context, "app")); - let builder = context.create_builder(); - - debug_assert_eq!(exposed_to_host.len(), 1); - let (main_fn_symbol, main_fn_var) = exposed_to_host.iter().next().unwrap(); - let main_fn_symbol = *main_fn_symbol; - let main_fn_var = *main_fn_var; - - // pretty-print the expr type string for later. - name_all_type_vars(main_fn_var, &mut subs); - let content = subs.get(main_fn_var).content; - let expr_type_str = content_to_string(content.clone(), &subs, home, &interns); - - let (_, main_fn_layout) = procedures - .keys() - .find(|(s, _)| *s == main_fn_symbol) - .unwrap() - .clone(); - - let ptr_bytes = target.pointer_width().unwrap().bytes() as u32; - - let module = arena.alloc(module); - let (module_pass, function_pass) = - roc_gen::llvm::build::construct_optimization_passes(module, opt_level); - - // Compile and add all the Procs before adding main - let env = roc_gen::llvm::build::Env { - arena: &arena, - builder: &builder, - context: &context, - interns, - module, - ptr_bytes, - leak: false, - // important! we don't want any procedures to get the C calling convention - exposed_to_host: MutSet::default(), - }; - - let mut layout_ids = roc_gen::layout_id::LayoutIds::default(); - let mut headers = Vec::with_capacity(procedures.len()); - - // Add all the Proc headers to the module. - // We have to do this in a separate pass first, - // because their bodies may reference each other. - let mut scope = roc_gen::llvm::build::Scope::default(); - for ((symbol, layout), proc) in procedures.drain() { - let fn_val = build_proc_header(&env, &mut layout_ids, symbol, &layout, &proc); - - if proc.args.is_empty() { - // this is a 0-argument thunk, i.e. a top-level constant definition - // it must be in-scope everywhere in the module! - scope.insert_top_level_thunk(symbol, layout, fn_val); - } - - headers.push((proc, fn_val)); - } - - // Build each proc using its header info. - for (proc, fn_val) in headers { - let mut current_scope = scope.clone(); - - // only have top-level thunks for this proc's module in scope - // this retain is not needed for correctness, but will cause less confusion when debugging - let home = proc.name.module_id(); - current_scope.retain_top_level_thunks_for_module(home); - - build_proc(&env, &mut layout_ids, scope.clone(), proc, fn_val); - - if fn_val.verify(true) { - function_pass.run_on(&fn_val); - } else { - use roc_builtins::std::Mode; - - let mode = match stdlib_mode { - Mode::Uniqueness => "OPTIMIZED", - Mode::Standard => "NON-OPTIMIZED", - }; - - eprintln!( - "\n\nFunction {:?} failed LLVM verification in {} build. Its content was:\n", - fn_val.get_name().to_str().unwrap(), - mode, - ); - - fn_val.print_to_stderr(); - - panic!( - "The preceding code was from {:?}, which failed LLVM verification in {} build.", - fn_val.get_name().to_str().unwrap(), - mode, - ); - } - } - let (main_fn_name, main_fn) = roc_gen::llvm::build::promote_to_main_function( - &env, - &mut layout_ids, - main_fn_symbol, - &main_fn_layout, - ); - - // Uncomment this to see the module's un-optimized LLVM instruction output: - // env.module.print_to_stderr(); - - if main_fn.verify(true) { - function_pass.run_on(&main_fn); - } else { - panic!("Main function {} failed LLVM verification in build. Uncomment things nearby to see more details.", main_fn_name); - } - - module_pass.run_on(env.module); - - // Verify the module - if let Err(errors) = env.module.verify() { - panic!("Errors defining module: {:?}", errors); - } - - // Uncomment this to see the module's optimized LLVM instruction output: - // env.module.print_to_stderr(); - - let lib = module_to_dylib(&env.module, &target, opt_level) - .expect("Error loading compiled dylib for test"); - let answer = unsafe { - eval::jit_to_ast( - &arena, - lib, - main_fn_name, - &main_fn_layout, - &content, - &env.interns, - home, - &subs, - ptr_bytes, - ) - }; - let mut expr = bumpalo::collections::String::new_in(&arena); - - answer.format_with_options(&mut expr, Parens::NotNeeded, Newlines::Yes, 0); - - Ok(ReplOutput::NoProblems { - expr: expr.into_bump_str().to_string(), - expr_type: expr_type_str, - }) - } -} - -enum ReplOutput { - Problems(Vec), - NoProblems { expr: String, expr_type: String }, -} - -pub fn infer_expr( - subs: Subs, - problems: &mut Vec, - constraint: &Constraint, - expr_var: Variable, -) -> (Content, Subs) { - let env = solve::Env { - aliases: MutMap::default(), - vars_by_symbol: SendMap::default(), - }; - let (solved, _) = solve::run(&env, problems, subs, constraint); - - let content = solved.inner().get_without_compacting(expr_var).content; - - (content, solved.into_inner()) -} - -pub fn parse_loc_with<'a>( - arena: &'a Bump, - bytes: &'a [u8], -) -> Result>, Fail> { - let state = State::new(&bytes, Attempting::Module); - let parser = space0_before(loc(roc_parse::expr::expr(0)), 0); - let answer = parser.parse(&arena, state); - - answer - .map(|(loc_expr, _)| loc_expr) - .map_err(|(fail, _)| fail) -} - -pub fn can_expr(expr_bytes: &[u8]) -> Result { - can_expr_with(&Bump::new(), repl_home(), expr_bytes) -} - -// TODO make this return a named struct instead of a big tuple -#[allow(clippy::type_complexity)] -pub fn uniq_expr( - expr_bytes: &[u8], -) -> Result< - ( - Located, - Output, - Vec, - Subs, - Variable, - Constraint, - ModuleId, - Interns, - ), - Fail, -> { - let declared_idents: &ImMap = &ImMap::default(); - - uniq_expr_with(&Bump::new(), expr_bytes, declared_idents) -} - -// TODO make this return a named struct instead of a big tuple -#[allow(clippy::type_complexity)] -pub fn uniq_expr_with( - arena: &Bump, - expr_bytes: &[u8], - declared_idents: &ImMap, -) -> Result< - ( - Located, - Output, - Vec, - Subs, - Variable, - Constraint, - ModuleId, - Interns, - ), - Fail, -> { - let home = repl_home(); - let CanExprOut { - loc_expr, - output, - problems, - var_store: mut old_var_store, - var, - interns, - .. - } = can_expr_with(arena, home, expr_bytes)?; - - // double check - let mut var_store = VarStore::new(old_var_store.fresh()); - - let expected2 = Expected::NoExpectation(Type::Variable(var)); - let constraint = roc_constrain::uniq::constrain_declaration( - home, - &mut var_store, - Region::zero(), - &loc_expr, - declared_idents, - expected2, - ); - - let stdlib = uniq_stdlib(); - - let types = stdlib.types; - let imports: Vec<_> = types - .into_iter() - .map(|(symbol, (solved_type, region))| Import { - loc_symbol: Located::at(region, symbol), - solved_type, - }) - .collect(); - - // load builtin values - - // TODO what to do with those rigids? - let (_introduced_rigids, constraint) = - constrain_imported_values(imports, constraint, &mut var_store); - - let subs2 = Subs::new(var_store.into()); - - Ok(( - loc_expr, output, problems, subs2, var, constraint, home, interns, - )) -} - -pub struct CanExprOut { - pub loc_expr: Located, - pub output: Output, - pub problems: Vec, - pub home: ModuleId, - pub interns: Interns, - pub var_store: VarStore, - pub var: Variable, - pub constraint: Constraint, -} - -pub fn can_expr_with(arena: &Bump, home: ModuleId, expr_bytes: &[u8]) -> Result { - let loc_expr = parse_loc_with(&arena, expr_bytes)?; - let mut var_store = VarStore::default(); - let var = var_store.fresh(); - let expected = Expected::NoExpectation(Type::Variable(var)); - let module_ids = ModuleIds::default(); - - // Desugar operators (convert them to Apply calls, taking into account - // operator precedence and associativity rules), before doing other canonicalization. - // - // If we did this *during* canonicalization, then each time we - // visited a BinOp node we'd recursively try to apply this to each of its nested - // operators, and then again on *their* nested operators, ultimately applying the - // rules multiple times unnecessarily. - let loc_expr = operator::desugar_expr(arena, &loc_expr); - - let mut scope = Scope::new(home, &mut var_store); - let dep_idents = IdentIds::exposed_builtins(0); - let mut env = roc_can::env::Env::new(home, dep_idents, &module_ids, IdentIds::default()); - let (loc_expr, output) = canonicalize_expr( - &mut env, - &mut var_store, - &mut scope, - Region::zero(), - &loc_expr.value, - ); - - // Add builtin defs (e.g. List.get) directly to the canonical Expr, - // since we aren't using modules here. - let mut with_builtins = loc_expr.value; - let builtin_defs = roc_can::builtins::builtin_defs(&mut var_store); - - for (symbol, def) in builtin_defs { - if output.references.lookups.contains(&symbol) || output.references.calls.contains(&symbol) - { - with_builtins = Expr::LetNonRec( - Box::new(def), - Box::new(Located { - region: Region::zero(), - value: with_builtins, - }), - var_store.fresh(), - ); - } - } - - let loc_expr = Located { - region: loc_expr.region, - value: with_builtins, - }; - - let constraint = constrain_expr( - &roc_constrain::expr::Env { - rigids: ImMap::default(), - home, - }, - loc_expr.region, - &loc_expr.value, - expected, - ); - - let types = roc_builtins::std::types(); - - let imports: Vec<_> = types - .into_iter() - .map(|(symbol, (solved_type, region))| Import { - loc_symbol: Located::at(region, symbol), - solved_type, - }) - .collect(); - - //load builtin values - let (_introduced_rigids, constraint) = - constrain_imported_values(imports, constraint, &mut var_store); - - // TODO determine what to do with those rigids - // for var in introduced_rigids { - // output.ftv.insert(var, format!("internal_{:?}", var).into()); - // } - - let mut all_ident_ids = MutMap::default(); - - // When pretty printing types, we may need the exposed builtins, - // so include them in the Interns we'll ultimately return. - for (module_id, ident_ids) in IdentIds::exposed_builtins(0) { - all_ident_ids.insert(module_id, ident_ids); - } - - all_ident_ids.insert(home, env.ident_ids); - - let interns = Interns { - module_ids: env.module_ids.clone(), - all_ident_ids, - }; - - Ok(CanExprOut { - loc_expr, - output, - problems: env.problems, - home: env.home, - var_store, - interns, - var, - constraint, - }) -} - -pub fn mut_map_from_pairs(pairs: I) -> MutMap -where - I: IntoIterator, - K: Hash + Eq, -{ - let mut answer = MutMap::default(); - - for (key, value) in pairs { - answer.insert(key, value); - } - - answer -} - -pub fn im_map_from_pairs(pairs: I) -> ImMap -where - I: IntoIterator, - K: Hash + Eq + Clone, - V: Clone, -{ - let mut answer = ImMap::default(); - - for (key, value) in pairs { - answer.insert(key, value); - } - - answer -} - -pub fn send_set_from(elems: I) -> SendSet -where - I: IntoIterator, - V: Hash + Eq + Clone, -{ - let mut answer = SendSet::default(); - - for elem in elems { - answer.insert(elem); - } - - answer -} - -// Check constraints -// -// Keep track of the used (in types or expectations) variables, and the declared variables (in -// flex_vars or rigid_vars fields of LetConstraint. These roc_collections should match: no duplicates -// and no variables that are used but not declared are allowed. -// -// There is one exception: the initial variable (that stores the type of the whole expression) is -// never declared, but is used. -pub fn assert_correct_variable_usage(constraint: &Constraint) { - // variables declared in constraint (flex_vars or rigid_vars) - // and variables actually used in constraints - let (declared, used) = variable_usage(constraint); - - let used: ImSet = used.into(); - let mut decl: ImSet = declared.rigid_vars.clone().into(); - - for var in declared.flex_vars.clone() { - decl.insert(var); - } - - let diff = used.clone().relative_complement(decl); - - // NOTE: this checks whether we're using variables that are not declared. For recursive type - // definitions, their rigid types are declared twice, which is correct! - if !diff.is_empty() { - println!("VARIABLE USAGE PROBLEM"); - - println!("used: {:?}", &used); - println!("rigids: {:?}", &declared.rigid_vars); - println!("flexs: {:?}", &declared.flex_vars); - - println!("difference: {:?}", &diff); - - panic!("variable usage problem (see stdout for details)"); - } -} - -#[derive(Default)] -pub struct SeenVariables { - pub rigid_vars: Vec, - pub flex_vars: Vec, -} - -pub fn variable_usage(con: &Constraint) -> (SeenVariables, Vec) { - let mut declared = SeenVariables::default(); - let mut used = ImSet::default(); - variable_usage_help(con, &mut declared, &mut used); - - used.remove(unsafe { &Variable::unsafe_test_debug_variable(1) }); - - let mut used_vec: Vec = used.into_iter().collect(); - used_vec.sort(); - - declared.rigid_vars.sort(); - declared.flex_vars.sort(); - - (declared, used_vec) -} - -fn variable_usage_help(con: &Constraint, declared: &mut SeenVariables, used: &mut ImSet) { - use Constraint::*; - - match con { - True | SaveTheEnvironment => (), - Eq(tipe, expectation, _, _) => { - for v in tipe.variables() { - used.insert(v); - } - - for v in expectation.get_type_ref().variables() { - used.insert(v); - } - } - Store(tipe, var, _, _) => { - for v in tipe.variables() { - used.insert(v); - } - - used.insert(*var); - } - Lookup(_, expectation, _) => { - for v in expectation.get_type_ref().variables() { - used.insert(v); - } - } - Pattern(_, _, tipe, pexpectation) => { - for v in tipe.variables() { - used.insert(v); - } - - for v in pexpectation.get_type_ref().variables() { - used.insert(v); - } - } - Let(letcon) => { - declared.rigid_vars.extend(letcon.rigid_vars.clone()); - declared.flex_vars.extend(letcon.flex_vars.clone()); - - variable_usage_help(&letcon.defs_constraint, declared, used); - variable_usage_help(&letcon.ret_constraint, declared, used); - } - And(constraints) => { - for sub in constraints { - variable_usage_help(sub, declared, used); - } - } - } -} diff --git a/cli/src/repl/debug.rs b/cli/src/repl/debug.rs new file mode 100644 index 0000000000..8a26015036 --- /dev/null +++ b/cli/src/repl/debug.rs @@ -0,0 +1,109 @@ + +// Check constraints +// +// Keep track of the used (in types or expectations) variables, and the declared variables (in +// flex_vars or rigid_vars fields of LetConstraint. These roc_collections should match: no duplicates +// and no variables that are used but not declared are allowed. +// +// There is one exception: the initial variable (that stores the type of the whole expression) is +// never declared, but is used. +pub fn assert_correct_variable_usage(constraint: &Constraint) { + // variables declared in constraint (flex_vars or rigid_vars) + // and variables actually used in constraints + let (declared, used) = variable_usage(constraint); + + let used: ImSet = used.into(); + let mut decl: ImSet = declared.rigid_vars.clone().into(); + + for var in declared.flex_vars.clone() { + decl.insert(var); + } + + let diff = used.clone().relative_complement(decl); + + // NOTE: this checks whether we're using variables that are not declared. For recursive type + // definitions, their rigid types are declared twice, which is correct! + if !diff.is_empty() { + println!("VARIABLE USAGE PROBLEM"); + + println!("used: {:?}", &used); + println!("rigids: {:?}", &declared.rigid_vars); + println!("flexs: {:?}", &declared.flex_vars); + + println!("difference: {:?}", &diff); + + panic!("variable usage problem (see stdout for details)"); + } +} + +#[derive(Default)] +pub struct SeenVariables { + pub rigid_vars: Vec, + pub flex_vars: Vec, +} + +pub fn variable_usage(con: &Constraint) -> (SeenVariables, Vec) { + let mut declared = SeenVariables::default(); + let mut used = ImSet::default(); + variable_usage_help(con, &mut declared, &mut used); + + used.remove(unsafe { &Variable::unsafe_test_debug_variable(1) }); + + let mut used_vec: Vec = used.into_iter().collect(); + used_vec.sort(); + + declared.rigid_vars.sort(); + declared.flex_vars.sort(); + + (declared, used_vec) +} + +fn variable_usage_help(con: &Constraint, declared: &mut SeenVariables, used: &mut ImSet) { + use Constraint::*; + + match con { + True | SaveTheEnvironment => (), + Eq(tipe, expectation, _, _) => { + for v in tipe.variables() { + used.insert(v); + } + + for v in expectation.get_type_ref().variables() { + used.insert(v); + } + } + Store(tipe, var, _, _) => { + for v in tipe.variables() { + used.insert(v); + } + + used.insert(*var); + } + Lookup(_, expectation, _) => { + for v in expectation.get_type_ref().variables() { + used.insert(v); + } + } + Pattern(_, _, tipe, pexpectation) => { + for v in tipe.variables() { + used.insert(v); + } + + for v in pexpectation.get_type_ref().variables() { + used.insert(v); + } + } + Let(letcon) => { + declared.rigid_vars.extend(letcon.rigid_vars.clone()); + declared.flex_vars.extend(letcon.flex_vars.clone()); + + variable_usage_help(&letcon.defs_constraint, declared, used); + variable_usage_help(&letcon.ret_constraint, declared, used); + } + And(constraints) => { + for sub in constraints { + variable_usage_help(sub, declared, used); + } + } + } +} \ No newline at end of file diff --git a/cli/src/repl/gen.rs b/cli/src/repl/gen.rs new file mode 100644 index 0000000000..9633216861 --- /dev/null +++ b/cli/src/repl/gen.rs @@ -0,0 +1,267 @@ +use crate::repl::eval; +use bumpalo::Bump; +use inkwell::context::Context; +use roc_build::link::module_to_dylib; +use roc_collections::all::{MutMap, MutSet}; +use roc_fmt::annotation::Formattable; +use roc_fmt::annotation::{Newlines, Parens}; +use roc_gen::llvm::build::{build_proc, build_proc_header, OptLevel}; +use roc_parse::parser::Fail; +use roc_types::pretty_print::{content_to_string, name_all_type_vars}; +use std::path::{Path, PathBuf}; +use std::str::from_utf8_unchecked; +use target_lexicon::Triple; + +pub enum ReplOutput { + Problems(Vec), + NoProblems { expr: String, expr_type: String }, +} + +pub fn gen(src: &[u8], target: Triple, opt_level: OptLevel) -> Result { + use roc_reporting::report::{ + can_problem, mono_problem, type_problem, RocDocAllocator, DEFAULT_PALETTE, + }; + + let arena = Bump::new(); + + // SAFETY: we've already verified that this is valid UTF-8 during parsing. + let src_str: &str = unsafe { from_utf8_unchecked(src) }; + + let stdlib = roc_builtins::std::standard_stdlib(); + let stdlib_mode = stdlib.mode; + let filename = PathBuf::from("REPL.roc"); + let src_dir = Path::new("fake/test/path"); + + let module_src = promote_expr_to_module(src_str); + + let exposed_types = MutMap::default(); + let loaded = roc_load::file::load_and_monomorphize_from_str( + &arena, + filename, + &module_src, + stdlib, + src_dir, + exposed_types, + ); + + let mut loaded = loaded.expect("failed to load module"); + + use roc_load::file::MonomorphizedModule; + let MonomorphizedModule { + mut procedures, + interns, + exposed_to_host, + mut subs, + module_id: home, + sources, + .. + } = loaded; + + let mut lines = Vec::new(); + + for (home, (module_path, src)) in sources { + let can_problems = loaded.can_problems.remove(&home).unwrap_or_default(); + let type_problems = loaded.type_problems.remove(&home).unwrap_or_default(); + let mono_problems = loaded.mono_problems.remove(&home).unwrap_or_default(); + + let error_count = can_problems.len() + type_problems.len() + mono_problems.len(); + + if error_count == 0 { + continue; + } + + let src_lines: Vec<&str> = src.split('\n').collect(); + let palette = DEFAULT_PALETTE; + + // Report parsing and canonicalization problems + let alloc = RocDocAllocator::new(&src_lines, home, &interns); + + for problem in can_problems.into_iter() { + let report = can_problem(&alloc, module_path.clone(), problem); + let mut buf = String::new(); + + report.render_color_terminal(&mut buf, &alloc, &palette); + + lines.push(buf); + } + + for problem in type_problems { + let report = type_problem(&alloc, module_path.clone(), problem); + let mut buf = String::new(); + + report.render_color_terminal(&mut buf, &alloc, &palette); + + lines.push(buf); + } + + for problem in mono_problems { + let report = mono_problem(&alloc, module_path.clone(), problem); + let mut buf = String::new(); + + report.render_color_terminal(&mut buf, &alloc, &palette); + + lines.push(buf); + } + } + + if !lines.is_empty() { + Ok(ReplOutput::Problems(lines)) + } else { + let context = Context::create(); + let module = arena.alloc(roc_gen::llvm::build::module_from_builtins(&context, "app")); + let builder = context.create_builder(); + + debug_assert_eq!(exposed_to_host.len(), 1); + let (main_fn_symbol, main_fn_var) = exposed_to_host.iter().next().unwrap(); + let main_fn_symbol = *main_fn_symbol; + let main_fn_var = *main_fn_var; + + // pretty-print the expr type string for later. + name_all_type_vars(main_fn_var, &mut subs); + let content = subs.get(main_fn_var).content; + let expr_type_str = content_to_string(content.clone(), &subs, home, &interns); + + let (_, main_fn_layout) = procedures + .keys() + .find(|(s, _)| *s == main_fn_symbol) + .unwrap() + .clone(); + + let ptr_bytes = target.pointer_width().unwrap().bytes() as u32; + + let module = arena.alloc(module); + let (module_pass, function_pass) = + roc_gen::llvm::build::construct_optimization_passes(module, opt_level); + + // Compile and add all the Procs before adding main + let env = roc_gen::llvm::build::Env { + arena: &arena, + builder: &builder, + context: &context, + interns, + module, + ptr_bytes, + leak: false, + // important! we don't want any procedures to get the C calling convention + exposed_to_host: MutSet::default(), + }; + + let mut layout_ids = roc_gen::layout_id::LayoutIds::default(); + let mut headers = Vec::with_capacity(procedures.len()); + + // Add all the Proc headers to the module. + // We have to do this in a separate pass first, + // because their bodies may reference each other. + let mut scope = roc_gen::llvm::build::Scope::default(); + for ((symbol, layout), proc) in procedures.drain() { + let fn_val = build_proc_header(&env, &mut layout_ids, symbol, &layout, &proc); + + if proc.args.is_empty() { + // this is a 0-argument thunk, i.e. a top-level constant definition + // it must be in-scope everywhere in the module! + scope.insert_top_level_thunk(symbol, layout, fn_val); + } + + headers.push((proc, fn_val)); + } + + // Build each proc using its header info. + for (proc, fn_val) in headers { + let mut current_scope = scope.clone(); + + // only have top-level thunks for this proc's module in scope + // this retain is not needed for correctness, but will cause less confusion when debugging + let home = proc.name.module_id(); + current_scope.retain_top_level_thunks_for_module(home); + + build_proc(&env, &mut layout_ids, scope.clone(), proc, fn_val); + + if fn_val.verify(true) { + function_pass.run_on(&fn_val); + } else { + use roc_builtins::std::Mode; + + let mode = match stdlib_mode { + Mode::Uniqueness => "OPTIMIZED", + Mode::Standard => "NON-OPTIMIZED", + }; + + eprintln!( + "\n\nFunction {:?} failed LLVM verification in {} build. Its content was:\n", + fn_val.get_name().to_str().unwrap(), + mode, + ); + + fn_val.print_to_stderr(); + + panic!( + "The preceding code was from {:?}, which failed LLVM verification in {} build.", + fn_val.get_name().to_str().unwrap(), + mode, + ); + } + } + let (main_fn_name, main_fn) = roc_gen::llvm::build::promote_to_main_function( + &env, + &mut layout_ids, + main_fn_symbol, + &main_fn_layout, + ); + + // Uncomment this to see the module's un-optimized LLVM instruction output: + // env.module.print_to_stderr(); + + if main_fn.verify(true) { + function_pass.run_on(&main_fn); + } else { + panic!("Main function {} failed LLVM verification in build. Uncomment things nearby to see more details.", main_fn_name); + } + + module_pass.run_on(env.module); + + // Verify the module + if let Err(errors) = env.module.verify() { + panic!("Errors defining module: {:?}", errors); + } + + // Uncomment this to see the module's optimized LLVM instruction output: + // env.module.print_to_stderr(); + + let lib = module_to_dylib(&env.module, &target, opt_level) + .expect("Error loading compiled dylib for test"); + let answer = unsafe { + eval::jit_to_ast( + &arena, + lib, + main_fn_name, + &main_fn_layout, + &content, + &env.interns, + home, + &subs, + ptr_bytes, + ) + }; + let mut expr = bumpalo::collections::String::new_in(&arena); + + answer.format_with_options(&mut expr, Parens::NotNeeded, Newlines::Yes, 0); + + Ok(ReplOutput::NoProblems { + expr: expr.into_bump_str().to_string(), + expr_type: expr_type_str, + }) + } +} + +fn promote_expr_to_module(src: &str) -> String { + let mut buffer = String::from("app Repl provides [ replOutput ] imports []\n\nreplOutput =\n"); + + for line in src.lines() { + // indent the body! + buffer.push_str(" "); + buffer.push_str(line); + buffer.push('\n'); + } + + buffer +} diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 2607d9234a..d872e93332 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -2801,8 +2801,6 @@ fn call_bitcode_fn_help<'a, 'ctx, 'env>( .get_function(fn_name) .unwrap_or_else(|| panic!("Unrecognized builtin function: {:?} - if you're working on the Roc compiler, do you need to rebuild the bitcode? See compiler/builtins/bitcode/README.md", fn_name)); - dbg!(fn_val); - let call = env.builder.build_call(fn_val, args, "call_builtin"); call.set_call_convention(fn_val.get_call_conventions()); From 93b6315f46f3e52c51736c50592b5e74aef0f107 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 16 Nov 2020 02:18:18 +0100 Subject: [PATCH 02/37] fix pattern match test ordering problem --- compiler/gen/tests/gen_primitives.rs | 99 ++++++++++++++++------------ compiler/mono/src/decision_tree.rs | 2 +- 2 files changed, 59 insertions(+), 42 deletions(-) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index fc9ddc7ac2..017eb5fa6e 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1341,47 +1341,9 @@ mod gen_primitives { ); } - #[test] - fn rbtree_balance_2() { - assert_non_opt_evals_to!( - indoc!( - r#" - app Test provides [ main ] imports [] - - NodeColor : [ Red, Black ] - - Dict k : [ Node NodeColor k (Dict k), Empty ] - - balance : NodeColor, k, Dict k, Dict k -> Dict k - balance = \color, key, left, right -> - when right is - Node Red rK _ -> - when left is - Node Red _ _ -> - Node - Red - key - Empty - - _ -> - Node color rK (Node Red key left ) - - _ -> - Empty - - main : Dict Int - main = - balance Red 0 Empty Empty - "# - ), - 0, - i64 - ); - } - #[test] #[ignore] - fn rbtree_balance() { + fn rbtree_balance_full() { assert_non_opt_evals_to!( indoc!( r#" @@ -1431,7 +1393,59 @@ mod gen_primitives { } #[test] - #[ignore] + fn nested_pattern_match_two_ways() { + // exposed an issue in the ordering of pattern match checks when ran with `--release` mode + assert_non_opt_evals_to!( + indoc!( + r#" + app Test provides [ main ] imports [] + + ConsList a : [ Cons a (ConsList a), Nil ] + + balance : ConsList Int -> Int + balance = \right -> + when right is + Cons 1 foo -> + when foo is + Cons 1 _ -> 3 + _ -> 3 + _ -> 3 + + main : Int + main = + when balance Nil is + _ -> 3 + "# + ), + 3, + i64 + ); + + assert_non_opt_evals_to!( + indoc!( + r#" + app Test provides [ main ] imports [] + + ConsList a : [ Cons a (ConsList a), Nil ] + + balance : ConsList Int -> Int + balance = \right -> + when right is + Cons 1 (Cons 1 _) -> 3 + _ -> 3 + + main : Int + main = + when balance Nil is + _ -> 3 + "# + ), + 3, + i64 + ); + } + + #[test] fn linked_list_guarded_double_pattern_match() { // the important part here is that the first case (with the nested Cons) does not match // TODO this also has undefined behavior @@ -1445,7 +1459,10 @@ mod gen_primitives { balance : ConsList Int -> Int balance = \right -> when right is - Cons 1 (Cons 1 _) -> 3 + Cons 1 foo -> + when foo is + Cons 1 _ -> 3 + _ -> 3 _ -> 3 main : Int diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 65ed5c3f0c..f079b4d5eb 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -1316,7 +1316,7 @@ fn compile_tests<'a>( cond = compile_guard(env, ret_layout.clone(), id, arena.alloc(stmt), fail, cond); } - for (new_stores, lhs, rhs, _layout) in tests.into_iter().rev() { + for (new_stores, lhs, rhs, _layout) in tests.into_iter() { cond = compile_test(env, ret_layout.clone(), new_stores, lhs, rhs, fail, cond); } cond From 9ac1491647966b117520d8979e33c2f7a0bd5763 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 16 Nov 2020 02:19:07 +0100 Subject: [PATCH 03/37] add test that fails because of a rigid problem --- compiler/solve/tests/solve_expr.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 27d58428b5..c26a5978b8 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -3614,4 +3614,33 @@ mod solve_expr { "Dict Int Int", ); } + + #[test] + #[ignore] + fn pattern_rigid_problem() { + infer_eq_without_problem( + indoc!( + r#" + app Test provides [ main ] imports [] + + Dict k : [ Node k (Dict k) (Dict k), Empty ] + + balance : k, Dict k -> Dict k + balance = \key, left -> + when left is + Node _ _ lRight -> + Node key lRight Empty + + _ -> + Empty + + + main : Dict Int + main = + balance 0 Empty + "# + ), + "Dict Int", + ); + } } From 8528e5affbcb9cf1f4b0a0c22ea435934d73edd1 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 01:58:30 +0100 Subject: [PATCH 04/37] upgrade inkwell --- Cargo.lock | 14 +++++++------- cli/Cargo.toml | 2 +- compiler/build/Cargo.toml | 2 +- compiler/gen/Cargo.toml | 2 +- compiler/gen/src/llvm/build_list.rs | 16 ++++++++++++---- compiler/gen/src/llvm/build_str.rs | 4 +++- editor/Cargo.toml | 2 +- 7 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b1b557d7c5..f3b9a54040 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1195,25 +1195,25 @@ dependencies = [ [[package]] name = "inkwell" version = "0.1.0" -source = "git+https://github.com/rtfeldman/inkwell?tag=llvm10-0.release2#d0a1ce5e678cf0f0e62b757760d1019301c603c9" +source = "git+https://github.com/rtfeldman/inkwell?tag=llvm10-0.release3#57e9f00d98fc99486e737c314e56a59498c5dbbb" dependencies = [ "either", "inkwell_internals", "libc", "llvm-sys", "once_cell", - "parking_lot 0.10.2", + "parking_lot 0.11.0", "regex", ] [[package]] name = "inkwell_internals" -version = "0.1.0" -source = "git+https://github.com/rtfeldman/inkwell?tag=llvm10-0.release2#d0a1ce5e678cf0f0e62b757760d1019301c603c9" +version = "0.2.0" +source = "git+https://github.com/rtfeldman/inkwell?tag=llvm10-0.release3#57e9f00d98fc99486e737c314e56a59498c5dbbb" dependencies = [ - "proc-macro2 0.4.30", - "quote 0.6.13", - "syn 0.15.44", + "proc-macro2 1.0.21", + "quote 1.0.7", + "syn 1.0.40", ] [[package]] diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4ddc752022..e37bc8781c 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -77,7 +77,7 @@ libloading = "0.6" # commit of TheDan64/inkwell, push a new tag which points to the latest commit, # change the tag value in this Cargo.toml to point to that tag, and `cargo update`. # This way, GitHub Actions works and nobody's builds get broken. -inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release2" } +inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release3" } target-lexicon = "0.10" [dev-dependencies] diff --git a/compiler/build/Cargo.toml b/compiler/build/Cargo.toml index d178dee1d7..a2a0eea088 100644 --- a/compiler/build/Cargo.toml +++ b/compiler/build/Cargo.toml @@ -45,7 +45,7 @@ tempfile = "3.1.0" # commit of TheDan64/inkwell, push a new tag which points to the latest commit, # change the tag value in this Cargo.toml to point to that tag, and `cargo update`. # This way, GitHub Actions works and nobody's builds get broken. -inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release2" } +inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release3" } target-lexicon = "0.10" [dev-dependencies] diff --git a/compiler/gen/Cargo.toml b/compiler/gen/Cargo.toml index f87447f23f..41420c9056 100644 --- a/compiler/gen/Cargo.toml +++ b/compiler/gen/Cargo.toml @@ -39,7 +39,7 @@ either = "1.6.1" # commit of TheDan64/inkwell, push a new tag which points to the latest commit, # change the tag value in this Cargo.toml to point to that tag, and `cargo update`. # This way, GitHub Actions works and nobody's builds get broken. -inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release2" } +inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release3" } target-lexicon = "0.10" libloading = "0.6" diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index 7ef5bd1593..77ee9909bc 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -185,7 +185,9 @@ pub fn list_prepend<'a, 'ctx, 'env>( // one we just malloc'd. // // TODO how do we decide when to do the small memcpy vs the normal one? - builder.build_memcpy(index_1_ptr, ptr_bytes, list_ptr, ptr_bytes, list_size); + builder + .build_memcpy(index_1_ptr, ptr_bytes, list_ptr, ptr_bytes, list_size) + .unwrap(); } else { panic!("TODO Cranelift currently only knows how to clone list elements that are Copy."); } @@ -626,7 +628,9 @@ pub fn list_append<'a, 'ctx, 'env>( // one we just malloc'd. // // TODO how do we decide when to do the small memcpy vs the normal one? - builder.build_memcpy(clone_ptr, ptr_bytes, list_ptr, ptr_bytes, list_size); + builder + .build_memcpy(clone_ptr, ptr_bytes, list_ptr, ptr_bytes, list_size) + .unwrap(); } else { panic!("TODO Cranelift currently only knows how to clone list elements that are Copy."); } @@ -1816,7 +1820,9 @@ pub fn clone_nonempty_list<'a, 'ctx, 'env>( // one we just malloc'd. // // TODO how do we decide when to do the small memcpy vs the normal one? - builder.build_memcpy(clone_ptr, ptr_bytes, elems_ptr, ptr_bytes, size); + builder + .build_memcpy(clone_ptr, ptr_bytes, elems_ptr, ptr_bytes, size) + .unwrap(); } else { panic!("TODO Cranelift currently only knows how to clone list elements that are Copy."); } @@ -1872,7 +1878,9 @@ pub fn clone_list<'a, 'ctx, 'env>( ); // copy old elements in - builder.build_memcpy(new_ptr, ptr_bytes, old_ptr, ptr_bytes, bytes); + builder + .build_memcpy(new_ptr, ptr_bytes, old_ptr, ptr_bytes, bytes) + .unwrap(); new_ptr } diff --git a/compiler/gen/src/llvm/build_str.rs b/compiler/gen/src/llvm/build_str.rs index 77c13c282f..24b18a5943 100644 --- a/compiler/gen/src/llvm/build_str.rs +++ b/compiler/gen/src/llvm/build_str.rs @@ -537,7 +537,9 @@ fn clone_nonempty_str<'a, 'ctx, 'env>( // Copy the bytes from the original array into the new // one we just malloc'd. - builder.build_memcpy(clone_ptr, ptr_bytes, bytes_ptr, ptr_bytes, len); + builder + .build_memcpy(clone_ptr, ptr_bytes, bytes_ptr, ptr_bytes, len) + .unwrap(); // Create a fresh wrapper struct for the newly populated array let struct_type = collection(ctx, env.ptr_bytes); diff --git a/editor/Cargo.toml b/editor/Cargo.toml index eaa6f140b8..b39514dd02 100644 --- a/editor/Cargo.toml +++ b/editor/Cargo.toml @@ -47,7 +47,7 @@ tokio = { version = "0.2", features = ["blocking", "fs", "sync", "rt-threaded", # commit of TheDan64/inkwell, push a new tag which points to the latest commit, # change the tag value in this Cargo.toml to point to that tag, and `cargo update`. # This way, GitHub Actions works and nobody's builds get broken. -inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release2" } +inkwell = { git = "https://github.com/rtfeldman/inkwell", tag = "llvm10-0.release3" } target-lexicon = "0.10" winit = "0.22" wgpu = "0.6" From 8f49b1afaa0cd899e451d73dd4ac79898118439f Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:13:40 +0100 Subject: [PATCH 05/37] re-enable RBTree.balance test --- compiler/gen/src/llvm/refcounting.rs | 4 +++- compiler/gen/tests/gen_primitives.rs | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 9ae829bc0c..57bf80a7c6 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -1203,7 +1203,9 @@ pub fn refcount_offset<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layou match layout { Layout::Builtin(Builtin::List(_, _)) => env.ptr_bytes as u64, Layout::Builtin(Builtin::Str) => env.ptr_bytes as u64, - Layout::RecursivePointer | Layout::RecursiveUnion(_) => env.ptr_bytes as u64, + Layout::RecursivePointer | Layout::Union(_) | Layout::RecursiveUnion(_) => { + env.ptr_bytes as u64 + } _ => (env.ptr_bytes as u64).max(value_bytes), } } diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index 017eb5fa6e..ad72ab66b6 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1342,7 +1342,6 @@ mod gen_primitives { } #[test] - #[ignore] fn rbtree_balance_full() { assert_non_opt_evals_to!( indoc!( From 4dc58b8107fc570e448effeeec736445515778eb Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:21:55 +0100 Subject: [PATCH 06/37] free pointers --- roc_std/src/alloca.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/roc_std/src/alloca.rs b/roc_std/src/alloca.rs index 5b9b280bd7..1fa9c26525 100644 --- a/roc_std/src/alloca.rs +++ b/roc_std/src/alloca.rs @@ -75,9 +75,12 @@ unsafe fn free_or_noop(ptr: *mut c_void) { #[cfg(not(debug_assertions))] #[inline(always)] -fn free_or_noop(_ptr: *mut c_void) { +unsafe fn free_or_noop(ptr: *mut c_void) { // In release builds, we'll have used alloca, // so there's nothing to free. + + // except that for now we always use malloc + libc::free(ptr) } #[cfg(test)] From bc74af1bd42852f6ba94f79db254a2cff38e391a Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:26:53 +0100 Subject: [PATCH 07/37] add assert that should pass, but doesn't always --- compiler/mono/src/ir.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index edbae1195b..0feb855f47 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5332,6 +5332,10 @@ pub fn from_can_pattern<'a>( let mut mono_args = Vec::with_capacity_in(arguments.len(), env.arena); // disregard the tag discriminant layout + + // TODO make this assert pass, it currently does not because + // 0-sized values are dropped out + // debug_assert_eq!(arguments.len(), argument_layouts[1..].len()); let it = argument_layouts[1..].iter(); for ((_, loc_pat), layout) in arguments.iter().zip(it) { mono_args.push(( From b6d46bb6f93833d494f7f5e93ef283c8b2cb9719 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:30:59 +0100 Subject: [PATCH 08/37] add another failing test case --- compiler/gen/tests/gen_primitives.rs | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index ad72ab66b6..90c695f2af 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1341,6 +1341,53 @@ mod gen_primitives { ); } + #[test] + #[ignore] + fn rbtree_balance_mono_problem() { + // because of how the function is written, only `Red` is used and so in the function's + // type, the first argument is a unit and dropped. Apparently something is weird with + // constraint generation where the specialization required by `main` does not fix the + // problem. As a result, the first argument is dropped and we run into issues down the line + // + // concretely, the `rRight` symbol will not be defined + assert_non_opt_evals_to!( + indoc!( + r#" + app Test provides [ main ] imports [] + + NodeColor : [ Red, Black ] + + Dict k v : [ Node NodeColor k v (Dict k v) (Dict k v), Empty ] + + # balance : NodeColor, k, v, Dict k v, Dict k v -> Dict k v + balance = \color, key, value, left, right -> + when right is + Node Red rK rV rLeft rRight -> + when left is + Node Red lK lV lLeft lRight -> + Node + Red + key + value + (Node Black lK lV lLeft lRight) + (Node Black rK rV rLeft rRight) + + _ -> + Node color rK rV (Node Red key value left rLeft) rRight + + _ -> + Empty + + main : Dict Int Int + main = + balance Red 0 0 Empty Empty + "# + ), + 1, + i64 + ); + } + #[test] fn rbtree_balance_full() { assert_non_opt_evals_to!( From 2082d6e57fc86f107d31a8c62e44decaef727101 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:33:17 +0100 Subject: [PATCH 09/37] add `alignment_bytes` --- compiler/mono/src/layout.rs | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 50b572c76c..2161cf4ba8 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -431,9 +431,33 @@ impl<'a> Layout<'a> { } } + pub fn alignment_bytes(&self) -> u32 { + match self { + Layout::Struct(fields) => fields + .iter() + .map(|x| x.alignment_bytes()) + .max() + .unwrap_or(0), + Layout::Union(tags) | Layout::RecursiveUnion(tags) => tags + .iter() + .map(|x| x.iter()) + .flatten() + .map(|x| x.alignment_bytes()) + .max() + .unwrap_or(0), + Layout::Builtin(builtin) => builtin.alignment_bytes(), + Layout::PhantomEmptyStruct => 0, + Layout::RecursivePointer => std::mem::align_of::<*const u8>() as u32, + Layout::FunctionPointer(_, _) => std::mem::align_of::<*const u8>() as u32, + Layout::Pointer(_) => std::mem::align_of::<*const u8>() as u32, + Layout::Closure(_, _, _) => todo!(), + } + } + pub fn is_refcounted(&self) -> bool { match self { Layout::Builtin(Builtin::List(_, _)) => true, + Layout::Builtin(Builtin::Str) => true, Layout::RecursiveUnion(_) => true, Layout::RecursivePointer => true, _ => false, @@ -631,6 +655,29 @@ impl<'a> Builtin<'a> { } } + pub fn alignment_bytes(&self) -> u32 { + use std::mem::align_of; + use Builtin::*; + + match self { + Int128 => align_of::() as u32, + Int64 => align_of::() as u32, + Int32 => align_of::() as u32, + Int16 => align_of::() as u32, + Int8 => align_of::() as u32, + Int1 => align_of::() as u32, + Float128 => align_of::() as u32, + Float64 => align_of::() as u32, + Float32 => align_of::() as u32, + Float16 => align_of::() as u32, + Str | EmptyStr => align_of::() as u32, + Map(_, _) | EmptyMap => todo!(), + Set(_) | EmptySet => todo!(), + EmptyList => 0, + List(_, element_layout) => element_layout.alignment_bytes(), + } + } + pub fn safe_to_memcpy(&self) -> bool { use Builtin::*; From 51f54efcee78eece9cce799476b4808f2b7facfe Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 02:40:21 +0100 Subject: [PATCH 10/37] fix mono tests --- compiler/mono/tests/test_mono.rs | 72 ++++++++++++++++---------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index e010927233..ab1dab9af4 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -524,14 +524,14 @@ mod test_mono { let Test.8 = 1i64; ret Test.8; in - let Test.9 = Index 1 Test.2; - let Test.10 = 0i64; - let Test.11 = Index 0 Test.9; - let Test.16 = lowlevel Eq Test.10 Test.11; + let Test.12 = 0i64; + let Test.13 = Index 0 Test.2; + let Test.16 = lowlevel Eq Test.12 Test.13; if Test.16 then - let Test.12 = 0i64; - let Test.13 = Index 0 Test.2; - let Test.15 = lowlevel Eq Test.12 Test.13; + let Test.9 = Index 1 Test.2; + let Test.10 = 0i64; + let Test.11 = Index 0 Test.9; + let Test.15 = lowlevel Eq Test.10 Test.11; if Test.15 then let Test.7 = Index 1 Test.2; let Test.3 = Index 1 Test.7; @@ -571,13 +571,13 @@ mod test_mono { let Test.5 = CallByName Num.14 Test.1 Test.2; ret Test.5; in - let Test.7 = Index 0 Test.3; - let Test.8 = 4i64; - let Test.13 = lowlevel Eq Test.8 Test.7; + let Test.9 = Index 1 Test.3; + let Test.10 = 3i64; + let Test.13 = lowlevel Eq Test.10 Test.9; if Test.13 then - let Test.9 = Index 1 Test.3; - let Test.10 = 3i64; - let Test.12 = lowlevel Eq Test.10 Test.9; + let Test.7 = Index 0 Test.3; + let Test.8 = 4i64; + let Test.12 = lowlevel Eq Test.8 Test.7; if Test.12 then let Test.4 = 9i64; ret Test.4; @@ -1328,15 +1328,15 @@ mod test_mono { let Test.18 = Array []; ret Test.18; in - let Test.19 = Index 0 Test.7; - let Test.20 = 1i64; - let Test.21 = Index 0 Test.19; - let Test.27 = lowlevel Eq Test.20 Test.21; + let Test.22 = Index 1 Test.7; + let Test.23 = 1i64; + let Test.24 = Index 0 Test.22; + let Test.27 = lowlevel Eq Test.23 Test.24; if Test.27 then - let Test.22 = Index 1 Test.7; - let Test.23 = 1i64; - let Test.24 = Index 0 Test.22; - let Test.26 = lowlevel Eq Test.23 Test.24; + let Test.19 = Index 0 Test.7; + let Test.20 = 1i64; + let Test.21 = Index 0 Test.19; + let Test.26 = lowlevel Eq Test.20 Test.21; if Test.26 then let Test.17 = Index 0 Test.7; let Test.3 = Index 1 Test.17; @@ -2019,14 +2019,14 @@ mod test_mono { let Test.8 = 1i64; ret Test.8; in - let Test.9 = Index 1 Test.2; - let Test.10 = 0i64; - let Test.11 = Index 0 Test.9; - let Test.16 = lowlevel Eq Test.10 Test.11; + let Test.12 = 0i64; + let Test.13 = Index 0 Test.2; + let Test.16 = lowlevel Eq Test.12 Test.13; if Test.16 then - let Test.12 = 0i64; - let Test.13 = Index 0 Test.2; - let Test.15 = lowlevel Eq Test.12 Test.13; + let Test.9 = Index 1 Test.2; + let Test.10 = 0i64; + let Test.11 = Index 0 Test.9; + let Test.15 = lowlevel Eq Test.10 Test.11; if Test.15 then let Test.7 = Index 1 Test.2; let Test.3 = Index 1 Test.7; @@ -2155,15 +2155,15 @@ mod test_mono { let Test.21 = Array []; ret Test.21; in - let Test.22 = Index 0 Test.12; - let Test.23 = 1i64; - let Test.24 = Index 0 Test.22; - let Test.30 = lowlevel Eq Test.23 Test.24; + let Test.25 = Index 1 Test.12; + let Test.26 = 1i64; + let Test.27 = Index 0 Test.25; + let Test.30 = lowlevel Eq Test.26 Test.27; if Test.30 then - let Test.25 = Index 1 Test.12; - let Test.26 = 1i64; - let Test.27 = Index 0 Test.25; - let Test.29 = lowlevel Eq Test.26 Test.27; + let Test.22 = Index 0 Test.12; + let Test.23 = 1i64; + let Test.24 = Index 0 Test.22; + let Test.29 = lowlevel Eq Test.23 Test.24; if Test.29 then let Test.20 = Index 0 Test.12; let Test.5 = Index 1 Test.20; From d8f720a92f544ff9fc0dd79089cfdf9311f148fc Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Tue, 17 Nov 2020 09:48:19 +0100 Subject: [PATCH 11/37] renamed repl function --- cli/src/repl.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/src/repl.rs b/cli/src/repl.rs index 6714566aab..9b3ec67652 100644 --- a/cli/src/repl.rs +++ b/cli/src/repl.rs @@ -75,7 +75,7 @@ pub fn main() -> io::Result<()> { } else if prev_line_blank { // After two blank lines in a row, give up and try parsing it // even though it's going to fail. This way you don't get stuck. - match print_output(pending_src.as_str()) { + match eval_and_format(pending_src.as_str()) { Ok(output) => { println!("{}", output); } @@ -97,12 +97,12 @@ pub fn main() -> io::Result<()> { } _ => { let result = if pending_src.is_empty() { - print_output(line) + eval_and_format(line) } else { pending_src.push('\n'); pending_src.push_str(line); - print_output(pending_src.as_str()) + eval_and_format(pending_src.as_str()) }; match result { @@ -117,7 +117,7 @@ pub fn main() -> io::Result<()> { // If we hit an eof, and we're allowed to keep going, // append the str to the src we're building up and continue. // (We only need to append it here if it was empty before; - // otherwise, we already appended it before calling print_output.) + // otherwise, we already appended it before calling eval_and_format.) if pending_src.is_empty() { pending_src.push_str(line); @@ -141,7 +141,7 @@ fn report_parse_error(fail: Fail) { println!("TODO Gracefully report parse error in repl: {:?}", fail); } -fn print_output(src: &str) -> Result { +fn eval_and_format(src: &str) -> Result { gen(src.as_bytes(), Triple::host(), OptLevel::Normal).map(|output| match output { ReplOutput::NoProblems { expr, expr_type } => { format!("\n{} \u{001b}[35m:\u{001b}[0m {}", expr, expr_type) From 5a8d665a820caa14879b30bae2869236747fc441 Mon Sep 17 00:00:00 2001 From: Anton-4 <17049058+Anton-4@users.noreply.github.com> Date: Tue, 17 Nov 2020 10:17:57 +0100 Subject: [PATCH 12/37] gen fix for move of layout_id to roc_mono --- cli/src/repl/gen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/repl/gen.rs b/cli/src/repl/gen.rs index 9633216861..414e869f24 100644 --- a/cli/src/repl/gen.rs +++ b/cli/src/repl/gen.rs @@ -146,7 +146,7 @@ pub fn gen(src: &[u8], target: Triple, opt_level: OptLevel) -> Result Date: Tue, 17 Nov 2020 15:06:15 +0100 Subject: [PATCH 13/37] fix offset calculation --- compiler/mono/src/layout.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 904a98de12..ff3b38fbb9 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -432,26 +432,28 @@ impl<'a> Layout<'a> { } } - pub fn alignment_bytes(&self) -> u32 { + pub fn alignment_bytes(&self, pointer_size: u32) -> u32 { match self { Layout::Struct(fields) => fields .iter() - .map(|x| x.alignment_bytes()) + .map(|x| x.alignment_bytes(pointer_size)) .max() .unwrap_or(0), Layout::Union(tags) | Layout::RecursiveUnion(tags) => tags .iter() .map(|x| x.iter()) .flatten() - .map(|x| x.alignment_bytes()) + .map(|x| x.alignment_bytes(pointer_size)) .max() .unwrap_or(0), - Layout::Builtin(builtin) => builtin.alignment_bytes(), + Layout::Builtin(builtin) => builtin.alignment_bytes(pointer_size), Layout::PhantomEmptyStruct => 0, - Layout::RecursivePointer => std::mem::align_of::<*const u8>() as u32, - Layout::FunctionPointer(_, _) => std::mem::align_of::<*const u8>() as u32, - Layout::Pointer(_) => std::mem::align_of::<*const u8>() as u32, - Layout::Closure(_, _, _) => todo!(), + Layout::RecursivePointer => pointer_size, + Layout::FunctionPointer(_, _) => pointer_size, + Layout::Pointer(_) => pointer_size, + Layout::Closure(_, captured, _) => { + pointer_size.max(captured.layout.alignment_bytes(pointer_size)) + } } } @@ -656,10 +658,13 @@ impl<'a> Builtin<'a> { } } - pub fn alignment_bytes(&self) -> u32 { + pub fn alignment_bytes(&self, pointer_size: u32) -> u32 { use std::mem::align_of; use Builtin::*; + // for our data structures, what counts is the alignment of the `( ptr, len )` tuple, and + // since both of those are one pointer size, the alignment of that structure is a pointer + // size match self { Int128 => align_of::() as u32, Int64 => align_of::() as u32, @@ -671,11 +676,10 @@ impl<'a> Builtin<'a> { Float64 => align_of::() as u32, Float32 => align_of::() as u32, Float16 => align_of::() as u32, - Str | EmptyStr => align_of::() as u32, - Map(_, _) | EmptyMap => todo!(), - Set(_) | EmptySet => todo!(), - EmptyList => 0, - List(_, element_layout) => element_layout.alignment_bytes(), + Str | EmptyStr => pointer_size, + Map(_, _) | EmptyMap => pointer_size, + Set(_) | EmptySet => pointer_size, + List(_, _) | EmptyList => pointer_size, } } From b529decdffb8a9173336d9352f491785a35c74f0 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 15:27:17 +0100 Subject: [PATCH 14/37] factor out refcounting operations --- compiler/gen/src/llvm/refcounting.rs | 153 +++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 12a4f515b9..e51f42530e 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -28,6 +28,159 @@ pub fn refcount_1(ctx: &Context, ptr_bytes: u32) -> IntValue<'_> { } } +struct PointerToRefcount<'ctx> { + value: PointerValue<'ctx>, +} + +struct PointerToMalloced<'ctx> { + value: PointerValue<'ctx>, +} + +impl<'ctx> PointerToRefcount<'ctx> { + fn from_ptr_to_data<'a, 'env>(env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>) -> Self { + // pointer to usize + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let ptr_as_int = + cast_basic_basic(env.builder, data_ptr.into(), refcount_type.into()).into_int_value(); + + // subtract offset, to access the refcount + let refcount_ptr_as_int = env.builder.build_int_sub( + ptr_as_int, + refcount_type.const_int(env.ptr_bytes as u64, false), + "make_refcount_ptr", + ); + + let refcount_ptr = env.builder.build_int_to_ptr( + refcount_ptr_as_int, + refcount_type.ptr_type(AddressSpace::Generic), + "get_refcount_ptr", + ); + + Self { + value: refcount_ptr, + } + } + + fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { + env.builder + .build_load(self.value, "get_refcount") + .into_int_value() + } + + fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { + env.builder.build_store(self.value, refcount); + } + + fn increment<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) { + let refcount = self.get_refcount(env); + let builder = env.builder; + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let max = builder.build_int_compare( + IntPredicate::EQ, + refcount, + refcount_type.const_int(REFCOUNT_MAX as u64, false), + "refcount_max_check", + ); + let incremented = builder.build_int_add( + refcount, + refcount_type.const_int(1 as u64, false), + "increment_refcount", + ); + + let new_refcount = builder + .build_select(max, refcount, incremented, "select_refcount") + .into_int_value(); + + self.set_refcount(env, new_refcount); + } + + fn decrement<'a, 'env>( + &self, + env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, + _layout: &Layout<'a>, + ) { + let builder = env.builder; + let ctx = env.context; + let refcount_type = ptr_int(ctx, env.ptr_bytes); + + let merge_block = ctx.append_basic_block(parent, "merge"); + + let refcount_ptr = self.value; + + let refcount = env + .builder + .build_load(refcount_ptr, "get_refcount") + .into_int_value(); + + let add_with_overflow = env + .call_intrinsic( + LLVM_SADD_WITH_OVERFLOW_I64, + &[ + refcount.into(), + refcount_type.const_int((-1 as i64) as u64, true).into(), + ], + ) + .into_struct_value(); + + let has_overflowed = builder + .build_extract_value(add_with_overflow, 1, "has_overflowed") + .unwrap(); + + let has_overflowed_comparison = builder.build_int_compare( + IntPredicate::EQ, + has_overflowed.into_int_value(), + ctx.bool_type().const_int(1 as u64, false), + "has_overflowed", + ); + + // build blocks + let then_block = ctx.append_basic_block(parent, "then"); + let else_block = ctx.append_basic_block(parent, "else"); + + // TODO what would be most optimial for the branch predictor + // + // are most refcounts 1 most of the time? or not? + builder.build_conditional_branch(has_overflowed_comparison, then_block, else_block); + + // build then block + { + builder.position_at_end(then_block); + if !env.leak { + builder.build_free(self.value); + } + builder.build_unconditional_branch(merge_block); + } + + // build else block + { + builder.position_at_end(else_block); + + let max = builder.build_int_compare( + IntPredicate::EQ, + refcount, + refcount_type.const_int(REFCOUNT_MAX as u64, false), + "refcount_max_check", + ); + let decremented = builder + .build_extract_value(add_with_overflow, 0, "decrement_refcount") + .unwrap() + .into_int_value(); + let selected = builder.build_select(max, refcount, decremented, "select_refcount"); + + env.builder.build_store(refcount_ptr, selected); + // self.set_refcount(env, selected); + + builder.build_unconditional_branch(merge_block); + } + + // emit merge block + builder.position_at_end(merge_block); + } +} + pub fn decrement_refcount_struct<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, From da3454ca6c095d1d9c786e809930dcd7494fd456 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 15:59:03 +0100 Subject: [PATCH 15/37] improvements --- compiler/gen/src/llvm/refcounting.rs | 30 +++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index e51f42530e..473843147a 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -3,7 +3,7 @@ use crate::llvm::build::{ FAST_CALL_CONV, LLVM_SADD_WITH_OVERFLOW_I64, }; use crate::llvm::build_list::list_len; -use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; +use crate::llvm::convert::{basic_type_from_layout, block_of_memory, get_ptr_type, ptr_int}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -28,7 +28,7 @@ pub fn refcount_1(ctx: &Context, ptr_bytes: u32) -> IntValue<'_> { } } -struct PointerToRefcount<'ctx> { +pub struct PointerToRefcount<'ctx> { value: PointerValue<'ctx>, } @@ -37,7 +37,10 @@ struct PointerToMalloced<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { - fn from_ptr_to_data<'a, 'env>(env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>) -> Self { + pub fn from_ptr_to_data<'a, 'env>( + env: &Env<'a, 'ctx, 'env>, + data_ptr: PointerValue<'ctx>, + ) -> Self { // pointer to usize let refcount_type = ptr_int(env.context, env.ptr_bytes); @@ -62,13 +65,29 @@ impl<'ctx> PointerToRefcount<'ctx> { } } - fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { + pub fn from_list_wrapper(env: &Env<'_, 'ctx, '_>, list_wrapper: StructValue<'ctx>) -> Self { + let ptr_as_int = env + .builder + .build_extract_value(list_wrapper, Builtin::WRAPPER_PTR, "read_list_ptr") + .unwrap() + .into_int_value(); + + let ptr = env.builder.build_int_to_ptr( + ptr_as_int, + env.context.i64_type().ptr_type(AddressSpace::Generic), + "list_int_to_ptr", + ); + + Self::from_ptr_to_data(env, ptr) + } + + pub fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { env.builder .build_load(self.value, "get_refcount") .into_int_value() } - fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { + pub fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { env.builder.build_store(self.value, refcount); } @@ -102,6 +121,7 @@ impl<'ctx> PointerToRefcount<'ctx> { parent: FunctionValue<'ctx>, _layout: &Layout<'a>, ) { + panic!(); let builder = env.builder; let ctx = env.context; let refcount_type = ptr_int(ctx, env.ptr_bytes); From 71dadac1eaa5a40cedca72adea17bbb45513259f Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:04:15 +0100 Subject: [PATCH 16/37] use new increment --- compiler/gen/src/llvm/refcounting.rs | 52 +++++----------------------- 1 file changed, 8 insertions(+), 44 deletions(-) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 473843147a..886a5b76a2 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -3,7 +3,7 @@ use crate::llvm::build::{ FAST_CALL_CONV, LLVM_SADD_WITH_OVERFLOW_I64, }; use crate::llvm::build_list::list_len; -use crate::llvm::convert::{basic_type_from_layout, block_of_memory, get_ptr_type, ptr_int}; +use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -505,8 +505,8 @@ fn build_inc_list_help<'a, 'ctx, 'env>( builder.position_at_end(increment_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, original_wrapper); - increment_refcount_help(env, refcount_ptr); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + refcount_ptr.increment(env); builder.build_unconditional_branch(cont_block); @@ -700,8 +700,9 @@ fn build_inc_str_help<'a, 'ctx, 'env>( builder.build_conditional_branch(is_big_and_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, str_wrapper); - increment_refcount_help(env, refcount_ptr); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, str_wrapper); + refcount_ptr.increment(env); + builder.build_unconditional_branch(cont_block); builder.position_at_end(cont_block); @@ -805,44 +806,6 @@ fn build_dec_str_help<'a, 'ctx, 'env>( builder.build_return(None); } -fn increment_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - layout: &Layout<'a>, - field_ptr: PointerValue<'ctx>, -) { - let refcount_ptr = get_refcount_ptr(env, layout, field_ptr); - increment_refcount_help(env, refcount_ptr); -} - -fn increment_refcount_help<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - refcount_ptr: PointerValue<'ctx>, -) { - let builder = env.builder; - let ctx = env.context; - let refcount_type = ptr_int(ctx, env.ptr_bytes); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); - - let max = builder.build_int_compare( - IntPredicate::EQ, - refcount, - refcount_type.const_int(REFCOUNT_MAX as u64, false), - "refcount_max_check", - ); - let incremented = builder.build_int_add( - refcount, - refcount_type.const_int(1 as u64, false), - "increment_refcount", - ); - let selected = builder.build_select(max, refcount, incremented, "select_refcount"); - - builder.build_store(refcount_ptr, selected); -} - fn decrement_refcount_ptr<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, @@ -1303,7 +1266,8 @@ pub fn build_inc_union_help<'a, 'ctx, 'env>( // TODO do this decrement before the recursive call? // Then the recursive call is potentially TCE'd - increment_refcount_ptr(env, &layout, recursive_field_ptr); + let refcount_ptr = PointerToRefcount::from_ptr_to_data(env, recursive_field_ptr); + refcount_ptr.increment(env); } else if field_layout.contains_refcounted() { let field_ptr = env .builder From 0ea7126096bf87b3918e39218a84c2b4f88ded3a Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:06:40 +0100 Subject: [PATCH 17/37] use PointerToRefcount in == 1 comparison --- compiler/gen/src/llvm/build.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index bb14b8ff9a..df23426cfa 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -10,7 +10,7 @@ use crate::llvm::convert::{ }; use crate::llvm::refcounting::{ decrement_refcount_layout, increment_refcount_layout, list_get_refcount_ptr, - refcount_is_one_comparison, + refcount_is_one_comparison, PointerToRefcount, }; use bumpalo::collections::Vec; use bumpalo::Bump; @@ -2654,12 +2654,8 @@ where let ret_type = basic_type_from_layout(env.arena, ctx, list_layout, env.ptr_bytes); - let refcount_ptr = list_get_refcount_ptr(env, list_layout, original_wrapper); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + let refcount = refcount_ptr.get_refcount(env); let comparison = refcount_is_one_comparison(env, refcount); From 797dd2b73492f93f33fa1c1d84609203c1cc68f4 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:58:25 +0100 Subject: [PATCH 18/37] use new decrement --- compiler/gen/src/llvm/build.rs | 4 +- compiler/gen/src/llvm/refcounting.rs | 216 ++++++++++----------------- 2 files changed, 84 insertions(+), 136 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index df23426cfa..2c5f89a8d9 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -9,8 +9,8 @@ use crate::llvm::convert::{ basic_type_from_layout, block_of_memory, collection, get_fn_type, get_ptr_type, ptr_int, }; use crate::llvm::refcounting::{ - decrement_refcount_layout, increment_refcount_layout, list_get_refcount_ptr, - refcount_is_one_comparison, PointerToRefcount, + decrement_refcount_layout, increment_refcount_layout, refcount_is_one_comparison, + PointerToRefcount, }; use bumpalo::collections::Vec; use bumpalo::Bump; diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 886a5b76a2..03843a57e5 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -32,10 +32,6 @@ pub struct PointerToRefcount<'ctx> { value: PointerValue<'ctx>, } -struct PointerToMalloced<'ctx> { - value: PointerValue<'ctx>, -} - impl<'ctx> PointerToRefcount<'ctx> { pub fn from_ptr_to_data<'a, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -115,25 +111,67 @@ impl<'ctx> PointerToRefcount<'ctx> { self.set_refcount(env, new_refcount); } - fn decrement<'a, 'env>( - &self, + fn decrement<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) { + let context = env.context; + let block = env.builder.get_insert_block().expect("to be in a function"); + + let alignment = layout.alignment_bytes(env.ptr_bytes); + + let fn_name = &format!("decrement_refcounted_ptr_{}", alignment); + + let function = match env.module.get_function(fn_name) { + Some(function_value) => function_value, + None => { + // inc and dec return void + let fn_type = context.void_type().fn_type( + &[context.i64_type().ptr_type(AddressSpace::Generic).into()], + false, + ); + + let function_value = + env.module + .add_function(fn_name, fn_type, Some(Linkage::Private)); + + // Because it's an internal-only function, it should use the fast calling convention. + function_value.set_call_conventions(FAST_CALL_CONV); + + Self::_build_decrement_function_body(env, function_value, alignment); + + function_value + } + }; + + let refcount_ptr = self.value; + + env.builder.position_at_end(block); + let call = env + .builder + .build_call(function, &[refcount_ptr.into()], fn_name); + + call.set_call_convention(FAST_CALL_CONV); + } + + fn _build_decrement_function_body<'a, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, - _layout: &Layout<'a>, + extra_bytes: u32, ) { - panic!(); let builder = env.builder; let ctx = env.context; let refcount_type = ptr_int(ctx, env.ptr_bytes); - let merge_block = ctx.append_basic_block(parent, "merge"); + let entry = ctx.append_basic_block(parent, "entry"); + builder.position_at_end(entry); - let refcount_ptr = self.value; + let refcount_ptr = { + let raw_refcount_ptr = parent.get_nth_param(0).unwrap(); + debug_assert!(raw_refcount_ptr.is_pointer_value()); + Self { + value: raw_refcount_ptr.into_pointer_value(), + } + }; - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); + let refcount = refcount_ptr.get_refcount(env); let add_with_overflow = env .call_intrinsic( @@ -169,9 +207,20 @@ impl<'ctx> PointerToRefcount<'ctx> { { builder.position_at_end(then_block); if !env.leak { - builder.build_free(self.value); + match extra_bytes { + n if env.ptr_bytes == n => { + // the refcount ptr is also the ptr to the malloced region + builder.build_free(refcount_ptr.value); + } + n if 2 * env.ptr_bytes == n => { + // we need to step back another ptr_bytes to get the malloced ptr + let malloced = Self::from_ptr_to_data(env, refcount_ptr.value); + builder.build_free(malloced.value); + } + n => unreachable!("invalid extra_bytes {:?}", n), + } } - builder.build_unconditional_branch(merge_block); + builder.build_return(None); } // build else block @@ -190,14 +239,10 @@ impl<'ctx> PointerToRefcount<'ctx> { .into_int_value(); let selected = builder.build_select(max, refcount, decremented, "select_refcount"); - env.builder.build_store(refcount_ptr, selected); - // self.set_refcount(env, selected); + refcount_ptr.set_refcount(env, selected.into_int_value()); - builder.build_unconditional_branch(merge_block); + builder.build_return(None); } - - // emit merge block - builder.position_at_end(merge_block); } } @@ -583,7 +628,7 @@ fn build_dec_list_help<'a, 'ctx, 'env>( let parent = fn_val; // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); + let cont_block = ctx.append_basic_block(parent, "after_decrement_block_build_dec_list_help"); let decrement_block = ctx.append_basic_block(parent, "decrement_block"); // currently, an empty list has a null-pointer in its length is 0 @@ -604,9 +649,12 @@ fn build_dec_list_help<'a, 'ctx, 'env>( builder.build_conditional_branch(is_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, original_wrapper); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + refcount_ptr.decrement(env, layout); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); + env.builder.build_unconditional_branch(cont_block); + + builder.position_at_end(cont_block); // this function returns void builder.build_return(None); @@ -793,112 +841,23 @@ fn build_dec_str_help<'a, 'ctx, 'env>( ); // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); + let cont_block = ctx.append_basic_block(parent, "after_decrement_block_build_dec_str_help"); let decrement_block = ctx.append_basic_block(parent, "decrement_block"); builder.build_conditional_branch(is_big_and_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, str_wrapper); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, str_wrapper); + refcount_ptr.decrement(env, layout); + + builder.build_unconditional_branch(cont_block); + + builder.position_at_end(cont_block); // this function returns void builder.build_return(None); } -fn decrement_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - parent: FunctionValue<'ctx>, - layout: &Layout<'a>, - field_ptr: PointerValue<'ctx>, -) { - let ctx = env.context; - - // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); - - let refcount_ptr = get_refcount_ptr(env, layout, field_ptr); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); -} - -fn decrement_refcount_help<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - parent: FunctionValue<'ctx>, - refcount_ptr: PointerValue<'ctx>, - cont_block: BasicBlock, -) { - let builder = env.builder; - let ctx = env.context; - let refcount_type = ptr_int(ctx, env.ptr_bytes); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); - - let add_with_overflow = env - .call_intrinsic( - LLVM_SADD_WITH_OVERFLOW_I64, - &[ - refcount.into(), - refcount_type.const_int((-1 as i64) as u64, true).into(), - ], - ) - .into_struct_value(); - - let has_overflowed = builder - .build_extract_value(add_with_overflow, 1, "has_overflowed") - .unwrap(); - - let has_overflowed_comparison = builder.build_int_compare( - IntPredicate::EQ, - has_overflowed.into_int_value(), - ctx.bool_type().const_int(1 as u64, false), - "has_overflowed", - ); - - // build blocks - let then_block = ctx.append_basic_block(parent, "then"); - let else_block = ctx.append_basic_block(parent, "else"); - - // TODO what would be most optimial for the branch predictor - // - // are most refcounts 1 most of the time? or not? - builder.build_conditional_branch(has_overflowed_comparison, then_block, else_block); - - // build then block - { - builder.position_at_end(then_block); - if !env.leak { - builder.build_free(refcount_ptr); - } - builder.build_unconditional_branch(cont_block); - } - - // build else block - { - builder.position_at_end(else_block); - - let max = builder.build_int_compare( - IntPredicate::EQ, - refcount, - refcount_type.const_int(REFCOUNT_MAX as u64, false), - "refcount_max_check", - ); - let decremented = builder - .build_extract_value(add_with_overflow, 0, "decrement_refcount") - .unwrap() - .into_int_value(); - let selected = builder.build_select(max, refcount, decremented, "select_refcount"); - builder.build_store(refcount_ptr, selected); - - builder.build_unconditional_branch(cont_block); - } - - // emit merge block - builder.position_at_end(cont_block); -} - /// Build an increment or decrement function for a specific layout pub fn build_header<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -1062,7 +1021,8 @@ pub fn build_dec_union_help<'a, 'ctx, 'env>( // TODO do this decrement before the recursive call? // Then the recursive call is potentially TCE'd - decrement_refcount_ptr(env, parent, &layout, recursive_field_ptr); + let refcount_ptr = PointerToRefcount::from_ptr_to_data(env, recursive_field_ptr); + refcount_ptr.decrement(env, &layout); } else if field_layout.contains_refcounted() { let field_ptr = env .builder @@ -1321,18 +1281,6 @@ pub fn list_get_refcount_ptr<'a, 'ctx, 'env>( get_refcount_ptr_help(env, layout, ptr_as_int) } -fn get_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - layout: &Layout<'a>, - ptr: PointerValue<'ctx>, -) -> PointerValue<'ctx> { - let refcount_type = ptr_int(env.context, env.ptr_bytes); - let ptr_as_int = - cast_basic_basic(env.builder, ptr.into(), refcount_type.into()).into_int_value(); - - get_refcount_ptr_help(env, layout, ptr_as_int) -} - pub fn refcount_offset<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> u64 { let value_bytes = layout.stack_size(env.ptr_bytes) as u64; From 3e7ac183692aa06d54df064674b5c0e33a08404b Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 19:53:55 +0100 Subject: [PATCH 19/37] allocate taking alignment into account --- compiler/gen/src/llvm/build.rs | 65 +++++++++++++--------------- compiler/gen/src/llvm/refcounting.rs | 15 ++++++- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 2c5f89a8d9..eb8f5e99cf 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -989,57 +989,54 @@ pub fn allocate_with_refcount<'a, 'ctx, 'env>( // bytes per element let bytes_len = len_type.const_int(value_bytes, false); - let offset = crate::llvm::refcounting::refcount_offset(env, layout); + let extra_bytes = layout.alignment_bytes(env.ptr_bytes); + let extra_bytes_intvalue = len_type.const_int(extra_bytes as u64, false); let ptr = { let len = bytes_len; - let len = - builder.build_int_add(len, len_type.const_int(offset, false), "add_refcount_space"); + let len = builder.build_int_add(len, extra_bytes_intvalue, "add_alignment_space"); env.builder - .build_array_malloc(ctx.i8_type(), len, "create_list_ptr") + .build_array_malloc(ctx.i8_type(), len, "create_ptr") .unwrap() // TODO check if malloc returned null; if so, runtime error for OOM! }; // We must return a pointer to the first element: - let ptr_bytes = env.ptr_bytes; - let int_type = ptr_int(ctx, ptr_bytes); - let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "allocate_refcount_pti"); - let incremented = builder.build_int_add( - ptr_as_int, - ctx.i64_type().const_int(offset, false), - "increment_list_ptr", - ); + let data_ptr = { + let int_type = ptr_int(ctx, env.ptr_bytes); + let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "calculate_data_ptr_pti"); + let incremented = builder.build_int_add( + ptr_as_int, + extra_bytes_intvalue, + "calculate_data_ptr_increment", + ); - let ptr_type = get_ptr_type(&value_type, AddressSpace::Generic); - let list_element_ptr = builder.build_int_to_ptr(incremented, ptr_type, "allocate_refcount_itp"); + let ptr_type = get_ptr_type(&value_type, AddressSpace::Generic); + builder.build_int_to_ptr(incremented, ptr_type, "calculate_data_ptr") + }; - // subtract ptr_size, to access the refcount - let refcount_ptr = builder.build_int_sub( - incremented, - ctx.i64_type().const_int(env.ptr_bytes as u64, false), - "refcount_ptr", - ); + let refcount_ptr = match extra_bytes { + n if n == env.ptr_bytes => { + // the malloced pointer is the same as the refcounted pointer + unsafe { PointerToRefcount::from_ptr(env, ptr) } + } + n if n == 2 * env.ptr_bytes => { + // the refcount is stored just before the start of the actual data + // but in this case (because of alignment) not at the start of the malloced buffer + PointerToRefcount::from_ptr_to_data(env, data_ptr) + } + n => unreachable!("invalid extra_bytes {}", n), + }; - let refcount_ptr = builder.build_int_to_ptr( - refcount_ptr, - int_type.ptr_type(AddressSpace::Generic), - "make ptr", - ); - - // the refcount of a new allocation is initially 1 - // we assume that the allocation is indeed used (dead variables are eliminated) - builder.build_store( - refcount_ptr, - crate::llvm::refcounting::refcount_1(ctx, env.ptr_bytes), - ); + let rc1 = crate::llvm::refcounting::refcount_1(ctx, env.ptr_bytes); + refcount_ptr.set_refcount(env, rc1); // store the value in the pointer - builder.build_store(list_element_ptr, value); + builder.build_store(data_ptr, value); - list_element_ptr + data_ptr } fn list_literal<'a, 'ctx, 'env>( diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 03843a57e5..8455eab23a 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -5,7 +5,6 @@ use crate::llvm::build::{ use crate::llvm::build_list::list_len; use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; use bumpalo::collections::Vec; -use inkwell::basic_block::BasicBlock; use inkwell::context::Context; use inkwell::module::Linkage; use inkwell::values::{BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue}; @@ -33,6 +32,20 @@ pub struct PointerToRefcount<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { + pub unsafe fn from_ptr<'a, 'env>(env: &Env<'a, 'ctx, 'env>, ptr: PointerValue<'ctx>) -> Self { + // must make sure it's a pointer to usize + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let value = cast_basic_basic( + env.builder, + ptr.into(), + refcount_type.ptr_type(AddressSpace::Generic).into(), + ) + .into_pointer_value(); + + Self { value } + } + pub fn from_ptr_to_data<'a, 'env>( env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>, From f067f694deec947d3a3a59c058081648d5fe1682 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 19:58:38 +0100 Subject: [PATCH 20/37] add Safety docs to unsafe function --- compiler/gen/src/llvm/refcounting.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 8455eab23a..7f254808ed 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -32,6 +32,11 @@ pub struct PointerToRefcount<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { + /// # Safety + /// + /// the invariant is that the given pointer really points to the refcount, + /// not the data, and only is the start of the malloced buffer if the alignment + /// works out that way. pub unsafe fn from_ptr<'a, 'env>(env: &Env<'a, 'ctx, 'env>, ptr: PointerValue<'ctx>) -> Self { // must make sure it's a pointer to usize let refcount_type = ptr_int(env.context, env.ptr_bytes); From 75930caddb23cd836166e5579eb65f311dd9328f Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 15:06:15 +0100 Subject: [PATCH 21/37] fix offset calculation --- compiler/mono/src/layout.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 904a98de12..ff3b38fbb9 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -432,26 +432,28 @@ impl<'a> Layout<'a> { } } - pub fn alignment_bytes(&self) -> u32 { + pub fn alignment_bytes(&self, pointer_size: u32) -> u32 { match self { Layout::Struct(fields) => fields .iter() - .map(|x| x.alignment_bytes()) + .map(|x| x.alignment_bytes(pointer_size)) .max() .unwrap_or(0), Layout::Union(tags) | Layout::RecursiveUnion(tags) => tags .iter() .map(|x| x.iter()) .flatten() - .map(|x| x.alignment_bytes()) + .map(|x| x.alignment_bytes(pointer_size)) .max() .unwrap_or(0), - Layout::Builtin(builtin) => builtin.alignment_bytes(), + Layout::Builtin(builtin) => builtin.alignment_bytes(pointer_size), Layout::PhantomEmptyStruct => 0, - Layout::RecursivePointer => std::mem::align_of::<*const u8>() as u32, - Layout::FunctionPointer(_, _) => std::mem::align_of::<*const u8>() as u32, - Layout::Pointer(_) => std::mem::align_of::<*const u8>() as u32, - Layout::Closure(_, _, _) => todo!(), + Layout::RecursivePointer => pointer_size, + Layout::FunctionPointer(_, _) => pointer_size, + Layout::Pointer(_) => pointer_size, + Layout::Closure(_, captured, _) => { + pointer_size.max(captured.layout.alignment_bytes(pointer_size)) + } } } @@ -656,10 +658,13 @@ impl<'a> Builtin<'a> { } } - pub fn alignment_bytes(&self) -> u32 { + pub fn alignment_bytes(&self, pointer_size: u32) -> u32 { use std::mem::align_of; use Builtin::*; + // for our data structures, what counts is the alignment of the `( ptr, len )` tuple, and + // since both of those are one pointer size, the alignment of that structure is a pointer + // size match self { Int128 => align_of::() as u32, Int64 => align_of::() as u32, @@ -671,11 +676,10 @@ impl<'a> Builtin<'a> { Float64 => align_of::() as u32, Float32 => align_of::() as u32, Float16 => align_of::() as u32, - Str | EmptyStr => align_of::() as u32, - Map(_, _) | EmptyMap => todo!(), - Set(_) | EmptySet => todo!(), - EmptyList => 0, - List(_, element_layout) => element_layout.alignment_bytes(), + Str | EmptyStr => pointer_size, + Map(_, _) | EmptyMap => pointer_size, + Set(_) | EmptySet => pointer_size, + List(_, _) | EmptyList => pointer_size, } } From c007b391050e9f4b6066bba8eb4403ac38d9e50a Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 15:27:17 +0100 Subject: [PATCH 22/37] factor out refcounting operations --- compiler/gen/src/llvm/refcounting.rs | 153 +++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 12a4f515b9..e51f42530e 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -28,6 +28,159 @@ pub fn refcount_1(ctx: &Context, ptr_bytes: u32) -> IntValue<'_> { } } +struct PointerToRefcount<'ctx> { + value: PointerValue<'ctx>, +} + +struct PointerToMalloced<'ctx> { + value: PointerValue<'ctx>, +} + +impl<'ctx> PointerToRefcount<'ctx> { + fn from_ptr_to_data<'a, 'env>(env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>) -> Self { + // pointer to usize + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let ptr_as_int = + cast_basic_basic(env.builder, data_ptr.into(), refcount_type.into()).into_int_value(); + + // subtract offset, to access the refcount + let refcount_ptr_as_int = env.builder.build_int_sub( + ptr_as_int, + refcount_type.const_int(env.ptr_bytes as u64, false), + "make_refcount_ptr", + ); + + let refcount_ptr = env.builder.build_int_to_ptr( + refcount_ptr_as_int, + refcount_type.ptr_type(AddressSpace::Generic), + "get_refcount_ptr", + ); + + Self { + value: refcount_ptr, + } + } + + fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { + env.builder + .build_load(self.value, "get_refcount") + .into_int_value() + } + + fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { + env.builder.build_store(self.value, refcount); + } + + fn increment<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) { + let refcount = self.get_refcount(env); + let builder = env.builder; + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let max = builder.build_int_compare( + IntPredicate::EQ, + refcount, + refcount_type.const_int(REFCOUNT_MAX as u64, false), + "refcount_max_check", + ); + let incremented = builder.build_int_add( + refcount, + refcount_type.const_int(1 as u64, false), + "increment_refcount", + ); + + let new_refcount = builder + .build_select(max, refcount, incremented, "select_refcount") + .into_int_value(); + + self.set_refcount(env, new_refcount); + } + + fn decrement<'a, 'env>( + &self, + env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, + _layout: &Layout<'a>, + ) { + let builder = env.builder; + let ctx = env.context; + let refcount_type = ptr_int(ctx, env.ptr_bytes); + + let merge_block = ctx.append_basic_block(parent, "merge"); + + let refcount_ptr = self.value; + + let refcount = env + .builder + .build_load(refcount_ptr, "get_refcount") + .into_int_value(); + + let add_with_overflow = env + .call_intrinsic( + LLVM_SADD_WITH_OVERFLOW_I64, + &[ + refcount.into(), + refcount_type.const_int((-1 as i64) as u64, true).into(), + ], + ) + .into_struct_value(); + + let has_overflowed = builder + .build_extract_value(add_with_overflow, 1, "has_overflowed") + .unwrap(); + + let has_overflowed_comparison = builder.build_int_compare( + IntPredicate::EQ, + has_overflowed.into_int_value(), + ctx.bool_type().const_int(1 as u64, false), + "has_overflowed", + ); + + // build blocks + let then_block = ctx.append_basic_block(parent, "then"); + let else_block = ctx.append_basic_block(parent, "else"); + + // TODO what would be most optimial for the branch predictor + // + // are most refcounts 1 most of the time? or not? + builder.build_conditional_branch(has_overflowed_comparison, then_block, else_block); + + // build then block + { + builder.position_at_end(then_block); + if !env.leak { + builder.build_free(self.value); + } + builder.build_unconditional_branch(merge_block); + } + + // build else block + { + builder.position_at_end(else_block); + + let max = builder.build_int_compare( + IntPredicate::EQ, + refcount, + refcount_type.const_int(REFCOUNT_MAX as u64, false), + "refcount_max_check", + ); + let decremented = builder + .build_extract_value(add_with_overflow, 0, "decrement_refcount") + .unwrap() + .into_int_value(); + let selected = builder.build_select(max, refcount, decremented, "select_refcount"); + + env.builder.build_store(refcount_ptr, selected); + // self.set_refcount(env, selected); + + builder.build_unconditional_branch(merge_block); + } + + // emit merge block + builder.position_at_end(merge_block); + } +} + pub fn decrement_refcount_struct<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, From 64dd9cc1a965f63ffcd731f76a3f972f121a6002 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 15:59:03 +0100 Subject: [PATCH 23/37] improvements --- compiler/gen/src/llvm/refcounting.rs | 30 +++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index e51f42530e..473843147a 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -3,7 +3,7 @@ use crate::llvm::build::{ FAST_CALL_CONV, LLVM_SADD_WITH_OVERFLOW_I64, }; use crate::llvm::build_list::list_len; -use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; +use crate::llvm::convert::{basic_type_from_layout, block_of_memory, get_ptr_type, ptr_int}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -28,7 +28,7 @@ pub fn refcount_1(ctx: &Context, ptr_bytes: u32) -> IntValue<'_> { } } -struct PointerToRefcount<'ctx> { +pub struct PointerToRefcount<'ctx> { value: PointerValue<'ctx>, } @@ -37,7 +37,10 @@ struct PointerToMalloced<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { - fn from_ptr_to_data<'a, 'env>(env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>) -> Self { + pub fn from_ptr_to_data<'a, 'env>( + env: &Env<'a, 'ctx, 'env>, + data_ptr: PointerValue<'ctx>, + ) -> Self { // pointer to usize let refcount_type = ptr_int(env.context, env.ptr_bytes); @@ -62,13 +65,29 @@ impl<'ctx> PointerToRefcount<'ctx> { } } - fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { + pub fn from_list_wrapper(env: &Env<'_, 'ctx, '_>, list_wrapper: StructValue<'ctx>) -> Self { + let ptr_as_int = env + .builder + .build_extract_value(list_wrapper, Builtin::WRAPPER_PTR, "read_list_ptr") + .unwrap() + .into_int_value(); + + let ptr = env.builder.build_int_to_ptr( + ptr_as_int, + env.context.i64_type().ptr_type(AddressSpace::Generic), + "list_int_to_ptr", + ); + + Self::from_ptr_to_data(env, ptr) + } + + pub fn get_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>) -> IntValue<'ctx> { env.builder .build_load(self.value, "get_refcount") .into_int_value() } - fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { + pub fn set_refcount<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, refcount: IntValue<'ctx>) { env.builder.build_store(self.value, refcount); } @@ -102,6 +121,7 @@ impl<'ctx> PointerToRefcount<'ctx> { parent: FunctionValue<'ctx>, _layout: &Layout<'a>, ) { + panic!(); let builder = env.builder; let ctx = env.context; let refcount_type = ptr_int(ctx, env.ptr_bytes); From b2d13543734637cd8f4ff68a5e62cff61775042b Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:04:15 +0100 Subject: [PATCH 24/37] use new increment --- compiler/gen/src/llvm/refcounting.rs | 52 +++++----------------------- 1 file changed, 8 insertions(+), 44 deletions(-) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 473843147a..886a5b76a2 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -3,7 +3,7 @@ use crate::llvm::build::{ FAST_CALL_CONV, LLVM_SADD_WITH_OVERFLOW_I64, }; use crate::llvm::build_list::list_len; -use crate::llvm::convert::{basic_type_from_layout, block_of_memory, get_ptr_type, ptr_int}; +use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; use bumpalo::collections::Vec; use inkwell::basic_block::BasicBlock; use inkwell::context::Context; @@ -505,8 +505,8 @@ fn build_inc_list_help<'a, 'ctx, 'env>( builder.position_at_end(increment_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, original_wrapper); - increment_refcount_help(env, refcount_ptr); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + refcount_ptr.increment(env); builder.build_unconditional_branch(cont_block); @@ -700,8 +700,9 @@ fn build_inc_str_help<'a, 'ctx, 'env>( builder.build_conditional_branch(is_big_and_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, str_wrapper); - increment_refcount_help(env, refcount_ptr); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, str_wrapper); + refcount_ptr.increment(env); + builder.build_unconditional_branch(cont_block); builder.position_at_end(cont_block); @@ -805,44 +806,6 @@ fn build_dec_str_help<'a, 'ctx, 'env>( builder.build_return(None); } -fn increment_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - layout: &Layout<'a>, - field_ptr: PointerValue<'ctx>, -) { - let refcount_ptr = get_refcount_ptr(env, layout, field_ptr); - increment_refcount_help(env, refcount_ptr); -} - -fn increment_refcount_help<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - refcount_ptr: PointerValue<'ctx>, -) { - let builder = env.builder; - let ctx = env.context; - let refcount_type = ptr_int(ctx, env.ptr_bytes); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); - - let max = builder.build_int_compare( - IntPredicate::EQ, - refcount, - refcount_type.const_int(REFCOUNT_MAX as u64, false), - "refcount_max_check", - ); - let incremented = builder.build_int_add( - refcount, - refcount_type.const_int(1 as u64, false), - "increment_refcount", - ); - let selected = builder.build_select(max, refcount, incremented, "select_refcount"); - - builder.build_store(refcount_ptr, selected); -} - fn decrement_refcount_ptr<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, @@ -1303,7 +1266,8 @@ pub fn build_inc_union_help<'a, 'ctx, 'env>( // TODO do this decrement before the recursive call? // Then the recursive call is potentially TCE'd - increment_refcount_ptr(env, &layout, recursive_field_ptr); + let refcount_ptr = PointerToRefcount::from_ptr_to_data(env, recursive_field_ptr); + refcount_ptr.increment(env); } else if field_layout.contains_refcounted() { let field_ptr = env .builder From 94a8d07fe790b7d8aa2cfbe163ebed393b51f971 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:06:40 +0100 Subject: [PATCH 25/37] use PointerToRefcount in == 1 comparison --- compiler/gen/src/llvm/build.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index bb14b8ff9a..df23426cfa 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -10,7 +10,7 @@ use crate::llvm::convert::{ }; use crate::llvm::refcounting::{ decrement_refcount_layout, increment_refcount_layout, list_get_refcount_ptr, - refcount_is_one_comparison, + refcount_is_one_comparison, PointerToRefcount, }; use bumpalo::collections::Vec; use bumpalo::Bump; @@ -2654,12 +2654,8 @@ where let ret_type = basic_type_from_layout(env.arena, ctx, list_layout, env.ptr_bytes); - let refcount_ptr = list_get_refcount_ptr(env, list_layout, original_wrapper); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + let refcount = refcount_ptr.get_refcount(env); let comparison = refcount_is_one_comparison(env, refcount); From e02cc3af2fba52e0ad99c26be1b64838d55ecf55 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 16:58:25 +0100 Subject: [PATCH 26/37] use new decrement --- compiler/gen/src/llvm/build.rs | 4 +- compiler/gen/src/llvm/refcounting.rs | 216 ++++++++++----------------- 2 files changed, 84 insertions(+), 136 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index df23426cfa..2c5f89a8d9 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -9,8 +9,8 @@ use crate::llvm::convert::{ basic_type_from_layout, block_of_memory, collection, get_fn_type, get_ptr_type, ptr_int, }; use crate::llvm::refcounting::{ - decrement_refcount_layout, increment_refcount_layout, list_get_refcount_ptr, - refcount_is_one_comparison, PointerToRefcount, + decrement_refcount_layout, increment_refcount_layout, refcount_is_one_comparison, + PointerToRefcount, }; use bumpalo::collections::Vec; use bumpalo::Bump; diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 886a5b76a2..03843a57e5 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -32,10 +32,6 @@ pub struct PointerToRefcount<'ctx> { value: PointerValue<'ctx>, } -struct PointerToMalloced<'ctx> { - value: PointerValue<'ctx>, -} - impl<'ctx> PointerToRefcount<'ctx> { pub fn from_ptr_to_data<'a, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -115,25 +111,67 @@ impl<'ctx> PointerToRefcount<'ctx> { self.set_refcount(env, new_refcount); } - fn decrement<'a, 'env>( - &self, + fn decrement<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) { + let context = env.context; + let block = env.builder.get_insert_block().expect("to be in a function"); + + let alignment = layout.alignment_bytes(env.ptr_bytes); + + let fn_name = &format!("decrement_refcounted_ptr_{}", alignment); + + let function = match env.module.get_function(fn_name) { + Some(function_value) => function_value, + None => { + // inc and dec return void + let fn_type = context.void_type().fn_type( + &[context.i64_type().ptr_type(AddressSpace::Generic).into()], + false, + ); + + let function_value = + env.module + .add_function(fn_name, fn_type, Some(Linkage::Private)); + + // Because it's an internal-only function, it should use the fast calling convention. + function_value.set_call_conventions(FAST_CALL_CONV); + + Self::_build_decrement_function_body(env, function_value, alignment); + + function_value + } + }; + + let refcount_ptr = self.value; + + env.builder.position_at_end(block); + let call = env + .builder + .build_call(function, &[refcount_ptr.into()], fn_name); + + call.set_call_convention(FAST_CALL_CONV); + } + + fn _build_decrement_function_body<'a, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, - _layout: &Layout<'a>, + extra_bytes: u32, ) { - panic!(); let builder = env.builder; let ctx = env.context; let refcount_type = ptr_int(ctx, env.ptr_bytes); - let merge_block = ctx.append_basic_block(parent, "merge"); + let entry = ctx.append_basic_block(parent, "entry"); + builder.position_at_end(entry); - let refcount_ptr = self.value; + let refcount_ptr = { + let raw_refcount_ptr = parent.get_nth_param(0).unwrap(); + debug_assert!(raw_refcount_ptr.is_pointer_value()); + Self { + value: raw_refcount_ptr.into_pointer_value(), + } + }; - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); + let refcount = refcount_ptr.get_refcount(env); let add_with_overflow = env .call_intrinsic( @@ -169,9 +207,20 @@ impl<'ctx> PointerToRefcount<'ctx> { { builder.position_at_end(then_block); if !env.leak { - builder.build_free(self.value); + match extra_bytes { + n if env.ptr_bytes == n => { + // the refcount ptr is also the ptr to the malloced region + builder.build_free(refcount_ptr.value); + } + n if 2 * env.ptr_bytes == n => { + // we need to step back another ptr_bytes to get the malloced ptr + let malloced = Self::from_ptr_to_data(env, refcount_ptr.value); + builder.build_free(malloced.value); + } + n => unreachable!("invalid extra_bytes {:?}", n), + } } - builder.build_unconditional_branch(merge_block); + builder.build_return(None); } // build else block @@ -190,14 +239,10 @@ impl<'ctx> PointerToRefcount<'ctx> { .into_int_value(); let selected = builder.build_select(max, refcount, decremented, "select_refcount"); - env.builder.build_store(refcount_ptr, selected); - // self.set_refcount(env, selected); + refcount_ptr.set_refcount(env, selected.into_int_value()); - builder.build_unconditional_branch(merge_block); + builder.build_return(None); } - - // emit merge block - builder.position_at_end(merge_block); } } @@ -583,7 +628,7 @@ fn build_dec_list_help<'a, 'ctx, 'env>( let parent = fn_val; // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); + let cont_block = ctx.append_basic_block(parent, "after_decrement_block_build_dec_list_help"); let decrement_block = ctx.append_basic_block(parent, "decrement_block"); // currently, an empty list has a null-pointer in its length is 0 @@ -604,9 +649,12 @@ fn build_dec_list_help<'a, 'ctx, 'env>( builder.build_conditional_branch(is_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, original_wrapper); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, original_wrapper); + refcount_ptr.decrement(env, layout); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); + env.builder.build_unconditional_branch(cont_block); + + builder.position_at_end(cont_block); // this function returns void builder.build_return(None); @@ -793,112 +841,23 @@ fn build_dec_str_help<'a, 'ctx, 'env>( ); // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); + let cont_block = ctx.append_basic_block(parent, "after_decrement_block_build_dec_str_help"); let decrement_block = ctx.append_basic_block(parent, "decrement_block"); builder.build_conditional_branch(is_big_and_non_empty, decrement_block, cont_block); builder.position_at_end(decrement_block); - let refcount_ptr = list_get_refcount_ptr(env, layout, str_wrapper); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); + let refcount_ptr = PointerToRefcount::from_list_wrapper(env, str_wrapper); + refcount_ptr.decrement(env, layout); + + builder.build_unconditional_branch(cont_block); + + builder.position_at_end(cont_block); // this function returns void builder.build_return(None); } -fn decrement_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - parent: FunctionValue<'ctx>, - layout: &Layout<'a>, - field_ptr: PointerValue<'ctx>, -) { - let ctx = env.context; - - // the block we'll always jump to when we're done - let cont_block = ctx.append_basic_block(parent, "after_decrement_block"); - - let refcount_ptr = get_refcount_ptr(env, layout, field_ptr); - decrement_refcount_help(env, parent, refcount_ptr, cont_block); -} - -fn decrement_refcount_help<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - parent: FunctionValue<'ctx>, - refcount_ptr: PointerValue<'ctx>, - cont_block: BasicBlock, -) { - let builder = env.builder; - let ctx = env.context; - let refcount_type = ptr_int(ctx, env.ptr_bytes); - - let refcount = env - .builder - .build_load(refcount_ptr, "get_refcount") - .into_int_value(); - - let add_with_overflow = env - .call_intrinsic( - LLVM_SADD_WITH_OVERFLOW_I64, - &[ - refcount.into(), - refcount_type.const_int((-1 as i64) as u64, true).into(), - ], - ) - .into_struct_value(); - - let has_overflowed = builder - .build_extract_value(add_with_overflow, 1, "has_overflowed") - .unwrap(); - - let has_overflowed_comparison = builder.build_int_compare( - IntPredicate::EQ, - has_overflowed.into_int_value(), - ctx.bool_type().const_int(1 as u64, false), - "has_overflowed", - ); - - // build blocks - let then_block = ctx.append_basic_block(parent, "then"); - let else_block = ctx.append_basic_block(parent, "else"); - - // TODO what would be most optimial for the branch predictor - // - // are most refcounts 1 most of the time? or not? - builder.build_conditional_branch(has_overflowed_comparison, then_block, else_block); - - // build then block - { - builder.position_at_end(then_block); - if !env.leak { - builder.build_free(refcount_ptr); - } - builder.build_unconditional_branch(cont_block); - } - - // build else block - { - builder.position_at_end(else_block); - - let max = builder.build_int_compare( - IntPredicate::EQ, - refcount, - refcount_type.const_int(REFCOUNT_MAX as u64, false), - "refcount_max_check", - ); - let decremented = builder - .build_extract_value(add_with_overflow, 0, "decrement_refcount") - .unwrap() - .into_int_value(); - let selected = builder.build_select(max, refcount, decremented, "select_refcount"); - builder.build_store(refcount_ptr, selected); - - builder.build_unconditional_branch(cont_block); - } - - // emit merge block - builder.position_at_end(cont_block); -} - /// Build an increment or decrement function for a specific layout pub fn build_header<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -1062,7 +1021,8 @@ pub fn build_dec_union_help<'a, 'ctx, 'env>( // TODO do this decrement before the recursive call? // Then the recursive call is potentially TCE'd - decrement_refcount_ptr(env, parent, &layout, recursive_field_ptr); + let refcount_ptr = PointerToRefcount::from_ptr_to_data(env, recursive_field_ptr); + refcount_ptr.decrement(env, &layout); } else if field_layout.contains_refcounted() { let field_ptr = env .builder @@ -1321,18 +1281,6 @@ pub fn list_get_refcount_ptr<'a, 'ctx, 'env>( get_refcount_ptr_help(env, layout, ptr_as_int) } -fn get_refcount_ptr<'a, 'ctx, 'env>( - env: &Env<'a, 'ctx, 'env>, - layout: &Layout<'a>, - ptr: PointerValue<'ctx>, -) -> PointerValue<'ctx> { - let refcount_type = ptr_int(env.context, env.ptr_bytes); - let ptr_as_int = - cast_basic_basic(env.builder, ptr.into(), refcount_type.into()).into_int_value(); - - get_refcount_ptr_help(env, layout, ptr_as_int) -} - pub fn refcount_offset<'a, 'ctx, 'env>(env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) -> u64 { let value_bytes = layout.stack_size(env.ptr_bytes) as u64; From 9198638c237f6c5fe92bf55e216143f15a3a4f50 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 19:53:55 +0100 Subject: [PATCH 27/37] allocate taking alignment into account --- compiler/gen/src/llvm/build.rs | 65 +++++++++++++--------------- compiler/gen/src/llvm/refcounting.rs | 15 ++++++- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 2c5f89a8d9..eb8f5e99cf 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -989,57 +989,54 @@ pub fn allocate_with_refcount<'a, 'ctx, 'env>( // bytes per element let bytes_len = len_type.const_int(value_bytes, false); - let offset = crate::llvm::refcounting::refcount_offset(env, layout); + let extra_bytes = layout.alignment_bytes(env.ptr_bytes); + let extra_bytes_intvalue = len_type.const_int(extra_bytes as u64, false); let ptr = { let len = bytes_len; - let len = - builder.build_int_add(len, len_type.const_int(offset, false), "add_refcount_space"); + let len = builder.build_int_add(len, extra_bytes_intvalue, "add_alignment_space"); env.builder - .build_array_malloc(ctx.i8_type(), len, "create_list_ptr") + .build_array_malloc(ctx.i8_type(), len, "create_ptr") .unwrap() // TODO check if malloc returned null; if so, runtime error for OOM! }; // We must return a pointer to the first element: - let ptr_bytes = env.ptr_bytes; - let int_type = ptr_int(ctx, ptr_bytes); - let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "allocate_refcount_pti"); - let incremented = builder.build_int_add( - ptr_as_int, - ctx.i64_type().const_int(offset, false), - "increment_list_ptr", - ); + let data_ptr = { + let int_type = ptr_int(ctx, env.ptr_bytes); + let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "calculate_data_ptr_pti"); + let incremented = builder.build_int_add( + ptr_as_int, + extra_bytes_intvalue, + "calculate_data_ptr_increment", + ); - let ptr_type = get_ptr_type(&value_type, AddressSpace::Generic); - let list_element_ptr = builder.build_int_to_ptr(incremented, ptr_type, "allocate_refcount_itp"); + let ptr_type = get_ptr_type(&value_type, AddressSpace::Generic); + builder.build_int_to_ptr(incremented, ptr_type, "calculate_data_ptr") + }; - // subtract ptr_size, to access the refcount - let refcount_ptr = builder.build_int_sub( - incremented, - ctx.i64_type().const_int(env.ptr_bytes as u64, false), - "refcount_ptr", - ); + let refcount_ptr = match extra_bytes { + n if n == env.ptr_bytes => { + // the malloced pointer is the same as the refcounted pointer + unsafe { PointerToRefcount::from_ptr(env, ptr) } + } + n if n == 2 * env.ptr_bytes => { + // the refcount is stored just before the start of the actual data + // but in this case (because of alignment) not at the start of the malloced buffer + PointerToRefcount::from_ptr_to_data(env, data_ptr) + } + n => unreachable!("invalid extra_bytes {}", n), + }; - let refcount_ptr = builder.build_int_to_ptr( - refcount_ptr, - int_type.ptr_type(AddressSpace::Generic), - "make ptr", - ); - - // the refcount of a new allocation is initially 1 - // we assume that the allocation is indeed used (dead variables are eliminated) - builder.build_store( - refcount_ptr, - crate::llvm::refcounting::refcount_1(ctx, env.ptr_bytes), - ); + let rc1 = crate::llvm::refcounting::refcount_1(ctx, env.ptr_bytes); + refcount_ptr.set_refcount(env, rc1); // store the value in the pointer - builder.build_store(list_element_ptr, value); + builder.build_store(data_ptr, value); - list_element_ptr + data_ptr } fn list_literal<'a, 'ctx, 'env>( diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 03843a57e5..8455eab23a 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -5,7 +5,6 @@ use crate::llvm::build::{ use crate::llvm::build_list::list_len; use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; use bumpalo::collections::Vec; -use inkwell::basic_block::BasicBlock; use inkwell::context::Context; use inkwell::module::Linkage; use inkwell::values::{BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue}; @@ -33,6 +32,20 @@ pub struct PointerToRefcount<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { + pub unsafe fn from_ptr<'a, 'env>(env: &Env<'a, 'ctx, 'env>, ptr: PointerValue<'ctx>) -> Self { + // must make sure it's a pointer to usize + let refcount_type = ptr_int(env.context, env.ptr_bytes); + + let value = cast_basic_basic( + env.builder, + ptr.into(), + refcount_type.ptr_type(AddressSpace::Generic).into(), + ) + .into_pointer_value(); + + Self { value } + } + pub fn from_ptr_to_data<'a, 'env>( env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>, From b85f1e41283c90632252ad7393597473f37982c6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 19:58:38 +0100 Subject: [PATCH 28/37] add Safety docs to unsafe function --- compiler/gen/src/llvm/refcounting.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 8455eab23a..7f254808ed 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -32,6 +32,11 @@ pub struct PointerToRefcount<'ctx> { } impl<'ctx> PointerToRefcount<'ctx> { + /// # Safety + /// + /// the invariant is that the given pointer really points to the refcount, + /// not the data, and only is the start of the malloced buffer if the alignment + /// works out that way. pub unsafe fn from_ptr<'a, 'env>(env: &Env<'a, 'ctx, 'env>, ptr: PointerValue<'ctx>) -> Self { // must make sure it's a pointer to usize let refcount_type = ptr_int(env.context, env.ptr_bytes); From 7fcc05b0a40008ef6e97d8eafc5caed372e9d716 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 20:26:07 +0100 Subject: [PATCH 29/37] add dibuilder --- cli/src/repl/gen.rs | 4 ++++ compiler/build/src/program.rs | 3 +++ compiler/gen/src/llvm/build.rs | 21 +++++++++++++++++++++ compiler/gen/tests/helpers/eval.rs | 4 ++++ 4 files changed, 32 insertions(+) diff --git a/cli/src/repl/gen.rs b/cli/src/repl/gen.rs index 414e869f24..41f84661d6 100644 --- a/cli/src/repl/gen.rs +++ b/cli/src/repl/gen.rs @@ -133,10 +133,14 @@ pub fn gen(src: &[u8], target: Triple, opt_level: OptLevel) -> Result { pub arena: &'a Bump, pub context: &'ctx Context, pub builder: &'env Builder<'ctx>, + pub dibuilder: &'env DebugInfoBuilder<'ctx>, + pub compile_unit: &'env DICompileUnit<'ctx>, pub module: &'ctx Module<'ctx>, pub interns: Interns, pub ptr_bytes: u32, @@ -178,6 +181,24 @@ impl<'a, 'ctx, 'env> Env<'a, 'ctx, 'env> { ], ) } + + pub fn new_debug_info(module: &Module<'ctx>) -> (DebugInfoBuilder<'ctx>, DICompileUnit<'ctx>) { + module.create_debug_info_builder( + true, + /* language */ inkwell::debug_info::DWARFSourceLanguage::C, + /* filename */ "roc_app", + /* directory */ ".", + /* producer */ "my llvm compiler frontend", + /* is_optimized */ false, + /* compiler command line flags */ "", + /* runtime_ver */ 0, + /* split_name */ "", + /* kind */ inkwell::debug_info::DWARFEmissionKind::Full, + /* dwo_id */ 0, + /* split_debug_inling */ false, + /* debug_info_for_profiling */ false, + ) + } } pub fn module_from_builtins<'ctx>(ctx: &'ctx Context, module_name: &str) -> Module<'ctx> { diff --git a/compiler/gen/tests/helpers/eval.rs b/compiler/gen/tests/helpers/eval.rs index 36de9bff62..bbefabbaa7 100644 --- a/compiler/gen/tests/helpers/eval.rs +++ b/compiler/gen/tests/helpers/eval.rs @@ -152,10 +152,14 @@ pub fn helper<'a>( let (module_pass, function_pass) = roc_gen::llvm::build::construct_optimization_passes(module, opt_level); + let (dibuilder, compile_unit) = roc_gen::llvm::build::Env::new_debug_info(module); + // Compile and add all the Procs before adding main let env = roc_gen::llvm::build::Env { arena: &arena, builder: &builder, + dibuilder: &dibuilder, + compile_unit: &compile_unit, context, interns, module, From 4e7e196fe124f4c016e0873c456e96a779a1663a Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 17 Nov 2020 20:34:29 +0100 Subject: [PATCH 30/37] add function debug info helper --- compiler/gen/src/llvm/build.rs | 40 +++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 104a3f39e0..e658a40b3b 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -17,7 +17,7 @@ use bumpalo::Bump; use inkwell::basic_block::BasicBlock; use inkwell::builder::Builder; use inkwell::context::Context; -use inkwell::debug_info::{DICompileUnit, DebugInfoBuilder}; +use inkwell::debug_info::{DICompileUnit, DISubprogram, DebugInfoBuilder}; use inkwell::memory_buffer::MemoryBuffer; use inkwell::module::{Linkage, Module}; use inkwell::passes::{PassManager, PassManagerBuilder}; @@ -199,6 +199,44 @@ impl<'a, 'ctx, 'env> Env<'a, 'ctx, 'env> { /* debug_info_for_profiling */ false, ) } + + pub fn new_subprogram(&self, function_name: &str) -> DISubprogram<'ctx> { + use inkwell::debug_info::AsDIScope; + use inkwell::debug_info::DIFlagsConstants; + + let dibuilder = self.dibuilder; + let compile_unit = self.compile_unit; + + let ditype = dibuilder + .create_basic_type( + "type_name", + 0_u64, + 0x00, + inkwell::debug_info::DIFlags::PUBLIC, + ) + .unwrap(); + + let subroutine_type = dibuilder.create_subroutine_type( + compile_unit.get_file(), + /* return type */ Some(ditype.as_type()), + /* parameter types */ &[], + inkwell::debug_info::DIFlags::PUBLIC, + ); + + dibuilder.create_function( + /* scope */ compile_unit.as_debug_info_scope(), + /* func name */ function_name, + /* linkage_name */ None, + /* file */ compile_unit.get_file(), + /* line_no */ 0, + /* DIType */ subroutine_type, + /* is_local_to_unit */ true, + /* is_definition */ true, + /* scope_line */ 0, + /* flags */ inkwell::debug_info::DIFlags::PUBLIC, + /* is_optimized */ false, + ) + } } pub fn module_from_builtins<'ctx>(ctx: &'ctx Context, module_name: &str) -> Module<'ctx> { From 8df5d5c13ce9c18ed9b2d4ebf6985bbb34aad14a Mon Sep 17 00:00:00 2001 From: rvcas Date: Tue, 17 Nov 2020 21:53:49 -0500 Subject: [PATCH 31/37] feat(parse): support capturing a str in Pattern::Underscore --- compiler/can/src/pattern.rs | 2 +- compiler/fmt/src/pattern.rs | 4 ++-- compiler/parse/src/ast.rs | 5 +++-- compiler/parse/src/expr.rs | 6 ++++-- compiler/parse/tests/test_parse.rs | 10 +++++----- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/compiler/can/src/pattern.rs b/compiler/can/src/pattern.rs index 43e76963d2..c5cd46b202 100644 --- a/compiler/can/src/pattern.rs +++ b/compiler/can/src/pattern.rs @@ -197,7 +197,7 @@ pub fn canonicalize_pattern<'a>( ptype => unsupported_pattern(env, ptype, region), }, - Underscore => match pattern_type { + Underscore(_) => match pattern_type { WhenBranch | FunctionArg => Pattern::Underscore, ptype => unsupported_pattern(env, ptype, region), }, diff --git a/compiler/fmt/src/pattern.rs b/compiler/fmt/src/pattern.rs index 547ccf19c1..ad0982138d 100644 --- a/compiler/fmt/src/pattern.rs +++ b/compiler/fmt/src/pattern.rs @@ -37,7 +37,7 @@ impl<'a> Formattable<'a> for Pattern<'a> { | Pattern::NonBase10Literal { .. } | Pattern::FloatLiteral(_) | Pattern::StrLiteral(_) - | Pattern::Underscore + | Pattern::Underscore(_) | Pattern::Malformed(_) | Pattern::QualifiedIdentifier { .. } => false, } @@ -128,7 +128,7 @@ impl<'a> Formattable<'a> for Pattern<'a> { StrLiteral(literal) => { todo!("Format string literal: {:?}", literal); } - Underscore => buf.push('_'), + Underscore(name) => buf.push_str(name), // Space SpaceBefore(sub_pattern, spaces) => { diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 1d15acba21..6abdac6439 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -437,7 +437,7 @@ pub enum Pattern<'a> { }, FloatLiteral(&'a str), StrLiteral(StrLiteral<'a>), - Underscore, + Underscore(&'a str), // Space SpaceBefore(&'a Pattern<'a>, &'a [CommentOrNewline<'a>]), @@ -554,7 +554,8 @@ impl<'a> Pattern<'a> { ) => string_x == string_y && base_x == base_y && is_negative_x == is_negative_y, (FloatLiteral(x), FloatLiteral(y)) => x == y, (StrLiteral(x), StrLiteral(y)) => x == y, - (Underscore, Underscore) => true, + // TODO do we want to compare anything here? + (Underscore(_), Underscore(_)) => true, // Space (SpaceBefore(x, _), SpaceBefore(y, _)) => x.equivalent(y), diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index df2fe8017f..5de9a4c121 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -666,7 +666,7 @@ fn annotation_or_alias<'a>( NumLiteral(_) | NonBase10Literal { .. } | FloatLiteral(_) | StrLiteral(_) => { Def::NotYetImplemented("TODO gracefully handle trying to annotate a litera") } - Underscore => { + Underscore(_) => { Def::NotYetImplemented("TODO gracefully handle trying to give a type annotation to an undrscore") } Malformed(_) => { @@ -1086,7 +1086,9 @@ fn string_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { } fn underscore_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { - map!(ascii_char(b'_'), |_| Pattern::Underscore) + map_with_arena!(ascii_char(b'_'), |_arena, _thing| { + Pattern::Underscore(&"_") + }) } fn record_destructure<'a>(min_indent: u16) -> impl Parser<'a, Pattern<'a>> { diff --git a/compiler/parse/tests/test_parse.rs b/compiler/parse/tests/test_parse.rs index 82d1818d9d..473c4a4c97 100644 --- a/compiler/parse/tests/test_parse.rs +++ b/compiler/parse/tests/test_parse.rs @@ -1414,7 +1414,7 @@ mod test_parse { #[test] fn single_underscore_closure() { let arena = Bump::new(); - let pattern = Located::new(0, 0, 1, 2, Underscore); + let pattern = Located::new(0, 0, 1, 2, Underscore(&"_")); let patterns = &[pattern]; let expected = Closure(patterns, arena.alloc(Located::new(0, 0, 6, 8, Num("42")))); let actual = parse_expr_with(&arena, "\\_ -> 42"); @@ -1464,8 +1464,8 @@ mod test_parse { #[test] fn closure_with_underscores() { let arena = Bump::new(); - let underscore1 = Located::new(0, 0, 1, 2, Underscore); - let underscore2 = Located::new(0, 0, 4, 5, Underscore); + let underscore1 = Located::new(0, 0, 1, 2, Underscore(&"_")); + let underscore2 = Located::new(0, 0, 4, 5, Underscore(&"_")); let patterns = bumpalo::vec![in &arena; underscore1, underscore2]; let expected = Closure( arena.alloc(patterns), @@ -2477,7 +2477,7 @@ mod test_parse { guard: None, }); let newlines = &[Newline]; - let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore), newlines); + let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"_")), newlines); let loc_pattern2 = Located::new(2, 2, 4, 5, pattern2); let expr2 = Num("4"); let loc_expr2 = Located::new(2, 2, 9, 10, expr2); @@ -2522,7 +2522,7 @@ mod test_parse { guard: None, }); let newlines = &[Newline]; - let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore), newlines); + let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"_")), newlines); let loc_pattern2 = Located::new(2, 2, 4, 5, pattern2); let expr2 = Num("4"); let loc_expr2 = Located::new(2, 2, 9, 10, expr2); From 00880806364289c48011da8fcf511a2ca9fc5e26 Mon Sep 17 00:00:00 2001 From: rvcas Date: Tue, 17 Nov 2020 23:46:40 -0500 Subject: [PATCH 32/37] check point for review --- compiler/parse/src/expr.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 5de9a4c121..ef0740a0b7 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -1086,9 +1086,18 @@ fn string_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { } fn underscore_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { - map_with_arena!(ascii_char(b'_'), |_arena, _thing| { - Pattern::Underscore(&"_") - }) + move |arena: &'a Bump, state: State<'a>| { + let (_, next_state) = ascii_char(b'_').parse(arena, state)?; + + let (output, final_state) = lowercase_ident().parse(arena, next_state)?; + + let mut name = String::new_in(arena); + + name.push('_'); + name.push_str(output); + + Ok((Pattern::Underscore(&"_"), final_state)) + } } fn record_destructure<'a>(min_indent: u16) -> impl Parser<'a, Pattern<'a>> { From 1e7dcaebd8660d81f869d10575c6573fd81b66cf Mon Sep 17 00:00:00 2001 From: rvcas Date: Wed, 18 Nov 2020 07:35:01 -0500 Subject: [PATCH 33/37] finish parsing named underscores --- compiler/parse/src/expr.rs | 12 +++++------- compiler/parse/tests/test_parse.rs | 6 +++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index ef0740a0b7..d518bb823a 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -1089,14 +1089,12 @@ fn underscore_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { move |arena: &'a Bump, state: State<'a>| { let (_, next_state) = ascii_char(b'_').parse(arena, state)?; - let (output, final_state) = lowercase_ident().parse(arena, next_state)?; + let (output, final_state) = optional(lowercase_ident()).parse(arena, next_state)?; - let mut name = String::new_in(arena); - - name.push('_'); - name.push_str(output); - - Ok((Pattern::Underscore(&"_"), final_state)) + match output { + Some(name) => Ok((Pattern::Underscore(name), final_state)), + None => Ok((Pattern::Underscore(&"_"), final_state)), + } } } diff --git a/compiler/parse/tests/test_parse.rs b/compiler/parse/tests/test_parse.rs index 473c4a4c97..d889843f92 100644 --- a/compiler/parse/tests/test_parse.rs +++ b/compiler/parse/tests/test_parse.rs @@ -1465,13 +1465,13 @@ mod test_parse { fn closure_with_underscores() { let arena = Bump::new(); let underscore1 = Located::new(0, 0, 1, 2, Underscore(&"_")); - let underscore2 = Located::new(0, 0, 4, 5, Underscore(&"_")); + let underscore2 = Located::new(0, 0, 4, 9, Underscore(&"name")); let patterns = bumpalo::vec![in &arena; underscore1, underscore2]; let expected = Closure( arena.alloc(patterns), - arena.alloc(Located::new(0, 0, 9, 11, Num("42"))), + arena.alloc(Located::new(0, 0, 13, 15, Num("42"))), ); - let actual = parse_expr_with(&arena, "\\_, _ -> 42"); + let actual = parse_expr_with(&arena, "\\_, _name -> 42"); assert_eq!(Ok(expected), actual); } From f471d5bbe12ed4bd6fc660787b374e785a4c2942 Mon Sep 17 00:00:00 2001 From: rvcas Date: Wed, 18 Nov 2020 07:40:06 -0500 Subject: [PATCH 34/37] we do want to compare underscore values --- compiler/parse/src/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 6abdac6439..4529a0c930 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -555,7 +555,7 @@ impl<'a> Pattern<'a> { (FloatLiteral(x), FloatLiteral(y)) => x == y, (StrLiteral(x), StrLiteral(y)) => x == y, // TODO do we want to compare anything here? - (Underscore(_), Underscore(_)) => true, + (Underscore(x), Underscore(y)) => x == y, // Space (SpaceBefore(x, _), SpaceBefore(y, _)) => x.equivalent(y), From 72c85009c65485ad5686224464afc67ca4a039ba Mon Sep 17 00:00:00 2001 From: rvcas Date: Wed, 18 Nov 2020 08:25:40 -0500 Subject: [PATCH 35/37] do not set an underscore on parse and only prefix in formatter --- compiler/fmt/src/pattern.rs | 5 ++++- compiler/parse/src/ast.rs | 1 - compiler/parse/src/expr.rs | 2 +- compiler/parse/tests/test_parse.rs | 8 ++++---- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/compiler/fmt/src/pattern.rs b/compiler/fmt/src/pattern.rs index ad0982138d..3f4fb2667f 100644 --- a/compiler/fmt/src/pattern.rs +++ b/compiler/fmt/src/pattern.rs @@ -128,7 +128,10 @@ impl<'a> Formattable<'a> for Pattern<'a> { StrLiteral(literal) => { todo!("Format string literal: {:?}", literal); } - Underscore(name) => buf.push_str(name), + Underscore(name) => { + buf.push('_'); + buf.push_str(name); + } // Space SpaceBefore(sub_pattern, spaces) => { diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 4529a0c930..579c8f2fc3 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -554,7 +554,6 @@ impl<'a> Pattern<'a> { ) => string_x == string_y && base_x == base_y && is_negative_x == is_negative_y, (FloatLiteral(x), FloatLiteral(y)) => x == y, (StrLiteral(x), StrLiteral(y)) => x == y, - // TODO do we want to compare anything here? (Underscore(x), Underscore(y)) => x == y, // Space diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index d518bb823a..11453dbb25 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -1093,7 +1093,7 @@ fn underscore_pattern<'a>() -> impl Parser<'a, Pattern<'a>> { match output { Some(name) => Ok((Pattern::Underscore(name), final_state)), - None => Ok((Pattern::Underscore(&"_"), final_state)), + None => Ok((Pattern::Underscore(&""), final_state)), } } } diff --git a/compiler/parse/tests/test_parse.rs b/compiler/parse/tests/test_parse.rs index d889843f92..9ac546d639 100644 --- a/compiler/parse/tests/test_parse.rs +++ b/compiler/parse/tests/test_parse.rs @@ -1414,7 +1414,7 @@ mod test_parse { #[test] fn single_underscore_closure() { let arena = Bump::new(); - let pattern = Located::new(0, 0, 1, 2, Underscore(&"_")); + let pattern = Located::new(0, 0, 1, 2, Underscore(&"")); let patterns = &[pattern]; let expected = Closure(patterns, arena.alloc(Located::new(0, 0, 6, 8, Num("42")))); let actual = parse_expr_with(&arena, "\\_ -> 42"); @@ -1464,7 +1464,7 @@ mod test_parse { #[test] fn closure_with_underscores() { let arena = Bump::new(); - let underscore1 = Located::new(0, 0, 1, 2, Underscore(&"_")); + let underscore1 = Located::new(0, 0, 1, 2, Underscore(&"")); let underscore2 = Located::new(0, 0, 4, 9, Underscore(&"name")); let patterns = bumpalo::vec![in &arena; underscore1, underscore2]; let expected = Closure( @@ -2477,7 +2477,7 @@ mod test_parse { guard: None, }); let newlines = &[Newline]; - let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"_")), newlines); + let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"")), newlines); let loc_pattern2 = Located::new(2, 2, 4, 5, pattern2); let expr2 = Num("4"); let loc_expr2 = Located::new(2, 2, 9, 10, expr2); @@ -2522,7 +2522,7 @@ mod test_parse { guard: None, }); let newlines = &[Newline]; - let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"_")), newlines); + let pattern2 = Pattern::SpaceBefore(arena.alloc(Underscore(&"")), newlines); let loc_pattern2 = Located::new(2, 2, 4, 5, pattern2); let expr2 = Num("4"); let loc_expr2 = Located::new(2, 2, 9, 10, expr2); From 6f4585e7d84c587f2e4531bbd3e2a2367985f23e Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 18 Nov 2020 19:24:49 +0100 Subject: [PATCH 36/37] add subprogram debug info --- compiler/gen/src/llvm/build.rs | 74 ++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index e658a40b3b..b136b44c8b 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -17,7 +17,7 @@ use bumpalo::Bump; use inkwell::basic_block::BasicBlock; use inkwell::builder::Builder; use inkwell::context::Context; -use inkwell::debug_info::{DICompileUnit, DISubprogram, DebugInfoBuilder}; +use inkwell::debug_info::{AsDIScope, DICompileUnit, DISubprogram, DebugInfoBuilder}; use inkwell::memory_buffer::MemoryBuffer; use inkwell::module::{Linkage, Module}; use inkwell::passes::{PassManager, PassManagerBuilder}; @@ -201,7 +201,6 @@ impl<'a, 'ctx, 'env> Env<'a, 'ctx, 'env> { } pub fn new_subprogram(&self, function_name: &str) -> DISubprogram<'ctx> { - use inkwell::debug_info::AsDIScope; use inkwell::debug_info::DIFlagsConstants; let dibuilder = self.dibuilder; @@ -1042,21 +1041,20 @@ pub fn allocate_with_refcount<'a, 'ctx, 'env>( let ctx = env.context; let value_type = basic_type_from_layout(env.arena, ctx, layout, env.ptr_bytes); - let value_bytes = layout.stack_size(env.ptr_bytes) as u64; + let value_bytes = layout.stack_size(env.ptr_bytes); let len_type = env.ptr_int(); - // bytes per element - let bytes_len = len_type.const_int(value_bytes, false); let extra_bytes = layout.alignment_bytes(env.ptr_bytes); - let extra_bytes_intvalue = len_type.const_int(extra_bytes as u64, false); let ptr = { - let len = bytes_len; - let len = builder.build_int_add(len, extra_bytes_intvalue, "add_alignment_space"); + let len = value_bytes as u64 + extra_bytes as u64; + + // bytes per element + let bytes_len = len_type.const_int(len, false); env.builder - .build_array_malloc(ctx.i8_type(), len, "create_ptr") + .build_array_malloc(ctx.i8_type(), bytes_len, "create_ptr") .unwrap() // TODO check if malloc returned null; if so, runtime error for OOM! @@ -1065,15 +1063,33 @@ pub fn allocate_with_refcount<'a, 'ctx, 'env>( // We must return a pointer to the first element: let data_ptr = { let int_type = ptr_int(ctx, env.ptr_bytes); - let ptr_as_int = builder.build_ptr_to_int(ptr, int_type, "calculate_data_ptr_pti"); - let incremented = builder.build_int_add( - ptr_as_int, - extra_bytes_intvalue, - "calculate_data_ptr_increment", - ); + let as_usize_ptr = cast_basic_basic( + env.builder, + ptr.into(), + int_type.ptr_type(AddressSpace::Generic).into(), + ) + .into_pointer_value(); + + let index = match extra_bytes { + n if n == env.ptr_bytes => 1, + n if n == 2 * env.ptr_bytes => 2, + _ => unreachable!("invalid extra_bytes"), + }; + + let index_intvalue = int_type.const_int(index, false); let ptr_type = get_ptr_type(&value_type, AddressSpace::Generic); - builder.build_int_to_ptr(incremented, ptr_type, "calculate_data_ptr") + + unsafe { + cast_basic_basic( + env.builder, + env.builder + .build_in_bounds_gep(as_usize_ptr, &[index_intvalue], "get_data_ptr") + .into(), + ptr_type.into(), + ) + .into_pointer_value() + } }; let refcount_ptr = match extra_bytes { @@ -1666,7 +1682,10 @@ fn expose_function_to_host<'a, 'ctx, 'env>( ) { let c_function_name: String = format!("{}_exposed", roc_function.get_name().to_str().unwrap()); - expose_function_to_host_help(env, roc_function, &c_function_name); + let result = expose_function_to_host_help(env, roc_function, &c_function_name); + + let subprogram = env.new_subprogram(&c_function_name); + result.set_subprogram(subprogram); } fn expose_function_to_host_help<'a, 'ctx, 'env>( @@ -2011,6 +2030,9 @@ pub fn build_proc_header<'a, 'ctx, 'env>( fn_val.set_call_conventions(FAST_CALL_CONV); + let subprogram = env.new_subprogram(&fn_name); + fn_val.set_subprogram(subprogram); + if env.exposed_to_host.contains(&symbol) { expose_function_to_host(env, fn_val); } @@ -2150,6 +2172,24 @@ pub fn build_proc<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + context, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + + builder.set_current_debug_location(&context, loc); + // Add args to scope for (arg_val, (layout, arg_symbol)) in fn_val.get_param_iter().zip(args) { set_name(arg_val, arg_symbol.ident_string(&env.interns)); From 5ce8a665f0ec088b285271b5c69eb5c3ce326d8d Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 18 Nov 2020 20:08:43 +0100 Subject: [PATCH 37/37] add finalize everywhere, and fix some oversights --- cli/src/repl/gen.rs | 5 + compiler/build/src/program.rs | 9 ++ compiler/gen/src/llvm/build.rs | 20 +++ compiler/gen/src/llvm/refcounting.rs | 196 ++++++++++++++++++++++++--- compiler/gen/tests/helpers/eval.rs | 5 + 5 files changed, 213 insertions(+), 22 deletions(-) diff --git a/cli/src/repl/gen.rs b/cli/src/repl/gen.rs index 41f84661d6..9c1a1c1389 100644 --- a/cli/src/repl/gen.rs +++ b/cli/src/repl/gen.rs @@ -180,6 +180,9 @@ pub fn gen(src: &[u8], target: Triple, opt_level: OptLevel) -> Result Result( env.module .add_function(c_function_name, c_function_type, Some(Linkage::External)); + let subprogram = env.new_subprogram(c_function_name); + c_function.set_subprogram(subprogram); + // STEP 2: build the exposed function's body let builder = env.builder; let context = env.context; @@ -1719,6 +1722,23 @@ fn expose_function_to_host_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = c_function.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + env.context, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(env.context, loc); + // drop the final argument, which is the pointer we write the result into let args = c_function.get_params(); let output_arg_index = args.len() - 1; diff --git a/compiler/gen/src/llvm/refcounting.rs b/compiler/gen/src/llvm/refcounting.rs index 7f254808ed..a8652d41cb 100644 --- a/compiler/gen/src/llvm/refcounting.rs +++ b/compiler/gen/src/llvm/refcounting.rs @@ -6,6 +6,7 @@ use crate::llvm::build_list::list_len; use crate::llvm::convert::{basic_type_from_layout, block_of_memory, ptr_int}; use bumpalo::collections::Vec; use inkwell::context::Context; +use inkwell::debug_info::AsDIScope; use inkwell::module::Linkage; use inkwell::values::{BasicValueEnum, FunctionValue, IntValue, PointerValue, StructValue}; use inkwell::{AddressSpace, IntPredicate}; @@ -55,24 +56,22 @@ impl<'ctx> PointerToRefcount<'ctx> { env: &Env<'a, 'ctx, 'env>, data_ptr: PointerValue<'ctx>, ) -> Self { + let builder = env.builder; // pointer to usize let refcount_type = ptr_int(env.context, env.ptr_bytes); - let ptr_as_int = - cast_basic_basic(env.builder, data_ptr.into(), refcount_type.into()).into_int_value(); + let ptr_as_usize_ptr = cast_basic_basic( + builder, + data_ptr.into(), + refcount_type.ptr_type(AddressSpace::Generic).into(), + ) + .into_pointer_value(); - // subtract offset, to access the refcount - let refcount_ptr_as_int = env.builder.build_int_sub( - ptr_as_int, - refcount_type.const_int(env.ptr_bytes as u64, false), - "make_refcount_ptr", - ); - - let refcount_ptr = env.builder.build_int_to_ptr( - refcount_ptr_as_int, - refcount_type.ptr_type(AddressSpace::Generic), - "get_refcount_ptr", - ); + // get a pointer to index -1 + let index_intvalue = refcount_type.const_int((-1 as i64) as u64, false); + let refcount_ptr = unsafe { + builder.build_in_bounds_gep(ptr_as_usize_ptr, &[index_intvalue], "get_rc_ptr") + }; Self { value: refcount_ptr, @@ -132,6 +131,7 @@ impl<'ctx> PointerToRefcount<'ctx> { fn decrement<'a, 'env>(&self, env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>) { let context = env.context; let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let alignment = layout.alignment_bytes(env.ptr_bytes); @@ -153,6 +153,9 @@ impl<'ctx> PointerToRefcount<'ctx> { // Because it's an internal-only function, it should use the fast calling convention. function_value.set_call_conventions(FAST_CALL_CONV); + let subprogram = env.new_subprogram(fn_name); + function_value.set_subprogram(subprogram); + Self::_build_decrement_function_body(env, function_value, alignment); function_value @@ -162,6 +165,8 @@ impl<'ctx> PointerToRefcount<'ctx> { let refcount_ptr = self.value; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[refcount_ptr.into()], fn_name); @@ -181,6 +186,24 @@ impl<'ctx> PointerToRefcount<'ctx> { let entry = ctx.append_basic_block(parent, "entry"); builder.position_at_end(entry); + let subprogram = parent.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ subprogram.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + &ctx, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + + env.builder.set_current_debug_location(&ctx, loc); + let refcount_ptr = { let raw_refcount_ptr = parent.get_nth_param(0).unwrap(); debug_assert!(raw_refcount_ptr.is_pointer_value()); @@ -490,6 +513,7 @@ pub fn build_inc_list<'a, 'ctx, 'env>( original_wrapper: StructValue<'ctx>, ) { let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::INC; let fn_name = layout_ids @@ -499,7 +523,7 @@ pub fn build_inc_list<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_inc_list_help(env, layout_ids, layout, function_value); @@ -508,6 +532,8 @@ pub fn build_inc_list<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[original_wrapper.into()], "increment_list"); @@ -529,6 +555,23 @@ fn build_inc_list_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + ctx, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&ctx, loc); + let mut scope = Scope::default(); // Add args to scope @@ -586,6 +629,7 @@ pub fn build_dec_list<'a, 'ctx, 'env>( original_wrapper: StructValue<'ctx>, ) { let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::DEC; let fn_name = layout_ids @@ -595,7 +639,7 @@ pub fn build_dec_list<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_dec_list_help(env, layout_ids, layout, function_value); @@ -604,6 +648,8 @@ pub fn build_dec_list<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[original_wrapper.into()], "decrement_list"); @@ -624,6 +670,23 @@ fn build_dec_list_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + ctx, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&ctx, loc); + let mut scope = Scope::default(); // Add args to scope @@ -685,6 +748,7 @@ pub fn build_inc_str<'a, 'ctx, 'env>( original_wrapper: StructValue<'ctx>, ) { let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::INC; let fn_name = layout_ids @@ -694,7 +758,7 @@ pub fn build_inc_str<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_inc_str_help(env, layout_ids, layout, function_value); @@ -703,6 +767,8 @@ pub fn build_inc_str<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[original_wrapper.into()], "increment_str"); @@ -723,6 +789,23 @@ fn build_inc_str_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + ctx, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&ctx, loc); + let mut scope = Scope::default(); // Add args to scope @@ -784,6 +867,7 @@ pub fn build_dec_str<'a, 'ctx, 'env>( original_wrapper: StructValue<'ctx>, ) { let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::DEC; let fn_name = layout_ids @@ -793,7 +877,7 @@ pub fn build_dec_str<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_dec_str_help(env, layout_ids, layout, function_value); @@ -802,6 +886,8 @@ pub fn build_dec_str<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[original_wrapper.into()], "decrement_str"); @@ -822,6 +908,23 @@ fn build_dec_str_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + ctx, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&ctx, loc); + let mut scope = Scope::default(); // Add args to scope @@ -880,7 +983,7 @@ fn build_dec_str_help<'a, 'ctx, 'env>( pub fn build_header<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, layout: &Layout<'a>, - fn_name: String, + fn_name: &str, ) -> FunctionValue<'ctx> { let arena = env.arena; let context = &env.context; @@ -892,11 +995,16 @@ pub fn build_header<'a, 'ctx, 'env>( let fn_val = env .module - .add_function(fn_name.as_str(), fn_type, Some(Linkage::Private)); + .add_function(fn_name, fn_type, Some(Linkage::Private)); // Because it's an internal-only function, it should use the fast calling convention. fn_val.set_call_conventions(FAST_CALL_CONV); + let subprogram = env.new_subprogram(&fn_name); + fn_val.set_subprogram(subprogram); + + env.dibuilder.finalize(); + fn_val } @@ -909,6 +1017,7 @@ pub fn build_dec_union<'a, 'ctx, 'env>( let layout = Layout::Union(fields); let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::DEC; let fn_name = layout_ids @@ -918,7 +1027,7 @@ pub fn build_dec_union<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_dec_union_help(env, layout_ids, fields, function_value); @@ -927,6 +1036,9 @@ pub fn build_dec_union<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); + let call = env .builder .build_call(function, &[value], "decrement_union"); @@ -952,11 +1064,29 @@ pub fn build_dec_union_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + context, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&context, loc); + let mut scope = Scope::default(); // Add args to scope let arg_symbol = Symbol::ARG_1; let layout = Layout::Union(tags); + let arg_val = fn_val.get_param_iter().next().unwrap(); set_name(arg_val, arg_symbol.ident_string(&env.interns)); @@ -984,6 +1114,8 @@ pub fn build_dec_union_help<'a, 'ctx, 'env>( let merge_block = env.context.append_basic_block(parent, "decrement_merge"); + builder.set_current_debug_location(&context, loc); + for (tag_id, field_layouts) in tags.iter().enumerate() { // if none of the fields are or contain anything refcounted, just move on if !field_layouts @@ -1097,6 +1229,7 @@ pub fn build_inc_union<'a, 'ctx, 'env>( let layout = Layout::Union(fields); let block = env.builder.get_insert_block().expect("to be in a function"); + let di_location = env.builder.get_current_debug_location().unwrap(); let symbol = Symbol::INC; let fn_name = layout_ids @@ -1106,7 +1239,7 @@ pub fn build_inc_union<'a, 'ctx, 'env>( let function = match env.module.get_function(fn_name.as_str()) { Some(function_value) => function_value, None => { - let function_value = build_header(env, &layout, fn_name); + let function_value = build_header(env, &layout, &fn_name); build_inc_union_help(env, layout_ids, fields, function_value); @@ -1115,6 +1248,8 @@ pub fn build_inc_union<'a, 'ctx, 'env>( }; env.builder.position_at_end(block); + env.builder + .set_current_debug_location(env.context, di_location); let call = env .builder .build_call(function, &[value], "increment_union"); @@ -1140,6 +1275,23 @@ pub fn build_inc_union_help<'a, 'ctx, 'env>( builder.position_at_end(entry); + let func_scope = fn_val.get_subprogram().unwrap(); + let lexical_block = env.dibuilder.create_lexical_block( + /* scope */ func_scope.as_debug_info_scope(), + /* file */ env.compile_unit.get_file(), + /* line_no */ 0, + /* column_no */ 0, + ); + + let loc = env.dibuilder.create_debug_location( + context, + /* line */ 0, + /* column */ 0, + /* current_scope */ lexical_block.as_debug_info_scope(), + /* inlined_at */ None, + ); + builder.set_current_debug_location(&context, loc); + let mut scope = Scope::default(); // Add args to scope diff --git a/compiler/gen/tests/helpers/eval.rs b/compiler/gen/tests/helpers/eval.rs index bbefabbaa7..95959224dd 100644 --- a/compiler/gen/tests/helpers/eval.rs +++ b/compiler/gen/tests/helpers/eval.rs @@ -199,6 +199,9 @@ pub fn helper<'a>( build_proc(&env, &mut layout_ids, scope.clone(), proc, fn_val); + // call finalize() before any code generation/verification + env.dibuilder.finalize(); + if fn_val.verify(true) { function_pass.run_on(&fn_val); } else { @@ -232,6 +235,8 @@ pub fn helper<'a>( &main_fn_layout, ); + env.dibuilder.finalize(); + // Uncomment this to see the module's un-optimized LLVM instruction output: // env.module.print_to_stderr();