Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 92 additions & 6 deletions cranelift/isle/isle/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,25 @@ struct BodyContext<'a, W> {
is_bound: StableSet<BindingId>,
term_name: &'a str,
emit_logging: bool,

// Extra fields for iterator-returning terms.
// These fields are used to generate optimized Rust code for iterator-returning terms.
/// The number of match splits that have been generated.
/// This is used to generate unique names for the match splits.
match_split: usize,

/// The action to take when the iterator overflows.
iter_overflow_action: &'static str,
}

impl<'a, W: Write> BodyContext<'a, W> {
fn new(out: &'a mut W, ruleset: &'a RuleSet, term_name: &'a str, emit_logging: bool) -> Self {
fn new(
out: &'a mut W,
ruleset: &'a RuleSet,
term_name: &'a str,
emit_logging: bool,
iter_overflow_action: &'static str,
) -> Self {
Self {
out,
ruleset,
Expand All @@ -84,6 +99,8 @@ impl<'a, W: Write> BodyContext<'a, W> {
is_bound: Default::default(),
term_name,
emit_logging,
match_split: Default::default(),
iter_overflow_action,
}
}

Expand Down Expand Up @@ -426,7 +443,17 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{

let termdata = &self.termenv.terms[termid.index()];
let term_name = &self.typeenv.syms[termdata.name.index()];
let mut ctx = BodyContext::new(code, ruleset, term_name, options.emit_logging);

// Split a match if the term returns an iterator.
let mut ctx = BodyContext::new(
code,
ruleset,
term_name,
options.emit_logging,
"return;", // At top level, we just return.
);

// Generate the function signature.
writeln!(ctx.out)?;
writeln!(
ctx.out,
Expand Down Expand Up @@ -470,6 +497,7 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
ReturnKind::Option => write!(ctx.out, "Option<{ret}>")?,
ReturnKind::Plain => write!(ctx.out, "{ret}")?,
};
// Generating the function signature is done.

let last_expr = if let Some(EvalStep {
check: ControlFlow::Return { .. },
Expand Down Expand Up @@ -530,6 +558,21 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
Nested::Cases(block.steps.iter())
}

fn block_weight(block: &Block) -> usize {
fn cf_weight(cf: &ControlFlow) -> usize {
match cf {
ControlFlow::Match { arms, .. } => {
arms.iter().map(|a| Codegen::block_weight(&a.body)).sum()
}
ControlFlow::Equal { body, .. } => Codegen::block_weight(body),
ControlFlow::Loop { body, .. } => Codegen::block_weight(body),
ControlFlow::Return { .. } => 0,
}
}

block.steps.iter().map(|s| 1 + cf_weight(&s.check)).sum()
}

fn emit_block<W: Write>(
&self,
ctx: &mut BodyContext<W>,
Expand All @@ -538,8 +581,19 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
last_expr: &str,
scope: StableSet<BindingId>,
) -> std::fmt::Result {
let mut stack = Vec::new();
ctx.begin_block()?;
self.emit_block_contents(ctx, block, ret_kind, last_expr, scope)
}

fn emit_block_contents<W: Write>(
&self,
ctx: &mut BodyContext<W>,
block: &Block,
ret_kind: ReturnKind,
last_expr: &str,
scope: StableSet<BindingId>,
) -> std::fmt::Result {
let mut stack = Vec::new();
stack.push((Self::validate_block(ret_kind, block), last_expr, scope));

while let Some((mut nested, last_line, scope)) = stack.pop() {
Expand Down Expand Up @@ -706,8 +760,8 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
writeln!(ctx.out, "));")?;
writeln!(
ctx.out,
"{}if returns.len() >= MAX_ISLE_RETURNS {{ return; }}",
ctx.indent
"{}if returns.len() >= MAX_ISLE_RETURNS {{ {} }}",
ctx.indent, ctx.iter_overflow_action
)?;
}
}
Expand All @@ -729,7 +783,39 @@ impl<L: Length, C> Length for ContextIterWrapper<L, C> {{
self.emit_constraint(ctx, source, arm)?;
write!(ctx.out, " =>")?;
ctx.begin_block()?;
stack.push((Self::validate_block(ret_kind, &arm.body), "", scope));

// Compile-time optimization: huge function bodies (often from very large match arms
// of constructor bodies)cause rustc to spend a lot of time in analysis passes.
// Wrap such bodies in a local closure to move the bulk of the work into a separate body
// without needing to know the types of captured locals.
const MATCH_ARM_BODY_CLOSURE_THRESHOLD: usize = 256;
if ret_kind == ReturnKind::Iterator
&& Codegen::block_weight(&arm.body) > MATCH_ARM_BODY_CLOSURE_THRESHOLD
Comment on lines +787 to +793
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding my previous comment: it could potentially make sense to make MATCH_ARM_BODY_CLOSURE_THRESHOLD a dynamic ISLE compilation option, rather than a constant, and then tweak that value in Cranelift's invocation of the ISLE compiler depending on if a cargo feature is enabled or whether this is a release vs debug build of Cranelift or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I experimented three values for this: 128, 256, 512.
The best one was 256 for the current ruleset (~500 rules), and 512 for my massive ruleset (~6,700 rules).
There could be some relation between threshold value and the ruleset size. Additionally, the parallelism available of the development environment could affect too.

However, as this could affect the quality of the compiled cranelift, I assumed at first sticking to the best value was a good idea.

{
let closure_id = ctx.match_split;
ctx.match_split += 1;

write!(ctx.out, "{}if (|| -> bool", &ctx.indent)?;
ctx.begin_block()?;

let old_overflow_action = ctx.iter_overflow_action;
ctx.iter_overflow_action = "return true;";
let closure_scope = ctx.enter_scope();
self.emit_block_contents(ctx, &arm.body, ret_kind, "false", closure_scope)?;
ctx.iter_overflow_action = old_overflow_action;

// Close `if (|| -> bool { ... })()` and stop the outer function on
// iterator-overflow.
writeln!(
ctx.out,
"{})() {{ {} }} // __isle_arm_{}",
&ctx.indent, ctx.iter_overflow_action, closure_id
)?;

ctx.end_block("", scope)?;
} else {
stack.push((Self::validate_block(ret_kind, &arm.body), "", scope));
}
}
}
}
Expand Down