Skip to content

Commit

Permalink
fix: Disable inlining until it can be made sound
Browse files Browse the repository at this point in the history
  • Loading branch information
@Marwes
Marwes committed Aug 11, 2020
1 parent f9051e3 commit 97b63c4
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 134 deletions.
13 changes: 0 additions & 13 deletions check/tests/pass.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1135,16 +1135,3 @@ match writer with
"#,
"test.List String"
}

test_check! {
issue_863,
r#"
#[infix(right, 0)]
let (<|) f x : (a -> b) -> a -> b = f x
let g f x = x
let f a =
g a <| f a
{ f }
"#
}
17 changes: 15 additions & 2 deletions tests/inline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ use support::*;

mod support;

use gluon::{self, query::{Compilation, AsyncCompilation}, vm::core::tests::check_expr_eq, ThreadExt};

use gluon::{
self,
query::{AsyncCompilation, Compilation},
vm::core::tests::check_expr_eq,
ThreadExt,
};

#[ignore]
#[tokio::test]
async fn inline_cross_module() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -35,6 +41,7 @@ async fn inline_cross_module() {
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn inline_with_record_pattern_in_module() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -81,6 +88,7 @@ num.(+) 3
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn inline_across_two_modules() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -111,6 +119,7 @@ async fn inline_across_two_modules() {
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn prune_prelude() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -141,6 +150,7 @@ async fn prune_prelude() {
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn prune_factorial() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -184,6 +194,7 @@ async fn prune_factorial() {
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn inline_num() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -253,6 +264,7 @@ match A with
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn inline_cmp() {
let _ = env_logger::try_init();
Expand Down Expand Up @@ -286,6 +298,7 @@ m.(<)
check_expr_eq(core_expr.value.expr(), expected_str);
}

#[ignore]
#[tokio::test]
async fn inline_option() {
let _ = env_logger::try_init();
Expand Down
136 changes: 17 additions & 119 deletions vm/src/core/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{
use itertools::Itertools;

use crate::base::{
ast::{Typed, TypedIdent},
ast::TypedIdent,
fnv::{FnvMap, FnvSet},
kind::{ArcKind, KindEnv},
merge::{merge_collect, merge_fn},
Expand Down Expand Up @@ -364,6 +364,7 @@ impl<'l> ReducedExpr<'l> {
}

impl<'l> ReducedClosure<'l> {
#[allow(dead_code)]
fn with<R>(
self,
allocator: &'l Allocator<'l>,
Expand Down Expand Up @@ -623,6 +624,7 @@ impl<'l, 'g> FunctionEnv<'l, 'g> {
pub(crate) struct Compiler<'a, 'e> {
allocator: &'e Allocator<'e>,
globals: &'a dyn Fn(&Symbol) -> Option<GlobalBinding>,
#[allow(dead_code)]
env: &'a dyn OptimizeEnv<Type = ArcType>,
local_bindings: ScopedMap<Symbol, Option<CostBinding<'e>>>,
all_local_bindings: FnvMap<Symbol, CostBinding<'e>>,
Expand Down Expand Up @@ -873,6 +875,7 @@ impl<'a, 'e> Compiler<'a, 'e> {
added
}

#[allow(dead_code)]
fn cost(&self, resolver: &dyn Resolver<'_, '_>, id: &SymbolRef) -> Cost {
let cost = self.costs.cost(id);
if cost == Cost::max_value() {
Expand Down Expand Up @@ -1167,6 +1170,7 @@ impl<'a, 'e> Compiler<'a, 'e> {
}
}

#[allow(dead_code)]
fn update_types(
&mut self,
map: &mut FnvMap<Symbol, ArcType>,
Expand Down Expand Up @@ -1701,7 +1705,7 @@ impl<'a, 'e> Compiler<'a, 'e> {

fn inline_call<'b>(
&mut self,
function: &mut FunctionEnvs<'e, 'a>,
_function: &mut FunctionEnvs<'e, 'a>,
resolver: &dyn Resolver<'e, 'b>,
expr: CExpr<'b>,
f: CExpr<'b>,
Expand All @@ -1722,123 +1726,7 @@ impl<'a, 'e> Compiler<'a, 'e> {
}
_ => None,
},
Binding::Closure(closure) => {
let (closure_id, closure_args, closure_body) = closure.as_ref();

let cost = closure.clone().with(
self.allocator,
self.inlined_global_bindings,
|resolver, _| self.cost(resolver, &*closure_id.name),
);
trace!("COST {:?}: {}", closure_id.name, cost);

if cost > 40 || closure_args.len() > args.len() {
None
} else {
function.start_function(self);
trace!("{} -- {}", closure_args.len(), args.len());

self.bindings_in_scope.enter_scope();
self.local_bindings.enter_scope();

let mut no_inline_args = Vec::new();

let all_args = args.clone();

for (name, value) in closure_args
.iter()
.zip(args.by_ref().map(|arg| resolver.wrap(arg)))
{
if !self
.push_inline_stack_var(name.name.clone(), Some(value.clone().into()))
{
let inline_name = Symbol::from("inline_bind");
let typ = value.as_ref().try_type_of(&self.env);
let typed_ident = TypedIdent {
name: inline_name,
typ: typ.unwrap(),
};
let expr = &*self
.allocator
.arena
.alloc(Expr::Ident(typed_ident.clone(), value.as_ref().span()));
self.bindings_in_scope.insert(typed_ident.name.clone(), ());
no_inline_args.push((typed_ident, value));
self.local_bindings.insert(
name.name.clone(),
Some(CostBinding {
cost: 0,
bind: expr.into(),
}),
);
}
}

let expr = closure_body
.with(
self.allocator,
self.inlined_global_bindings,
|resolver, closure_body| self.compile(resolver, closure_body, function),
)
.map(|new_expr| {
let mut map = FnvMap::default();
for (arg_name, arg_value) in closure_args
.iter()
.zip(all_args.map(|arg| resolver.wrap(arg)))
{
check::unify_type::instantiation(
&self.env,
&mut |id, typ| {
map.insert(id.clone(), typ.clone());
},
&arg_name.typ,
&arg_value.as_ref().env_type_of(&self.env),
);
}

let new_expr =
self.update_types(&mut map, new_expr).unwrap_or(new_expr);

let allocator = self.allocator;

let args = allocator
.arena
.alloc_fixed(args.map(|e| resolver.produce(e).clone()));
let new_expr = if !args.is_empty() {
// TODO Avoid allocating args and cloning them after into the
// slice
allocator.arena.alloc(Expr::Call(new_expr, args))
} else {
new_expr
};
trace!("INLINED {} ==>> {}", expr, new_expr);
new_expr
})
.map(|body| {
no_inline_args
.into_iter()
.rev()
.fold(body, |body, (name, expr)| {
let expr = expr
.into_local(&self.inlined_global_bindings, &self.allocator);
&*self.allocator.arena.alloc(Expr::Let(
self.allocator.let_binding_arena.alloc(LetBinding {
name,
span_start: expr.span().start(),
expr: Named::Expr(expr),
}),
body,
))
})
});

self.bindings_in_scope.exit_scope();
self.local_bindings.exit_scope();

function.end_function(self);
expr
}
}
Binding::Closure(..) => None,
}
}

Expand Down Expand Up @@ -2422,6 +2310,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, "3");
}

#[ignore]
#[test]
fn fold_function_call_basic() {
let _ = ::env_logger::try_init();
Expand All @@ -2433,6 +2322,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, "3");
}

#[ignore]
#[test]
fn fold_function_call_with_unknown_parameters() {
let _ = ::env_logger::try_init();
Expand All @@ -2451,6 +2341,7 @@ pub(crate) mod tests {
);
}

#[ignore]
#[test]
fn fold_extra_arguments() {
let _ = ::env_logger::try_init();
Expand All @@ -2463,6 +2354,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, "3");
}

#[ignore]
#[test]
fn fold_function_call_partial() {
let _ = ::env_logger::try_init();
Expand All @@ -2475,6 +2367,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, "16");
}

#[ignore]
#[test]
fn fold_function_call_implicit() {
let _ = ::env_logger::try_init();
Expand All @@ -2488,6 +2381,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, "3");
}

#[ignore]
#[test]
fn fold_applicative() {
let _ = ::env_logger::try_init();
Expand Down Expand Up @@ -2616,6 +2510,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, expected);
}

#[ignore]
#[test]
fn factorial() {
let _ = ::env_logger::try_init();
Expand Down Expand Up @@ -2648,6 +2543,7 @@ pub(crate) mod tests {
assert_eq_expr!(expr, expected);
}

#[ignore]
#[test]
fn match_record_nested_match() {
let _ = ::env_logger::try_init();
Expand Down Expand Up @@ -2722,6 +2618,7 @@ pub(crate) mod tests {
}
}

#[ignore]
#[test]
fn fold_global_function_call_with_unknown_parameters() {
let _ = ::env_logger::try_init();
Expand All @@ -2746,6 +2643,7 @@ pub(crate) mod tests {
);
}

#[ignore]
#[test]
fn fold_global_function_call_through_two_modules_simple() {
let _ = ::env_logger::try_init();
Expand Down

0 comments on commit 97b63c4

Please sign in to comment.