From 6bb9301848c45a3abc7cca75cf123d1c9233b216 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 21 Mar 2024 12:17:36 +0100 Subject: [PATCH 01/11] Add Instruction::Jump, ::JumpConditionally --- compiler/vm/src/byte_code.rs | 21 +++++++++++++++++++++ compiler/vm/src/instructions.rs | 15 +++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/compiler/vm/src/byte_code.rs b/compiler/vm/src/byte_code.rs index 1e6cc4347..e967dbfac 100644 --- a/compiler/vm/src/byte_code.rs +++ b/compiler/vm/src/byte_code.rs @@ -103,6 +103,17 @@ pub enum Instruction { /// called. Return, + /// A more efficient alternative to `Call` that doesn't need a function + /// allocation. + /// + /// Does not modify the stack. + Jump { target: InstructionPointer }, + + /// Similar to `Jump`, but only jumps if the top of the stack is `True`. + /// + /// a, condition -> a + JumpConditionally { target: InstructionPointer }, + /// Panics. Because the panic instruction only occurs inside the generated /// needs function, the reason is already guaranteed to be a text. /// @@ -185,6 +196,12 @@ impl Instruction { // Only modifies the call stack and the instruction pointer. // Leaves the return value untouched on the stack. } + Self::Jump { .. } => { + // Does not modify the stack. + } + Self::JumpConditionally { .. } => { + stack.pop(); // condition + } Self::Panic => { stack.pop(); // responsible stack.pop(); // reason @@ -392,6 +409,10 @@ impl Instruction { ); } Self::Return => {} + Self::Jump { target } | Self::JumpConditionally { target } => { + builder.push(" to ", None, EnumSet::empty()); + builder.push(format!("{target:?}"), None, EnumSet::empty()); + } Self::Panic => {} Self::TraceCallStarts { num_args } | Self::TraceTailCall { num_args } => { builder.push( diff --git a/compiler/vm/src/instructions.rs b/compiler/vm/src/instructions.rs index 688826592..0781f1a6f 100644 --- a/compiler/vm/src/instructions.rs +++ b/compiler/vm/src/instructions.rs @@ -147,6 +147,21 @@ impl MachineState { self.next_instruction = self.call_stack.pop(); InstructionResult::Done } + Instruction::Jump { target } => { + self.next_instruction = Some(*target); + InstructionResult::Done + } + Instruction::JumpConditionally { target } => { + let condition = self.pop_from_data_stack(); + let condition = Tag::try_from(condition) + .unwrap() + .try_into_bool(heap) + .unwrap(); + if condition { + self.next_instruction = Some(*target); + } + InstructionResult::Done + } Instruction::Panic => { let responsible_for_panic = self.pop_from_data_stack(); let reason = self.pop_from_data_stack(); From 4621536c014c1043e0813f47dff27b70d138d6b3 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 14 Mar 2024 22:06:54 +0100 Subject: [PATCH 02/11] Add .direnv/ to .gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index c9dffa03e..2d258c924 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,9 @@ callgrind.out.* # ANTLR .antlr/ +# direnv +.direnv/ + # Rust target From cb20733c9164ffe7f583dc1c8d47903b5e766fb2 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 21 Mar 2024 13:50:25 +0100 Subject: [PATCH 03/11] Add lir::Expression::Jump, ::JumpConditionally --- compiler/frontend/src/lir/expression.rs | 25 ++++++++++ compiler/vm/src/lir_to_byte_code.rs | 64 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/compiler/frontend/src/lir/expression.rs b/compiler/frontend/src/lir/expression.rs index af392e81e..17dc3a275 100644 --- a/compiler/frontend/src/lir/expression.rs +++ b/compiler/frontend/src/lir/expression.rs @@ -48,6 +48,14 @@ pub enum Expression { responsible: Id, }, + Jump { + target: Id, + }, + JumpConditionally { + target: Id, + condition: Id, + }, + Panic { reason: Id, responsible: Id, @@ -128,6 +136,13 @@ impl Expression { } *responsible = replacer(*responsible); } + Self::Jump { target } => { + *target = replacer(*target); + } + Self::JumpConditionally { target, condition } => { + *target = replacer(*target); + *condition = replacer(*condition); + } Self::Panic { reason, responsible, @@ -266,6 +281,16 @@ impl Expression { responsible.build_rich_ir_with_constants(builder, constants, body); builder.push(" is responsible)", None, EnumSet::empty()); } + Self::Jump { target } => { + builder.push("jump to ", None, EnumSet::empty()); + target.build_rich_ir_with_constants(builder, constants, body); + } + Self::JumpConditionally { target, condition } => { + builder.push("jump to ", None, EnumSet::empty()); + target.build_rich_ir_with_constants(builder, constants, body); + builder.push(" if ", None, EnumSet::empty()); + condition.build_rich_ir_with_constants(builder, constants, body); + } Self::Panic { reason, responsible, diff --git a/compiler/vm/src/lir_to_byte_code.rs b/compiler/vm/src/lir_to_byte_code.rs index 375a07390..775d8485f 100644 --- a/compiler/vm/src/lir_to_byte_code.rs +++ b/compiler/vm/src/lir_to_byte_code.rs @@ -68,6 +68,19 @@ struct LoweringContext<'c> { body_mapping: FxHashMap, stack: Vec, instructions: Vec, + pending_jump_targets: Vec, +} +struct PendingJumpTarget { + /// Index of the jump instruction in the current body's instructions. + jump_index: usize, + + /// ID of the target expression. + target_id: Id, + + /// Index of the jump target in the current body's instructions. + /// + /// This is `None` until we're lowering the target. + target_index: Option, } impl<'c> LoweringContext<'c> { fn compile(module: Module, lir: &Lir) -> ByteCode { @@ -97,6 +110,7 @@ impl<'c> LoweringContext<'c> { body_mapping: FxHashMap::default(), stack: vec![], instructions: vec![], + pending_jump_targets: vec![], }; let mut start = None; for (id, _) in lir.bodies().ids_and_bodies() { @@ -113,6 +127,7 @@ impl<'c> LoweringContext<'c> { fn compile_body(&mut self, body_id: BodyId) -> InstructionPointer { let old_stack = mem::take(&mut self.stack); let old_instructions = mem::take(&mut self.instructions); + let old_pending_jump_targets = mem::take(&mut self.pending_jump_targets); let body = self.lir.bodies().get(body_id); for captured in body.captured_ids() { @@ -144,6 +159,20 @@ impl<'c> LoweringContext<'c> { self.emit(dummy_id, Instruction::Return); } + for pending_jump_target in &self.pending_jump_targets { + let target_index = pending_jump_target + .target_index + .expect("Jump target not lowered."); + let target = InstructionPointer::from(self.instructions.len() + target_index); + self.instructions[pending_jump_target.jump_index] = match &self.instructions + [pending_jump_target.jump_index] + { + Instruction::Jump { .. } => Instruction::Jump { target }, + Instruction::JumpConditionally { .. } => Instruction::JumpConditionally { target }, + _ => unreachable!(), + }; + } + let num_instructions = self.instructions.len(); let start = self.byte_code.instructions.len().into(); self.byte_code.instructions.append(&mut self.instructions); @@ -154,11 +183,19 @@ impl<'c> LoweringContext<'c> { self.stack = old_stack; self.instructions = old_instructions; + self.pending_jump_targets = old_pending_jump_targets; start } fn compile_expression(&mut self, id: Id, expression: &Expression) { + for pending_jump_target in &mut self.pending_jump_targets { + if pending_jump_target.target_id == id { + assert_eq!(pending_jump_target.target_index, None); + pending_jump_target.target_index = Some(self.instructions.len()); + } + } + match expression { Expression::CreateTag { symbol, value } => { let symbol = self @@ -235,6 +272,33 @@ impl<'c> LoweringContext<'c> { }, ); } + Expression::Jump { target } => { + self.pending_jump_targets.push(PendingJumpTarget { + jump_index: self.instructions.len(), + target_id: *target, + target_index: None, + }); + self.emit( + id, + Instruction::Jump { + target: InstructionPointer::null_pointer(), + }, + ); + } + Expression::JumpConditionally { target, condition } => { + self.emit_reference_to(*condition); + self.pending_jump_targets.push(PendingJumpTarget { + jump_index: self.instructions.len(), + target_id: *target, + target_index: None, + }); + self.emit( + id, + Instruction::JumpConditionally { + target: InstructionPointer::null_pointer(), + }, + ); + } Expression::Panic { reason, responsible, From 9951dd421ea2bd27788bfcc31f96dc12dd9ade1f Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 17:34:30 +0100 Subject: [PATCH 04/11] Simplify Jump, JumpConditionally instructions to IfElse --- compiler/frontend/src/lir/expression.rs | 71 ++++++++++++++++------ compiler/vm/src/byte_code.rs | 50 ++++++++++----- compiler/vm/src/instructions.rs | 28 ++++++--- compiler/vm/src/lir_to_byte_code.rs | 81 +++++++------------------ 4 files changed, 131 insertions(+), 99 deletions(-) diff --git a/compiler/frontend/src/lir/expression.rs b/compiler/frontend/src/lir/expression.rs index 17dc3a275..3f55d0337 100644 --- a/compiler/frontend/src/lir/expression.rs +++ b/compiler/frontend/src/lir/expression.rs @@ -48,12 +48,13 @@ pub enum Expression { responsible: Id, }, - Jump { - target: Id, - }, - JumpConditionally { - target: Id, + IfElse { condition: Id, + then_body_id: BodyId, + then_captured: Vec, + else_body_id: BodyId, + else_captured: Vec, + responsible: Id, }, Panic { @@ -136,12 +137,22 @@ impl Expression { } *responsible = replacer(*responsible); } - Self::Jump { target } => { - *target = replacer(*target); - } - Self::JumpConditionally { target, condition } => { - *target = replacer(*target); + Self::IfElse { + condition, + then_body_id: _, + then_captured, + else_body_id: _, + else_captured, + responsible, + } => { *condition = replacer(*condition); + for captured in then_captured { + *captured = replacer(*captured); + } + for captured in else_captured { + *captured = replacer(*captured); + } + *responsible = replacer(*responsible); } Self::Panic { reason, @@ -281,15 +292,39 @@ impl Expression { responsible.build_rich_ir_with_constants(builder, constants, body); builder.push(" is responsible)", None, EnumSet::empty()); } - Self::Jump { target } => { - builder.push("jump to ", None, EnumSet::empty()); - target.build_rich_ir_with_constants(builder, constants, body); - } - Self::JumpConditionally { target, condition } => { - builder.push("jump to ", None, EnumSet::empty()); - target.build_rich_ir_with_constants(builder, constants, body); - builder.push(" if ", None, EnumSet::empty()); + Self::IfElse { + condition, + then_body_id, + then_captured, + else_body_id, + else_captured, + responsible, + } => { + builder.push("if ", None, EnumSet::empty()); condition.build_rich_ir_with_constants(builder, constants, body); + builder.push(" then call ", None, EnumSet::empty()); + then_body_id.build_rich_ir(builder); + if !then_captured.is_empty() { + builder.push(" capturing ", None, EnumSet::empty()); + builder.push_children_custom( + then_captured, + |builder, it| it.build_rich_ir_with_constants(builder, constants, body), + ", ", + ); + } + builder.push(" else call ", None, EnumSet::empty()); + else_body_id.build_rich_ir(builder); + if !else_captured.is_empty() { + builder.push(" capturing ", None, EnumSet::empty()); + builder.push_children_custom( + else_captured, + |builder, it| it.build_rich_ir_with_constants(builder, constants, body), + ", ", + ); + } + builder.push(" (", None, EnumSet::empty()); + responsible.build_rich_ir_with_constants(builder, constants, body); + builder.push(" is responsible)", None, EnumSet::empty()); } Self::Panic { reason, diff --git a/compiler/vm/src/byte_code.rs b/compiler/vm/src/byte_code.rs index e967dbfac..7218913fd 100644 --- a/compiler/vm/src/byte_code.rs +++ b/compiler/vm/src/byte_code.rs @@ -103,16 +103,16 @@ pub enum Instruction { /// called. Return, - /// A more efficient alternative to `Call` that doesn't need a function - /// allocation. + /// Conditionally calls either `then_target` or `else_target` depending on + /// the `condition`. /// - /// Does not modify the stack. - Jump { target: InstructionPointer }, - - /// Similar to `Jump`, but only jumps if the top of the stack is `True`. - /// - /// a, condition -> a - JumpConditionally { target: InstructionPointer }, + /// a, condition, responsible -> a + IfElse { + then_target: InstructionPointer, + then_captured: Vec, + else_target: InstructionPointer, + else_captured: Vec, + }, /// Panics. Because the panic instruction only occurs inside the generated /// needs function, the reason is already guaranteed to be a text. @@ -196,10 +196,8 @@ impl Instruction { // Only modifies the call stack and the instruction pointer. // Leaves the return value untouched on the stack. } - Self::Jump { .. } => { - // Does not modify the stack. - } - Self::JumpConditionally { .. } => { + Self::IfElse { .. } => { + stack.pop(); // responsible stack.pop(); // condition } Self::Panic => { @@ -409,9 +407,29 @@ impl Instruction { ); } Self::Return => {} - Self::Jump { target } | Self::JumpConditionally { target } => { - builder.push(" to ", None, EnumSet::empty()); - builder.push(format!("{target:?}"), None, EnumSet::empty()); + Self::IfElse { + then_target, + then_captured, + else_target, + else_captured, + } => { + builder.push( + format!( + " then call {then_target:?} capturing {} else call {else_target:?} capturing {}", + if then_captured.is_empty() { + "nothing".to_string() + } else { + then_captured.iter().join(", ") + }, + if else_captured.is_empty() { + "nothing".to_string() + } else { + else_captured.iter().join(", ") + }, + ), + None, + EnumSet::empty(), + ); } Self::Panic => {} Self::TraceCallStarts { num_args } | Self::TraceTailCall { num_args } => { diff --git a/compiler/vm/src/instructions.rs b/compiler/vm/src/instructions.rs index 0781f1a6f..da6396134 100644 --- a/compiler/vm/src/instructions.rs +++ b/compiler/vm/src/instructions.rs @@ -147,19 +147,33 @@ impl MachineState { self.next_instruction = self.call_stack.pop(); InstructionResult::Done } - Instruction::Jump { target } => { - self.next_instruction = Some(*target); - InstructionResult::Done - } - Instruction::JumpConditionally { target } => { + Instruction::IfElse { + then_target, + then_captured, + else_target, + else_captured, + } => { + let responsible = self.pop_from_data_stack(); let condition = self.pop_from_data_stack(); let condition = Tag::try_from(condition) .unwrap() .try_into_bool(heap) .unwrap(); - if condition { - self.next_instruction = Some(*target); + let (target, captured) = if condition { + (*then_target, then_captured) + } else { + (*else_target, else_captured) + }; + + if let Some(next_instruction) = self.next_instruction { + self.call_stack.push(next_instruction); + } + for offset in captured { + let captured = self.get_from_data_stack(*offset); + self.data_stack.push(captured); } + self.push_to_data_stack(responsible); + self.next_instruction = Some(target); InstructionResult::Done } Instruction::Panic => { diff --git a/compiler/vm/src/lir_to_byte_code.rs b/compiler/vm/src/lir_to_byte_code.rs index 775d8485f..e12f78741 100644 --- a/compiler/vm/src/lir_to_byte_code.rs +++ b/compiler/vm/src/lir_to_byte_code.rs @@ -68,19 +68,6 @@ struct LoweringContext<'c> { body_mapping: FxHashMap, stack: Vec, instructions: Vec, - pending_jump_targets: Vec, -} -struct PendingJumpTarget { - /// Index of the jump instruction in the current body's instructions. - jump_index: usize, - - /// ID of the target expression. - target_id: Id, - - /// Index of the jump target in the current body's instructions. - /// - /// This is `None` until we're lowering the target. - target_index: Option, } impl<'c> LoweringContext<'c> { fn compile(module: Module, lir: &Lir) -> ByteCode { @@ -110,7 +97,6 @@ impl<'c> LoweringContext<'c> { body_mapping: FxHashMap::default(), stack: vec![], instructions: vec![], - pending_jump_targets: vec![], }; let mut start = None; for (id, _) in lir.bodies().ids_and_bodies() { @@ -127,7 +113,6 @@ impl<'c> LoweringContext<'c> { fn compile_body(&mut self, body_id: BodyId) -> InstructionPointer { let old_stack = mem::take(&mut self.stack); let old_instructions = mem::take(&mut self.instructions); - let old_pending_jump_targets = mem::take(&mut self.pending_jump_targets); let body = self.lir.bodies().get(body_id); for captured in body.captured_ids() { @@ -159,20 +144,6 @@ impl<'c> LoweringContext<'c> { self.emit(dummy_id, Instruction::Return); } - for pending_jump_target in &self.pending_jump_targets { - let target_index = pending_jump_target - .target_index - .expect("Jump target not lowered."); - let target = InstructionPointer::from(self.instructions.len() + target_index); - self.instructions[pending_jump_target.jump_index] = match &self.instructions - [pending_jump_target.jump_index] - { - Instruction::Jump { .. } => Instruction::Jump { target }, - Instruction::JumpConditionally { .. } => Instruction::JumpConditionally { target }, - _ => unreachable!(), - }; - } - let num_instructions = self.instructions.len(); let start = self.byte_code.instructions.len().into(); self.byte_code.instructions.append(&mut self.instructions); @@ -183,19 +154,11 @@ impl<'c> LoweringContext<'c> { self.stack = old_stack; self.instructions = old_instructions; - self.pending_jump_targets = old_pending_jump_targets; start } fn compile_expression(&mut self, id: Id, expression: &Expression) { - for pending_jump_target in &mut self.pending_jump_targets { - if pending_jump_target.target_id == id { - assert_eq!(pending_jump_target.target_index, None); - pending_jump_target.target_index = Some(self.instructions.len()); - } - } - match expression { Expression::CreateTag { symbol, value } => { let symbol = self @@ -235,6 +198,7 @@ impl<'c> LoweringContext<'c> { } Expression::CreateFunction { captured, body_id } => { let instruction_pointer = self.get_body(*body_id); + // PERF: Do we need to emit these references if we store stack offsets anyway? for captured in captured { self.emit_reference_to(*captured); } @@ -272,30 +236,31 @@ impl<'c> LoweringContext<'c> { }, ); } - Expression::Jump { target } => { - self.pending_jump_targets.push(PendingJumpTarget { - jump_index: self.instructions.len(), - target_id: *target, - target_index: None, - }); - self.emit( - id, - Instruction::Jump { - target: InstructionPointer::null_pointer(), - }, - ); - } - Expression::JumpConditionally { target, condition } => { + Expression::IfElse { + condition, + then_body_id, + then_captured, + else_body_id, + else_captured, + responsible, + } => { self.emit_reference_to(*condition); - self.pending_jump_targets.push(PendingJumpTarget { - jump_index: self.instructions.len(), - target_id: *target, - target_index: None, - }); + self.emit_reference_to(*responsible); + let then_target = self.get_body(*then_body_id); + let else_target = self.get_body(*else_body_id); self.emit( id, - Instruction::JumpConditionally { - target: InstructionPointer::null_pointer(), + Instruction::IfElse { + then_target, + then_captured: then_captured + .iter() + .map(|id| self.stack.find_id(*id)) + .collect(), + else_target, + else_captured: else_captured + .iter() + .map(|id| self.stack.find_id(*id)) + .collect(), }, ); } From c67cf1c12752202b469e3e74183c44f4e1b243bc Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 21:14:26 +0100 Subject: [PATCH 05/11] Add hyperfine --- flake.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/flake.nix b/flake.nix index d0bd42393..c5f5674d5 100644 --- a/flake.nix +++ b/flake.nix @@ -33,6 +33,7 @@ RUSTFLAGS = "-C link-arg=-fuse-ld=mold"; buildInputs = [ cargo-insta + hyperfine libffi llvmPackages_15.bintools llvmPackages_15.clangUseLLVM From aa7caa8e46ed4b2f52d90c2316511e1d101930f5 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 21:14:48 +0100 Subject: [PATCH 06/11] Fix --trace-calls --- compiler/cli/src/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/cli/src/debug.rs b/compiler/cli/src/debug.rs index 5ff54dd14..25375d993 100644 --- a/compiler/cli/src/debug.rs +++ b/compiler/cli/src/debug.rs @@ -110,7 +110,7 @@ pub struct PathAndExecutionTargetAndTracing { #[arg( long, default_value("off"), - default_missing_value("only-potentially-panicking"), + default_missing_value("only-for-panic-traces"), num_args(0..=1), require_equals(true) )] From 52301f650786684d8d98191d715fe3cc03ff8b43 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 21:15:53 +0100 Subject: [PATCH 07/11] Detect ifElse calls and optimize them --- compiler/frontend/src/mir_to_lir.rs | 54 ++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/compiler/frontend/src/mir_to_lir.rs b/compiler/frontend/src/mir_to_lir.rs index 82ab61d65..e0e0c1eb7 100644 --- a/compiler/frontend/src/mir_to_lir.rs +++ b/compiler/frontend/src/mir_to_lir.rs @@ -1,10 +1,11 @@ use crate::{ + builtin_functions::BuiltinFunction, error::CompilerError, - hir::{self}, + hir, hir_to_mir::ExecutionTarget, id::CountableId, lir::{self, Lir}, - mir::{self}, + mir, mir_optimize::OptimizeMir, string_to_rcst::ModuleError, utils::{HashMapExtension, HashSetExtension}, @@ -38,11 +39,17 @@ fn lir(db: &dyn MirToLir, target: ExecutionTarget, tracing: TracingConfig) -> Li Ok((Arc::new(lir), errors)) } -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] struct LoweringContext { constants: lir::Constants, constant_mapping: FxHashMap, bodies: lir::Bodies, + potential_if_else_bodies: FxHashMap, +} +#[derive(Debug)] +struct PotentialIfElseBody { + body_id: lir::BodyId, + captured: Vec, } impl LoweringContext { fn constant_for(&self, id: mir::Id) -> Option { @@ -247,10 +254,22 @@ impl CurrentBody { *responsible_parameter, body, ); + + let captured = self.ids_for(context, &captured); + + if parameters.is_empty() { + context.potential_if_else_bodies.force_insert( + id, + PotentialIfElseBody { + body_id, + captured: captured.clone(), + }, + ); + } + if captured.is_empty() { self.push_constant(context, id, body_id); } else { - let captured = self.ids_for(context, &captured); self.push(id, lir::Expression::CreateFunction { captured, body_id }); } } @@ -260,6 +279,26 @@ impl CurrentBody { arguments, responsible, } => { + if Self::is_builtin_if_else(context, *function) + && let Some(then_body) = context.potential_if_else_bodies.get(&arguments[0]) + && let Some(else_body) = context.potential_if_else_bodies.get(&arguments[1]) + { + let condition = self.id_for(context, arguments[2]); + let responsible = self.id_for_without_dup(context, *responsible); + self.push( + id, + lir::Expression::IfElse { + condition, + then_body_id: then_body.body_id, + then_captured: then_body.captured.clone(), + else_body_id: else_body.body_id, + else_captured: else_body.captured.clone(), + responsible, + }, + ); + return; + } + let function = self.id_for(context, *function); let arguments = self.ids_for(context, arguments); let responsible = self.id_for_without_dup(context, *responsible); @@ -381,6 +420,13 @@ impl CurrentBody { self.push(id, context.constant_for(id).unwrap()) } + #[must_use] + fn is_builtin_if_else(context: &LoweringContext, id: mir::Id) -> bool { + matches!( + context.constant_for(id), + Some(constant_id) if context.constants.get(constant_id) == &lir::Constant::Builtin(BuiltinFunction::IfElse), + ) + } fn push_constant( &mut self, From d419ceb5785bb2a1bcbb2b781adbd98db3351803 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 21:24:25 +0100 Subject: [PATCH 08/11] Fix formatting --- compiler/vm/src/byte_code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/vm/src/byte_code.rs b/compiler/vm/src/byte_code.rs index 7218913fd..58e6c10fc 100644 --- a/compiler/vm/src/byte_code.rs +++ b/compiler/vm/src/byte_code.rs @@ -426,7 +426,7 @@ impl Instruction { } else { else_captured.iter().join(", ") }, - ), + ), None, EnumSet::empty(), ); From 68a61fc874d6555b1a40ad91c3fbec69b5575ad4 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 23:25:25 +0100 Subject: [PATCH 09/11] Fix data stack in ifElse instruction --- compiler/vm/src/byte_code.rs | 1 + compiler/vm/src/instructions.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/vm/src/byte_code.rs b/compiler/vm/src/byte_code.rs index 58e6c10fc..c7637e3db 100644 --- a/compiler/vm/src/byte_code.rs +++ b/compiler/vm/src/byte_code.rs @@ -199,6 +199,7 @@ impl Instruction { Self::IfElse { .. } => { stack.pop(); // responsible stack.pop(); // condition + stack.push(result); // return value } Self::Panic => { stack.pop(); // responsible diff --git a/compiler/vm/src/instructions.rs b/compiler/vm/src/instructions.rs index da6396134..e512a6041 100644 --- a/compiler/vm/src/instructions.rs +++ b/compiler/vm/src/instructions.rs @@ -168,8 +168,12 @@ impl MachineState { if let Some(next_instruction) = self.next_instruction { self.call_stack.push(next_instruction); } - for offset in captured { - let captured = self.get_from_data_stack(*offset); + + // Initially, we need to adjust the offset because we already + // popped two values from the data stack. Afterwards, increment + // it for each value. + for (index, offset) in captured.iter().enumerate() { + let captured = self.get_from_data_stack(*offset - 2 + index); self.data_stack.push(captured); } self.push_to_data_stack(responsible); From 38e470da7ded425df29c9993919cc12199121c64 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 23:25:40 +0100 Subject: [PATCH 10/11] Add empty line between instruction traces --- compiler/vm/src/instructions.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/vm/src/instructions.rs b/compiler/vm/src/instructions.rs index e512a6041..afceded5a 100644 --- a/compiler/vm/src/instructions.rs +++ b/compiler/vm/src/instructions.rs @@ -23,6 +23,7 @@ impl MachineState { tracer: &mut impl Tracer, ) -> InstructionResult { if TRACE { + trace!(""); trace!("Running instruction: {instruction:?}"); trace!("Instruction pointer: {:?}", self.next_instruction.unwrap()); trace!( From 1f1e1eb822fe0cd8545b8074263102744e513e91 Mon Sep 17 00:00:00 2001 From: Jonas Wanke Date: Thu, 28 Mar 2024 23:27:14 +0100 Subject: [PATCH 11/11] Use ifElse expression for all then/else values --- compiler/frontend/src/mir_to_lir.rs | 104 ++++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 14 deletions(-) diff --git a/compiler/frontend/src/mir_to_lir.rs b/compiler/frontend/src/mir_to_lir.rs index e0e0c1eb7..40ed78bca 100644 --- a/compiler/frontend/src/mir_to_lir.rs +++ b/compiler/frontend/src/mir_to_lir.rs @@ -44,10 +44,11 @@ struct LoweringContext { constants: lir::Constants, constant_mapping: FxHashMap, bodies: lir::Bodies, - potential_if_else_bodies: FxHashMap, + potential_if_else_bodies: FxHashMap, + delegating_if_else_body_id: Option, } -#[derive(Debug)] -struct PotentialIfElseBody { +#[derive(Clone, Debug)] +struct IfElseBody { body_id: lir::BodyId, captured: Vec, } @@ -74,6 +75,41 @@ impl LoweringContext { ); self.bodies.push(body) } + + /// Creates a body capturing a function and then executing it. + /// + /// It's used for ifElse calls for which at least one of the branches' + /// functions is not exclusively used for ifElse bodies. + #[must_use] + fn delegating_if_else_body_id(&mut self) -> lir::BodyId { + self.delegating_if_else_body_id.unwrap_or_else(|| { + let target_function_id = mir::Id::from_usize(0); + let responsible_parameter_id = mir::Id::from_usize(1); + let call_id = mir::Id::from_usize(2); + + let mut current_body = CurrentBody::new( + FxHashSet::default(), + &[target_function_id], + &[], + responsible_parameter_id, + ); + let function = *current_body.id_mapping.get(&target_function_id).unwrap(); + let responsible = *current_body + .id_mapping + .get(&responsible_parameter_id) + .unwrap(); + current_body.push( + call_id, + lir::Expression::Call { + function, + arguments: vec![], + responsible, + }, + ); + let body = current_body.finish(&self.constant_mapping); + self.bodies.push(body) + }) + } } #[derive(Clone, Debug)] @@ -93,9 +129,9 @@ impl CurrentBody { body: &mir::Body, ) -> lir::Body { let mut lir_body = Self::new(original_hirs, captured, parameters, responsible_parameter); - for (id, expression) in body.iter() { + for (index, (id, expression)) in body.iter().enumerate() { lir_body.current_constant = None; - lir_body.compile_expression(context, id, expression); + lir_body.compile_expression(context, id, expression, &body.expressions[index + 1..]); } lir_body.finish(&context.constant_mapping) } @@ -147,6 +183,7 @@ impl CurrentBody { context: &mut LoweringContext, id: mir::Id, expression: &mir::Expression, + remaining_expressions: &[(mir::Id, mir::Expression)], ) { match expression { mir::Expression::Int(int) => self.push_constant(context, id, int.clone()), @@ -260,14 +297,35 @@ impl CurrentBody { if parameters.is_empty() { context.potential_if_else_bodies.force_insert( id, - PotentialIfElseBody { + IfElseBody { body_id, captured: captured.clone(), }, ); } - if captured.is_empty() { + if parameters.is_empty() + && remaining_expressions.iter().all(|(_, expression)| { + if let mir::Expression::Call { + function, + arguments, + .. + } = expression + && Self::is_builtin_if_else(context, *function) + && arguments[0] != id + { + // Call to the ifElse builtin function + true + } else { + // The expression doesn't reference the current body + !expression.referenced_ids().contains(&id) + } + }) + { + // If the function takes no parameters and is only used in + // calls to the ifElse builtin function, we refer to it in + // the special ifElse LIR expression. + } else if captured.is_empty() { self.push_constant(context, id, body_id); } else { self.push(id, lir::Expression::CreateFunction { captured, body_id }); @@ -279,20 +337,38 @@ impl CurrentBody { arguments, responsible, } => { - if Self::is_builtin_if_else(context, *function) - && let Some(then_body) = context.potential_if_else_bodies.get(&arguments[0]) - && let Some(else_body) = context.potential_if_else_bodies.get(&arguments[1]) - { - let condition = self.id_for(context, arguments[2]); + if Self::is_builtin_if_else(context, *function) { + let [condition, then_body, else_body] = arguments.as_slice() else { + unreachable!() + }; + + let condition = self.id_for(context, *condition); + let then_body = context + .potential_if_else_bodies + .get(then_body) + .cloned() + .unwrap_or_else(|| IfElseBody { + body_id: context.delegating_if_else_body_id(), + captured: vec![self.id_for(context, *then_body)], + }); + let else_body = context + .potential_if_else_bodies + .get(else_body) + .cloned() + .unwrap_or_else(|| IfElseBody { + body_id: context.delegating_if_else_body_id(), + captured: vec![self.id_for(context, *else_body)], + }); let responsible = self.id_for_without_dup(context, *responsible); + self.push( id, lir::Expression::IfElse { condition, then_body_id: then_body.body_id, - then_captured: then_body.captured.clone(), + then_captured: then_body.captured, else_body_id: else_body.body_id, - else_captured: else_body.captured.clone(), + else_captured: else_body.captured, responsible, }, );