-
Notifications
You must be signed in to change notification settings - Fork 640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compiler optimizations #7680
Compiler optimizations #7680
Changes from all commits
aabbeb7
17b38d3
188de63
4ef27e3
cc6a700
9533dd1
68a1f3c
1b699e8
d494603
14c227e
57df760
47c0c70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,29 @@ fn simplify_expression(expr: &mut Expression) -> bool { | |
} | ||
('|', Expression::BoolLiteral(false), e) => Some(std::mem::take(e)), | ||
('|', e, Expression::BoolLiteral(false)) => Some(std::mem::take(e)), | ||
('>', Expression::NumberLiteral(a, un1), Expression::NumberLiteral(b, un2)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could also work for ≤ and ≥ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wondering how >= fits in one character, but I guess I got my answer lol |
||
if un1 == un2 => | ||
{ | ||
Some(Expression::BoolLiteral(*a > *b)) | ||
} | ||
('<', Expression::NumberLiteral(a, un1), Expression::NumberLiteral(b, un2)) | ||
if un1 == un2 => | ||
{ | ||
Some(Expression::BoolLiteral(*a < *b)) | ||
} | ||
_ => None, | ||
}; | ||
if let Some(new) = new { | ||
*expr = new; | ||
} | ||
can_inline | ||
} | ||
Expression::UnaryOp { sub, op } => { | ||
ogoffart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let can_inline = simplify_expression(sub); | ||
let new = match (*op, &mut **sub) { | ||
('!', Expression::BoolLiteral(b)) => Some(Expression::BoolLiteral(!*b)), | ||
('-', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(-*n, *u)), | ||
('+', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(*n, *u)), | ||
_ => None, | ||
}; | ||
if let Some(new) = new { | ||
|
@@ -148,6 +171,38 @@ fn simplify_expression(expr: &mut Expression) -> bool { | |
} | ||
can_inline | ||
} | ||
Expression::Condition { condition, true_expr, false_expr } => { | ||
let mut can_inline = simplify_expression(condition); | ||
can_inline &= match &**condition { | ||
Expression::BoolLiteral(true) => { | ||
*expr = *true_expr.clone(); | ||
simplify_expression(expr) | ||
} | ||
Expression::BoolLiteral(false) => { | ||
*expr = *false_expr.clone(); | ||
simplify_expression(expr) | ||
} | ||
_ => simplify_expression(true_expr) && simplify_expression(false_expr), | ||
}; | ||
can_inline | ||
} | ||
Expression::CodeBlock(stmts) if stmts.len() == 1 => { | ||
*expr = stmts[0].clone(); | ||
simplify_expression(expr) | ||
} | ||
Expression::FunctionCall { function, arguments, .. } => { | ||
let mut args_can_inline = true; | ||
for arg in arguments.iter_mut() { | ||
args_can_inline &= simplify_expression(arg); | ||
} | ||
if args_can_inline { | ||
if let Some(inlined) = try_inline_function(function, arguments) { | ||
*expr = inlined; | ||
return true; | ||
} | ||
} | ||
false | ||
ogoffart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
Expression::ElementReference { .. } => false, | ||
Expression::LayoutCacheAccess { .. } => false, | ||
Expression::SolveLayout { .. } => false, | ||
|
@@ -199,6 +254,30 @@ fn extract_constant_property_reference(nr: &NamedReference) -> Option<Expression | |
Some(expression) | ||
} | ||
|
||
fn try_inline_function(function: &Callable, arguments: &[Expression]) -> Option<Expression> { | ||
let Callable::Function(function) = function else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for future improvements: we could also try to inline some built-in functions. |
||
return None; | ||
}; | ||
if !function.is_constant() { | ||
return None; | ||
} | ||
let mut body = extract_constant_property_reference(function)?; | ||
|
||
body.visit_recursive_mut(&mut |e| { | ||
if let Expression::FunctionParameterReference { index, ty } = e { | ||
let Some(e_new) = arguments.get(*index).cloned() else { return }; | ||
ogoffart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if e_new.ty() == *ty { | ||
*e = e_new; | ||
} | ||
} | ||
}); | ||
if simplify_expression(&mut body) { | ||
Some(body) | ||
} else { | ||
None | ||
} | ||
} | ||
|
||
#[test] | ||
fn test() { | ||
let mut compiler_config = | ||
|
@@ -210,12 +289,16 @@ fn test() { | |
/* ... */ | ||
struct Hello { s: string, v: float } | ||
global G { | ||
property <float> p : 3 * 2 + 15 ; | ||
pure function complicated(a: float ) -> bool { if a > 5 { return true; }; if a < 1 { return true; }; uncomplicated() } | ||
pure function uncomplicated( ) -> bool { false } | ||
out property <float> p : 3 * 2 + 15 ; | ||
property <string> q: "foo " + 42; | ||
out property <Hello> out: { s: q, v: p }; | ||
out property <float> w : -p / 2; | ||
out property <Hello> out: { s: q, v: complicated(w + 15) ? -123 : p }; | ||
} | ||
export component Foo { | ||
out property<float> out: G.out.v; | ||
out property<float> out1: G.w; | ||
out property<float> out2: G.out.v; | ||
} | ||
"# | ||
.into(), | ||
|
@@ -226,20 +309,17 @@ export component Foo { | |
spin_on::spin_on(crate::compile_syntax_node(doc_node, test_diags, compiler_config)); | ||
assert!(!diag.has_errors(), "slint compile error {:#?}", diag.to_string_vec()); | ||
|
||
let out_binding = doc | ||
.inner_components | ||
.last() | ||
.unwrap() | ||
.root_element | ||
.borrow() | ||
.bindings | ||
.get("out") | ||
.unwrap() | ||
.borrow() | ||
.expression | ||
.clone(); | ||
match &out_binding { | ||
Expression::NumberLiteral(n, _) => assert_eq!(*n, (3 * 2 + 15) as f64), | ||
_ => panic!("not number {out_binding:?}"), | ||
let expected_p = 3.0 * 2.0 + 15.0; | ||
let expected_w = -expected_p / 2.0; | ||
let bindings = &doc.inner_components.last().unwrap().root_element.borrow().bindings; | ||
let out1_binding = bindings.get("out1").unwrap().borrow().expression.clone(); | ||
match &out1_binding { | ||
Expression::NumberLiteral(n, _) => assert_eq!(*n, expected_w), | ||
_ => panic!("not number {out1_binding:?}"), | ||
} | ||
let out2_binding = bindings.get("out2").unwrap().borrow().expression.clone(); | ||
match &out2_binding { | ||
Expression::NumberLiteral(n, _) => assert_eq!(*n, expected_p), | ||
_ => panic!("not number {out2_binding:?}"), | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this actually happen?
I guess it's good to be resilient in this debug code anyway. But it is a bit worrisome if that happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something as simple as
cargo run --bin slint-compiler -- -f llr ./tests/cases/widgets/tableview.slint
crashes the printer on the most recent master.I guess there's an actual code generation issue here, but I didn't want to investigate. In fact I didn't want to push this commit at all, but it accidentally happened :D