From 3d317ed8e430da8b6e0a714ae78726cec8cfbf56 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 26 Jul 2024 20:08:04 +0530 Subject: [PATCH 01/19] value numbering analysis --- sway-ir/src/instruction.rs | 6 +- sway-ir/src/optimize.rs | 2 + sway-ir/src/optimize/cse.rs | 310 ++++++++++++++++++++++++++++++++++++ sway-ir/src/pass_manager.rs | 13 +- sway-ir/src/printer.rs | 3 +- sway-ir/tests/cse/cse1.ir | 17 ++ sway-ir/tests/cse/cse2.ir | 16 ++ sway-ir/tests/cse/cse3.ir | 22 +++ 8 files changed, 375 insertions(+), 14 deletions(-) create mode 100644 sway-ir/src/optimize/cse.rs create mode 100644 sway-ir/tests/cse/cse1.ir create mode 100644 sway-ir/tests/cse/cse2.ir create mode 100644 sway-ir/tests/cse/cse3.ir diff --git a/sway-ir/src/instruction.rs b/sway-ir/src/instruction.rs index 4932f4359ff..072d4ab1dfd 100644 --- a/sway-ir/src/instruction.rs +++ b/sway-ir/src/instruction.rs @@ -204,19 +204,19 @@ pub enum FuelVmInstruction { } /// Comparison operations. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub enum Predicate { Equal, LessThan, GreaterThan, } -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub enum UnaryOpKind { Not, } -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub enum BinaryOpKind { Add, Sub, diff --git a/sway-ir/src/optimize.rs b/sway-ir/src/optimize.rs index 7aeade50bfd..5b153b18603 100644 --- a/sway-ir/src/optimize.rs +++ b/sway-ir/src/optimize.rs @@ -19,6 +19,8 @@ pub mod const_demotion; pub use const_demotion::*; pub mod constants; pub use constants::*; +pub mod cse; +pub use cse::*; pub mod dce; pub use dce::*; pub mod inline; diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs new file mode 100644 index 00000000000..74532a6b6a0 --- /dev/null +++ b/sway-ir/src/optimize/cse.rs @@ -0,0 +1,310 @@ +//! Value numbering based common subexpression elimination + +use itertools::Itertools; +use rustc_hash::{FxHashMap, FxHasher}; +use slotmap::Key; +use std::{ + collections::hash_map, + fmt::Debug, + hash::{Hash, Hasher}, +}; + +use crate::{ + AnalysisResults, BinaryOpKind, Context, DebugWithContext, Function, InstOp, IrError, Pass, + PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, Value, POSTORDER_NAME, +}; + +pub const CSE_NAME: &str = "cse"; + +pub fn create_cse_pass() -> Pass { + Pass { + name: CSE_NAME, + descr: "Common subexpression elimination", + runner: ScopedPass::FunctionPass(PassMutability::Transform(cse)), + deps: vec![POSTORDER_NAME], + } +} + +#[derive(Clone, Copy, Eq, PartialEq, Hash, DebugWithContext)] +enum ValueNumber { + // Top of the lattice = Don't know = uninitialized + Top, + // Belongs to a congruence class represented by the inner value. + Number(Value), +} + +impl Debug for ValueNumber { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Top => write!(f, "Top"), + Self::Number(arg0) => write!(f, "v{:?}", arg0.0.data()), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Hash, DebugWithContext)] +enum Expr { + Phi(Vec), + UnaryOp { + op: UnaryOpKind, + arg: ValueNumber, + }, + BinaryOp { + op: BinaryOpKind, + arg1: ValueNumber, + arg2: ValueNumber, + }, + BitCast(ValueNumber, Type), + CastPtr(ValueNumber, Type), + Cmp(Predicate, ValueNumber, ValueNumber), + GetElemPtr { + base: ValueNumber, + elem_ptr_ty: Type, + indices: Vec, + }, + IntToPtr(ValueNumber, Type), + PtrToInt(ValueNumber, Type), +} + +/// Convert an instruction to an expression for hashing +/// Instructions that we don't handle will have their value numbers be equal to themselves. +fn instr_to_expr(context: &Context, vntable: &VNTable, instr: Value) -> Option { + match &instr.get_instruction(context).unwrap().op { + InstOp::AsmBlock(_, _) => None, + InstOp::UnaryOp { op, arg } => Some(Expr::UnaryOp { + op: *op, + arg: vntable.value_map.get(arg).cloned().unwrap(), + }), + InstOp::BinaryOp { op, arg1, arg2 } => Some(Expr::BinaryOp { + op: *op, + arg1: vntable.value_map.get(arg1).cloned().unwrap(), + arg2: vntable.value_map.get(arg2).cloned().unwrap(), + }), + InstOp::BitCast(val, ty) => Some(Expr::BitCast( + vntable.value_map.get(val).cloned().unwrap(), + *ty, + )), + InstOp::Branch(_) => None, + InstOp::Call(_, _) => None, + InstOp::CastPtr(val, ty) => Some(Expr::CastPtr( + vntable.value_map.get(val).cloned().unwrap(), + *ty, + )), + InstOp::Cmp(pred, val1, val2) => Some(Expr::Cmp( + *pred, + vntable.value_map.get(val1).cloned().unwrap(), + vntable.value_map.get(val2).cloned().unwrap(), + )), + InstOp::ConditionalBranch { .. } => None, + InstOp::ContractCall { .. } => None, + InstOp::FuelVm(_) => todo!(), + InstOp::GetLocal(_) => None, + InstOp::GetConfig(_, _) => None, + InstOp::GetElemPtr { + base, + elem_ptr_ty, + indices, + } => Some(Expr::GetElemPtr { + base: vntable.value_map.get(base).cloned().unwrap(), + elem_ptr_ty: *elem_ptr_ty, + indices: indices + .iter() + .map(|idx| vntable.value_map.get(idx).cloned().unwrap()) + .collect(), + }), + InstOp::IntToPtr(val, ty) => Some(Expr::IntToPtr( + vntable.value_map.get(val).cloned().unwrap(), + *ty, + )), + InstOp::Load(_) => None, + InstOp::MemCopyBytes { .. } => None, + InstOp::MemCopyVal { .. } => None, + InstOp::Nop => None, + InstOp::PtrToInt(val, ty) => Some(Expr::PtrToInt( + vntable.value_map.get(val).cloned().unwrap(), + *ty, + )), + InstOp::Ret(_, _) => None, + InstOp::Store { .. } => None, + } +} + +/// Convert a PHI argument to Expr +fn phi_to_expr(context: &Context, vntable: &VNTable, phi_arg: Value) -> Expr { + let phi_arg = phi_arg.get_argument(context).unwrap(); + let phi_args = phi_arg + .block + .pred_iter(context) + .map(|pred| { + let incoming_val = phi_arg + .get_val_coming_from(context, pred) + .expect("No parameter from predecessor"); + vntable.value_map.get(&incoming_val).cloned().unwrap() + }) + .collect(); + Expr::Phi(phi_args) +} + +#[derive(Default)] +struct VNTable { + value_map: FxHashMap, + expr_map: FxHashMap, +} + +impl Debug for VNTable { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "value_map:\n")?; + for (key, value) in &self.value_map { + write!(f, "\tv{:?} -> {:?}\n", key.0.data(), value)? + } + Ok(()) + } +} + +pub fn cse( + context: &mut Context, + analyses: &AnalysisResults, + function: Function, +) -> Result { + let mut vntable = VNTable::default(); + + // Function arg values map to themselves. + for arg in function.args_iter(context) { + vntable.value_map.insert(arg.1, ValueNumber::Number(arg.1)); + } + + // Initialize all instructions and constants. Constants need special treatmemt. + // They don't have PartialEq implemented. So we need to value number them manually. + // This map maps the hash of a constant value to all possible collisions of it. + let mut const_map = FxHashMap::>::default(); + for (_, inst) in function.instruction_iter(context) { + vntable.value_map.insert(inst, ValueNumber::Top); + for (const_opd_val, const_opd_const) in inst + .get_instruction(context) + .unwrap() + .op + .get_operands() + .iter() + .filter_map(|opd| opd.get_constant(context).map(|copd| (opd, copd))) + { + let mut state = FxHasher::default(); + const_opd_const.hash(&mut state); + let hash = state.finish(); + if let Some(existing_const) = const_map.get(&hash).and_then(|consts| { + consts.iter().find(|val| { + let c = val + .get_constant(context) + .expect("const_map can only contain consts"); + const_opd_const.eq(context, c) + }) + }) { + vntable + .value_map + .insert(*const_opd_val, ValueNumber::Number(*existing_const)); + } else { + const_map + .entry(hash) + .and_modify(|consts| consts.push(*const_opd_val)) + .or_insert_with(|| vec![*const_opd_val]); + vntable + .value_map + .insert(*const_opd_val, ValueNumber::Number(*const_opd_val)); + } + } + } + + // We need to iterate over the blocks in RPO. + let post_order: &PostOrder = analyses.get_analysis_result(function); + + let mut changed = true; + while changed { + changed = false; + // For each block in RPO: + for (block_idx, block) in post_order.po_to_block.iter().rev().enumerate() { + // Process PHIs and then the other instructions. + if block_idx != 0 { + // Entry block arguments are not PHIs. + for (phi, expr_opt) in block + .arg_iter(context) + .map(|arg| (*arg, Some(phi_to_expr(context, &vntable, *arg)))) + .collect_vec() + { + let expr = expr_opt.expect("PHIs must always translate to a valid Expr"); + // We first try to see if PHIs can be simplified into a single value. + let vn = { + let Expr::Phi(ref phi_args) = expr else { + panic!("Expr must be a PHI") + }; + phi_args + .iter() + .map(|vn| Some(*vn)) + .reduce(|vn1, vn2| { + // Here `None` indicates Bottom of the lattice. + if let (Some(vn1), Some(vn2)) = (vn1, vn2) { + match (vn1, vn2) { + (ValueNumber::Top, ValueNumber::Top) => { + Some(ValueNumber::Top) + } + (ValueNumber::Top, ValueNumber::Number(vn)) + | (ValueNumber::Number(vn), ValueNumber::Top) => { + Some(ValueNumber::Number(vn)) + } + (ValueNumber::Number(vn1), ValueNumber::Number(vn2)) => { + (vn1 == vn2).then(|| ValueNumber::Number(vn1)) + } + } + } else { + None + } + }) + .flatten() + // The PHI couldn't be simplifed to a single ValueNumber. + // lookup(expr, x) + .unwrap_or_else(|| match vntable.expr_map.entry(expr) { + hash_map::Entry::Occupied(occ) => *occ.get(), + hash_map::Entry::Vacant(vac) => { + *(vac.insert(ValueNumber::Number(phi))) + } + }) + }; + match vntable.value_map.entry(phi) { + hash_map::Entry::Occupied(occ) if *occ.get() == vn => {} + _ => { + changed = true; + vntable.value_map.insert(phi, vn); + } + } + } + } + + for (inst, expr_opt) in block + .instruction_iter(context) + .map(|instr| (instr, instr_to_expr(context, &vntable, instr))) + .collect_vec() + { + // lookup(expr, x) + let vn = if let Some(expr) = expr_opt { + match vntable.expr_map.entry(expr) { + hash_map::Entry::Occupied(occ) => *occ.get(), + hash_map::Entry::Vacant(vac) => *(vac.insert(ValueNumber::Number(inst))), + } + } else { + // Instructions that always map to their own value number + // (i.e., they can never be equal to some other instruction). + ValueNumber::Number(inst) + }; + match vntable.value_map.entry(inst) { + hash_map::Entry::Occupied(occ) if *occ.get() == vn => {} + _ => { + changed = true; + vntable.value_map.insert(inst, vn); + } + } + } + } + vntable.expr_map.clear(); + } + + dbg!(vntable); + Ok(false) +} diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 3aafda779ad..7c63ae68077 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -1,14 +1,5 @@ use crate::{ - create_arg_demotion_pass, create_const_demotion_pass, create_const_folding_pass, - create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, - create_fn_dce_pass, create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, - create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, - create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, - create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, - IrError, Module, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, DCE_NAME, - FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, - MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, - SROA_NAME, + create_arg_demotion_pass, create_const_demotion_pass, create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME }; use downcast_rs::{impl_downcast, Downcast}; use rustc_hash::FxHashMap; @@ -399,6 +390,7 @@ pub fn register_known_passes(pm: &mut PassManager) { pm.register(create_simplify_cfg_pass()); pm.register(create_fn_dce_pass()); pm.register(create_dce_pass()); + pm.register(create_cse_pass()); pm.register(create_arg_demotion_pass()); pm.register(create_const_demotion_pass()); pm.register(create_ret_demotion_pass()); @@ -417,6 +409,7 @@ pub fn create_o1_pass_group() -> PassGroup { o1.append_pass(FN_DCE_NAME); o1.append_pass(FN_INLINE_NAME); o1.append_pass(CONST_FOLDING_NAME); + o1.append_pass(CSE_NAME); o1.append_pass(SIMPLIFY_CFG_NAME); o1.append_pass(CONST_FOLDING_NAME); o1.append_pass(SIMPLIFY_CFG_NAME); diff --git a/sway-ir/src/printer.rs b/sway-ir/src/printer.rs index 19b90c92861..80f336c8d35 100644 --- a/sway-ir/src/printer.rs +++ b/sway-ir/src/printer.rs @@ -6,6 +6,7 @@ use std::collections::{BTreeMap, HashMap}; +use slotmap::Key; use sway_types::SourceEngine; use crate::{ @@ -1242,7 +1243,7 @@ impl Namer { fn default_name(&mut self, value: &Value) -> String { self.names.get(value).cloned().unwrap_or_else(|| { - let new_name = format!("v{}", self.next_value_idx); + let new_name = format!("v{:?}", value.0.data()); self.next_value_idx += 1; self.names.insert(*value, new_name.clone()); new_name diff --git a/sway-ir/tests/cse/cse1.ir b/sway-ir/tests/cse/cse1.ir new file mode 100644 index 00000000000..4ba901a8b80 --- /dev/null +++ b/sway-ir/tests/cse/cse1.ir @@ -0,0 +1,17 @@ +// regex: ID=[[:alpha:]0-9]+ + +script { + fn main() -> bool { + entry(): + v0 = const u64 11 + v1 = const u64 0 + v3 = add v0, v1 + v4 = add v0, v1 + v10 = const u64 10 + v11 = add v1, v0 + v2 = cmp eq v3 v4 + v3 = cmp eq v11 v3 + v4 = cmp eq v2 v3 + ret bool v4 + } +} diff --git a/sway-ir/tests/cse/cse2.ir b/sway-ir/tests/cse/cse2.ir new file mode 100644 index 00000000000..e268e03903c --- /dev/null +++ b/sway-ir/tests/cse/cse2.ir @@ -0,0 +1,16 @@ +// regex: ID=[[:alpha:]0-9]+ + +script { + fn main() -> bool { + entry(): + v0 = const u64 11 + v0_dup = const u64 11 + v1 = const u64 0 + v3 = add v0, v1 + v4 = add v0, v1 + v5 = sub v0, v3 + v6 = sub v0_dup, v4 + v2 = cmp eq v5 v6 + ret bool v2 + } +} diff --git a/sway-ir/tests/cse/cse3.ir b/sway-ir/tests/cse/cse3.ir new file mode 100644 index 00000000000..71994e9e408 --- /dev/null +++ b/sway-ir/tests/cse/cse3.ir @@ -0,0 +1,22 @@ +script { + entry fn main(a: u64, b: u64) -> () { + entry(a: u64, b: u64): + v5v1 = add a, b + v6v1 = const u64 0 + br while(v6v1, v5v1) + + while(v3v1: u64, v4v1: u64): + v8v1 = cmp lt v3v1 v4v1 + cbr v8v1, while_body(), end_while() + + while_body(): + v10v1 = add a, b + v11v1 = const u64 1 + v12v1 = add v3v1, v11v1 + br while(v12v1, v10v1) + + end_while(): + v14v1 = const unit () + ret () v14v1 + } +} From f5a48de8eb23e2057b5424ecf0fea88f275c733d Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Mon, 29 Jul 2024 12:13:53 +0530 Subject: [PATCH 02/19] initialize PHIs too --- sway-ir/src/optimize/cse.rs | 15 +++++++++++---- sway-ir/src/pass_manager.rs | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 74532a6b6a0..015b268331f 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -10,8 +10,9 @@ use std::{ }; use crate::{ - AnalysisResults, BinaryOpKind, Context, DebugWithContext, Function, InstOp, IrError, Pass, - PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, Value, POSTORDER_NAME, + block, function_print, AnalysisResults, BinaryOpKind, Context, DebugWithContext, Function, + InstOp, IrError, Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, + Value, POSTORDER_NAME, }; pub const CSE_NAME: &str = "cse"; @@ -97,7 +98,7 @@ fn instr_to_expr(context: &Context, vntable: &VNTable, instr: Value) -> Option None, InstOp::ContractCall { .. } => None, - InstOp::FuelVm(_) => todo!(), + InstOp::FuelVm(_) => None, InstOp::GetLocal(_) => None, InstOp::GetConfig(_, _) => None, InstOp::GetElemPtr { @@ -173,6 +174,13 @@ pub fn cse( vntable.value_map.insert(arg.1, ValueNumber::Number(arg.1)); } + // Map all other arg values map to Top. + for block in function.block_iter(context).skip(1) { + for arg in block.arg_iter(context) { + vntable.value_map.insert(*arg, ValueNumber::Top); + } + } + // Initialize all instructions and constants. Constants need special treatmemt. // They don't have PartialEq implemented. So we need to value number them manually. // This map maps the hash of a constant value to all possible collisions of it. @@ -305,6 +313,5 @@ pub fn cse( vntable.expr_map.clear(); } - dbg!(vntable); Ok(false) } diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 7c63ae68077..2f70aff256e 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -409,8 +409,8 @@ pub fn create_o1_pass_group() -> PassGroup { o1.append_pass(FN_DCE_NAME); o1.append_pass(FN_INLINE_NAME); o1.append_pass(CONST_FOLDING_NAME); - o1.append_pass(CSE_NAME); o1.append_pass(SIMPLIFY_CFG_NAME); + o1.append_pass(CSE_NAME); o1.append_pass(CONST_FOLDING_NAME); o1.append_pass(SIMPLIFY_CFG_NAME); o1.append_pass(FN_DCE_NAME); From 4a1cf031fec5ca8972d12a47def91e527dcedba2 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Wed, 14 Aug 2024 14:30:06 +0530 Subject: [PATCH 03/19] Add CSE optimization and update unit tests --- sway-ir/src/analysis/dominator.rs | 51 +++++++++---- sway-ir/src/optimize/cse.rs | 118 +++++++++++++++++++++++++++--- sway-ir/src/optimize/mem2reg.rs | 4 +- sway-ir/src/pass_manager.rs | 11 ++- sway-ir/src/printer.rs | 3 +- sway-ir/tests/cse/cse1.ir | 4 + sway-ir/tests/cse/cse2.ir | 6 ++ sway-ir/tests/cse/cse3.ir | 31 +++++--- sway-ir/tests/tests.rs | 28 +++++-- 9 files changed, 206 insertions(+), 50 deletions(-) diff --git a/sway-ir/src/analysis/dominator.rs b/sway-ir/src/analysis/dominator.rs index 72de08fb690..9dfe35a310f 100644 --- a/sway-ir/src/analysis/dominator.rs +++ b/sway-ir/src/analysis/dominator.rs @@ -28,7 +28,8 @@ impl DomTreeNode { } // The dominator tree is represented by mapping each Block to its DomTreeNode. -pub type DomTree = FxIndexMap; +#[derive(Default)] +pub struct DomTree(FxIndexMap); impl AnalysisResultT for DomTree {} // Dominance frontier sets. @@ -120,11 +121,11 @@ fn compute_dom_tree( let entry = function.get_entry_block(context); // This is to make the algorithm happy. It'll be changed to None later. - dom_tree.insert(entry, DomTreeNode::new(Some(entry))); + dom_tree.0.insert(entry, DomTreeNode::new(Some(entry))); // initialize the dominators tree. This allows us to do dom_tree[b] fearlessly. // Note that we just previously initialized "entry", so we skip that here. for b in po.po_to_block.iter().take(po.po_to_block.len() - 1) { - dom_tree.insert(*b, DomTreeNode::new(None)); + dom_tree.0.insert(*b, DomTreeNode::new(None)); } let mut changed = true; @@ -149,12 +150,12 @@ fn compute_dom_tree( .pred_iter(context) .filter(|p| **p != picked_pred && po.block_to_po.contains_key(p)) { - if dom_tree[p].parent.is_some() { + if dom_tree.0[p].parent.is_some() { // if doms[p] already calculated new_idom = intersect(po, &dom_tree, *p, new_idom); } } - let b_node = dom_tree.get_mut(b).unwrap(); + let b_node = dom_tree.0.get_mut(b).unwrap(); match b_node.parent { Some(idom) if idom == new_idom => {} _ => { @@ -175,29 +176,49 @@ fn compute_dom_tree( ) -> Block { while finger1 != finger2 { while po.block_to_po[&finger1] < po.block_to_po[&finger2] { - finger1 = dom_tree[&finger1].parent.unwrap(); + finger1 = dom_tree.0[&finger1].parent.unwrap(); } while po.block_to_po[&finger2] < po.block_to_po[&finger1] { - finger2 = dom_tree[&finger2].parent.unwrap(); + finger2 = dom_tree.0[&finger2].parent.unwrap(); } } finger1 } // Fix the root. - dom_tree.get_mut(&entry).unwrap().parent = None; + dom_tree.0.get_mut(&entry).unwrap().parent = None; // Build the children. let child_parent: Vec<_> = dom_tree + .0 .iter() .filter_map(|(n, n_node)| n_node.parent.map(|n_parent| (*n, n_parent))) .collect(); for (child, parent) in child_parent { - dom_tree.get_mut(&parent).unwrap().children.push(child); + dom_tree.0.get_mut(&parent).unwrap().children.push(child); } Ok(Box::new(dom_tree)) } +impl DomTree { + /// Does `dominator` dominate `dominatee`? + pub fn dominates(&self, dominator: Block, dominatee: Block) -> bool { + let mut node_opt = Some(dominatee); + while let Some(node) = node_opt { + if node == dominator { + return true; + } + node_opt = self.0[&node].parent; + } + false + } + + /// Get an iterator over the childre nodes + pub fn children(&self, node: Block) -> impl Iterator + '_ { + self.0[&node].children.iter().cloned() + } +} + pub const DOM_FRONTS_NAME: &str = "dominance-frontiers"; pub fn create_dom_fronts_pass() -> Pass { @@ -217,23 +238,23 @@ fn compute_dom_fronts( ) -> Result { let dom_tree: &DomTree = analyses.get_analysis_result(function); let mut res = DomFronts::default(); - for (b, _) in dom_tree.iter() { + for (b, _) in dom_tree.0.iter() { res.insert(*b, IndexSet::default()); } // for all nodes, b - for (b, _) in dom_tree.iter() { + for (b, _) in dom_tree.0.iter() { // if the number of predecessors of b >= 2 if b.num_predecessors(context) > 1 { // unwrap() is safe as b is not "entry", and hence must have idom. - let b_idom = dom_tree[b].parent.unwrap(); + let b_idom = dom_tree.0[b].parent.unwrap(); // for all (reachable) predecessors, p, of b - for p in b.pred_iter(context).filter(|&p| dom_tree.contains_key(p)) { + for p in b.pred_iter(context).filter(|&p| dom_tree.0.contains_key(p)) { let mut runner = *p; while runner != b_idom { // add b to runner’s dominance frontier set res.get_mut(&runner).unwrap().insert(*b); - runner = dom_tree[&runner].parent.unwrap(); + runner = dom_tree.0[&runner].parent.unwrap(); } } } @@ -244,7 +265,7 @@ fn compute_dom_fronts( /// Print dominator tree in the graphviz dot format. pub fn print_dot(context: &Context, func_name: &str, dom_tree: &DomTree) -> String { let mut res = format!("digraph {func_name} {{\n"); - for (b, idom) in dom_tree.iter() { + for (b, idom) in dom_tree.0.iter() { if let Some(idom) = idom.parent { let _ = writeln!( res, diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 015b268331f..b25a929e3ba 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -1,7 +1,9 @@ -//! Value numbering based common subexpression elimination +//! Value numbering based common subexpression elimination. +//! Reference: Value Driven Redundancy Elimination - Loren Taylor Simpson. +use core::panic; use itertools::Itertools; -use rustc_hash::{FxHashMap, FxHasher}; +use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; use slotmap::Key; use std::{ collections::hash_map, @@ -10,9 +12,9 @@ use std::{ }; use crate::{ - block, function_print, AnalysisResults, BinaryOpKind, Context, DebugWithContext, Function, - InstOp, IrError, Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, - Value, POSTORDER_NAME, + AnalysisResults, BinaryOpKind, Context, DebugWithContext, DomTree, Function, InstOp, IrError, + Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, Value, + DOMINATORS_NAME, POSTORDER_NAME, }; pub const CSE_NAME: &str = "cse"; @@ -22,7 +24,7 @@ pub fn create_cse_pass() -> Pass { name: CSE_NAME, descr: "Common subexpression elimination", runner: ScopedPass::FunctionPass(PassMutability::Transform(cse)), - deps: vec![POSTORDER_NAME], + deps: vec![POSTORDER_NAME, DOMINATORS_NAME], } } @@ -154,14 +156,49 @@ struct VNTable { impl Debug for VNTable { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "value_map:\n")?; - for (key, value) in &self.value_map { - write!(f, "\tv{:?} -> {:?}\n", key.0.data(), value)? - } + writeln!(f, "value_map:")?; + self.value_map.iter().for_each(|(key, value)| { + writeln!(f, "\tv{:?} -> {:?}", key.0.data(), value).expect("writeln! failed"); + }); Ok(()) } } +/// Wrapper around [DomTree::dominates] to work at instruction level. +/// Does `inst1` dominate `inst2` ? +fn dominates(context: &Context, dom_tree: &DomTree, inst1: Value, inst2: Value) -> bool { + let block1 = match &context.values[inst1.0].value { + crate::ValueDatum::Argument(arg) => arg.block, + crate::ValueDatum::Constant(_) => { + panic!("Shouldn't be querying dominance info for constants") + } + crate::ValueDatum::Instruction(i) => i.parent, + }; + let block2 = match &context.values[inst2.0].value { + crate::ValueDatum::Argument(arg) => arg.block, + crate::ValueDatum::Constant(_) => { + panic!("Shouldn't be querying dominance info for constants") + } + crate::ValueDatum::Instruction(i) => i.parent, + }; + + if block1 == block2 { + let inst1_idx = block1 + .instruction_iter(context) + .position(|inst| inst == inst1) + // Not found indicates a block argument + .unwrap_or(0); + let inst2_idx = block1 + .instruction_iter(context) + .position(|inst| inst == inst2) + // Not found indicates a block argument + .unwrap_or(0); + inst1_idx < inst2_idx + } else { + dom_tree.dominates(block1, block2) + } +} + pub fn cse( context: &mut Context, analyses: &AnalysisResults, @@ -224,6 +261,7 @@ pub fn cse( // We need to iterate over the blocks in RPO. let post_order: &PostOrder = analyses.get_analysis_result(function); + // RPO based value number (Sec 4.2). let mut changed = true; while changed { changed = false; @@ -258,7 +296,7 @@ pub fn cse( Some(ValueNumber::Number(vn)) } (ValueNumber::Number(vn1), ValueNumber::Number(vn2)) => { - (vn1 == vn2).then(|| ValueNumber::Number(vn1)) + (vn1 == vn2).then_some(ValueNumber::Number(vn1)) } } } else { @@ -313,5 +351,61 @@ pub fn cse( vntable.expr_map.clear(); } - Ok(false) + // create a partition of congruent (equal) values. + let mut partition = FxHashMap::>::default(); + vntable.value_map.iter().for_each(|(v, vn)| { + // If v is a constant or its value number is itself, don't add to the partition. + // The latter condition is so that we have only > 1 sized partitions. + if v.is_constant(context) + || matches!(vn, ValueNumber::Top) + || matches!(vn, ValueNumber::Number(v2) if v == v2) + { + return; + } + partition + .entry(*vn) + .and_modify(|part| { + part.insert(*v); + }) + .or_insert(vec![*v].into_iter().collect()); + }); + + // For convenience, now add back back `v` into `partition[VN[v]]` if it isn't already there. + partition.iter_mut().for_each(|(vn, v_part)| { + let ValueNumber::Number(v) = vn else { + panic!("We cannot have Top at this point"); + }; + v_part.insert(*v); + assert!( + v_part.len() > 1, + "We've only created partitions with size greater than 1" + ); + }); + + // There are two ways to replace congruent values (see the paper cited, Sec 5). + // 1. Dominator based. If v1 and v2 are equal, v1 dominates v2, we just remove v2 + // and replace its uses with v1. Simple, and what we're going to do. + // 2. AVAIL based. More powerful, but also requires a data-flow-analysis for AVAIL + // and later on, mem2reg again since replacements will need breaking SSA. + let dom_tree: &DomTree = analyses.get_analysis_result(function); + let mut replace_map = FxHashMap::::default(); + let mut modified = false; + // Check every set in the partition. + partition.iter().for_each(|(_leader, vals)| { + // Iterate over every pair of values, checking if one can replace the other. + for v_pair in vals.iter().combinations(2) { + let (v1, v2) = (*v_pair[0], *v_pair[1]); + if dominates(context, dom_tree, v1, v2) { + modified = true; + replace_map.insert(v2, v1); + } else if dominates(context, dom_tree, v2, v1) { + modified = true; + replace_map.insert(v1, v2); + } + } + }); + + function.replace_values(context, &replace_map, None); + + Ok(modified) } diff --git a/sway-ir/src/optimize/mem2reg.rs b/sway-ir/src/optimize/mem2reg.rs index 0a3484cec4a..c7c8a5b847b 100644 --- a/sway-ir/src/optimize/mem2reg.rs +++ b/sway-ir/src/optimize/mem2reg.rs @@ -319,12 +319,12 @@ pub fn promote_to_registers( } // Process dominator children. - for child in dom_tree[&node].children.iter() { + for child in dom_tree.children(node) { record_rewrites( context, function, dom_tree, - *child, + child, safe_locals, phi_to_local, name_stack, diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 2f70aff256e..1e2ce28c849 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -1,5 +1,14 @@ use crate::{ - create_arg_demotion_pass, create_const_demotion_pass, create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME + create_arg_demotion_pass, create_const_demotion_pass, create_const_folding_pass, + create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, + create_escaped_symbols_pass, create_fn_dce_pass, create_fn_dedup_debug_profile_pass, + create_fn_dedup_release_profile_pass, create_fn_inline_pass, create_mem2reg_pass, + create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass, + create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass, + create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module, + ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, CSE_NAME, DCE_NAME, FN_DCE_NAME, + FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, + MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME, }; use downcast_rs::{impl_downcast, Downcast}; use rustc_hash::FxHashMap; diff --git a/sway-ir/src/printer.rs b/sway-ir/src/printer.rs index 80f336c8d35..19b90c92861 100644 --- a/sway-ir/src/printer.rs +++ b/sway-ir/src/printer.rs @@ -6,7 +6,6 @@ use std::collections::{BTreeMap, HashMap}; -use slotmap::Key; use sway_types::SourceEngine; use crate::{ @@ -1243,7 +1242,7 @@ impl Namer { fn default_name(&mut self, value: &Value) -> String { self.names.get(value).cloned().unwrap_or_else(|| { - let new_name = format!("v{:?}", value.0.data()); + let new_name = format!("v{}", self.next_value_idx); self.next_value_idx += 1; self.names.insert(*value, new_name.clone()); new_name diff --git a/sway-ir/tests/cse/cse1.ir b/sway-ir/tests/cse/cse1.ir index 4ba901a8b80..687c12f8d76 100644 --- a/sway-ir/tests/cse/cse1.ir +++ b/sway-ir/tests/cse/cse1.ir @@ -1,14 +1,18 @@ // regex: ID=[[:alpha:]0-9]+ +// regex: VAR=v\d+ script { fn main() -> bool { entry(): v0 = const u64 11 v1 = const u64 0 + // check: $(v3=$VAR) = add v3 = add v0, v1 + // check: $(v4=$VAR) = add v4 = add v0, v1 v10 = const u64 10 v11 = add v1, v0 + // check: cmp eq $v3 $v3 v2 = cmp eq v3 v4 v3 = cmp eq v11 v3 v4 = cmp eq v2 v3 diff --git a/sway-ir/tests/cse/cse2.ir b/sway-ir/tests/cse/cse2.ir index e268e03903c..ec609f74d60 100644 --- a/sway-ir/tests/cse/cse2.ir +++ b/sway-ir/tests/cse/cse2.ir @@ -1,4 +1,5 @@ // regex: ID=[[:alpha:]0-9]+ +// regex: VAR=v\d+ script { fn main() -> bool { @@ -6,10 +7,15 @@ script { v0 = const u64 11 v0_dup = const u64 11 v1 = const u64 0 + // check: $(v3=$VAR) = add v3 = add v0, v1 + // check: $(v4=$VAR) = add v4 = add v0, v1 + // check: $(v5=$VAR) = sub v5 = sub v0, v3 + // check: $(v6=$VAR) = sub v6 = sub v0_dup, v4 + // check: cmp eq $v5 $v5 v2 = cmp eq v5 v6 ret bool v2 } diff --git a/sway-ir/tests/cse/cse3.ir b/sway-ir/tests/cse/cse3.ir index 71994e9e408..89a3df2fdd3 100644 --- a/sway-ir/tests/cse/cse3.ir +++ b/sway-ir/tests/cse/cse3.ir @@ -1,22 +1,29 @@ +// regex: ID=[[:alpha:]0-9]+ +// regex: VAR=v\d+ + script { entry fn main(a: u64, b: u64) -> () { entry(a: u64, b: u64): - v5v1 = add a, b - v6v1 = const u64 0 - br while(v6v1, v5v1) + // check: $(v5=$VAR) = add a, b + v5 = add a, b + v6 = const u64 0 + br while(v6, v5) - while(v3v1: u64, v4v1: u64): - v8v1 = cmp lt v3v1 v4v1 - cbr v8v1, while_body(), end_while() + while(v3: u64, v4: u64): + // check: cmp lt $VAR $v5 + v8 = cmp lt v3 v4 + cbr v8, while_body(), end_while() while_body(): - v10v1 = add a, b - v11v1 = const u64 1 - v12v1 = add v3v1, v11v1 - br while(v12v1, v10v1) + // check: $(v10=$VAR) = add a, b + v10 = add a, b + v11 = const u64 1 + v12 = add v3, v11 + // check: br while($VAR, $v5) + br while(v12, v10) end_while(): - v14v1 = const unit () - ret () v14v1 + v14 = const unit () + ret () v14 } } diff --git a/sway-ir/tests/tests.rs b/sway-ir/tests/tests.rs index 54bc88176c4..efbfc8ba1b0 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -3,12 +3,12 @@ use std::path::PathBuf; use itertools::Itertools; use sway_ir::{ create_arg_demotion_pass, create_const_demotion_pass, create_const_folding_pass, - create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, - create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_postorder_pass, - create_ret_demotion_pass, create_simplify_cfg_pass, metadata_to_inline, optimize as opt, - register_known_passes, Context, ExperimentalFlags, Function, IrError, PassGroup, PassManager, - Value, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, - MEM2REG_NAME, SROA_NAME, + create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, + create_escaped_symbols_pass, create_mem2reg_pass, create_memcpyopt_pass, + create_misc_demotion_pass, create_postorder_pass, create_ret_demotion_pass, + create_simplify_cfg_pass, metadata_to_inline, optimize as opt, register_known_passes, Context, + ExperimentalFlags, Function, IrError, PassGroup, PassManager, Value, DCE_NAME, FN_DCE_NAME, + FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, MEM2REG_NAME, SROA_NAME, }; use sway_types::SourceEngine; @@ -266,6 +266,22 @@ fn dce() { // ------------------------------------------------------------------------------------------------- +#[allow(clippy::needless_collect)] +#[test] +fn cse() { + run_tests("cse", |_first_line, ir: &mut Context| { + let mut pass_mgr = PassManager::default(); + let mut pass_group = PassGroup::default(); + pass_mgr.register(create_postorder_pass()); + pass_mgr.register(create_dominators_pass()); + let pass = pass_mgr.register(create_cse_pass()); + pass_group.append_pass(pass); + pass_mgr.run(ir, &pass_group).unwrap() + }) +} + +// ------------------------------------------------------------------------------------------------- + #[allow(clippy::needless_collect)] #[test] fn mem2reg() { From 504783cd173ae395e3405645be9d83f2a732c253 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Thu, 15 Aug 2024 16:41:12 +0530 Subject: [PATCH 04/19] do not replace consts --- sway-ir/src/optimize/cse.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index b25a929e3ba..783925ae5c9 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -12,9 +12,9 @@ use std::{ }; use crate::{ - AnalysisResults, BinaryOpKind, Context, DebugWithContext, DomTree, Function, InstOp, IrError, - Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, Value, - DOMINATORS_NAME, POSTORDER_NAME, + function_print, AnalysisResults, BinaryOpKind, Context, DebugWithContext, DomTree, Function, + InstOp, IrError, Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, + Value, DOMINATORS_NAME, POSTORDER_NAME, }; pub const CSE_NAME: &str = "cse"; @@ -177,6 +177,7 @@ fn dominates(context: &Context, dom_tree: &DomTree, inst1: Value, inst2: Value) let block2 = match &context.values[inst2.0].value { crate::ValueDatum::Argument(arg) => arg.block, crate::ValueDatum::Constant(_) => { + dbg!(inst1, inst2); panic!("Shouldn't be querying dominance info for constants") } crate::ValueDatum::Instruction(i) => i.parent, @@ -204,6 +205,9 @@ pub fn cse( analyses: &AnalysisResults, function: Function, ) -> Result { + if function.get_name(context) == "sha256_6" { + function_print(context, function); + } let mut vntable = VNTable::default(); // Function arg values map to themselves. @@ -358,7 +362,7 @@ pub fn cse( // The latter condition is so that we have only > 1 sized partitions. if v.is_constant(context) || matches!(vn, ValueNumber::Top) - || matches!(vn, ValueNumber::Number(v2) if v == v2) + || matches!(vn, ValueNumber::Number(v2) if (v == v2 || v2.is_constant(context))) { return; } From 10561bfdc8d6b4da596b948e5b3f2dfc2c5f8d57 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Thu, 15 Aug 2024 16:46:14 +0530 Subject: [PATCH 05/19] spell fix --- sway-ir/src/optimize/cse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 783925ae5c9..0c3925c1e72 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -308,7 +308,7 @@ pub fn cse( } }) .flatten() - // The PHI couldn't be simplifed to a single ValueNumber. + // The PHI couldn't be simplified to a single ValueNumber. // lookup(expr, x) .unwrap_or_else(|| match vntable.expr_map.entry(expr) { hash_map::Entry::Occupied(occ) => *occ.get(), From 03963c5983b33ed4e3c4b0722c7370764844bb87 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 16 Aug 2024 21:00:09 +0530 Subject: [PATCH 06/19] Should not combine 2 PHIs, they may be in different loops --- sway-ir/src/optimize/cse.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 0c3925c1e72..3c3d0eaab70 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -158,7 +158,9 @@ impl Debug for VNTable { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { writeln!(f, "value_map:")?; self.value_map.iter().for_each(|(key, value)| { - writeln!(f, "\tv{:?} -> {:?}", key.0.data(), value).expect("writeln! failed"); + if format!("v{:?}", key.0.data()) == "v620v3" { + writeln!(f, "\tv{:?} -> {:?}", key.0.data(), value).expect("writeln! failed"); + } }); Ok(()) } @@ -309,14 +311,9 @@ pub fn cse( }) .flatten() // The PHI couldn't be simplified to a single ValueNumber. - // lookup(expr, x) - .unwrap_or_else(|| match vntable.expr_map.entry(expr) { - hash_map::Entry::Occupied(occ) => *occ.get(), - hash_map::Entry::Vacant(vac) => { - *(vac.insert(ValueNumber::Number(phi))) - } - }) + .unwrap_or_else(|| ValueNumber::Number(phi)) }; + match vntable.value_map.entry(phi) { hash_map::Entry::Occupied(occ) if *occ.get() == vn => {} _ => { @@ -355,6 +352,8 @@ pub fn cse( vntable.expr_map.clear(); } + dbg!(&vntable); + // create a partition of congruent (equal) values. let mut partition = FxHashMap::>::default(); vntable.value_map.iter().for_each(|(v, vn)| { From 14528883b72f28be55452181f2819e9db311fea8 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 16 Aug 2024 21:00:56 +0530 Subject: [PATCH 07/19] clippy fix --- sway-ir/src/optimize/cse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 3c3d0eaab70..2d239bcdeb1 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -311,7 +311,7 @@ pub fn cse( }) .flatten() // The PHI couldn't be simplified to a single ValueNumber. - .unwrap_or_else(|| ValueNumber::Number(phi)) + .unwrap_or(ValueNumber::Number(phi)) }; match vntable.value_map.entry(phi) { From 8462fdfb4b62c1a332be6afde98ddcb913bb2908 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 16 Aug 2024 21:28:35 +0530 Subject: [PATCH 08/19] update tests --- .../language/u256/u256_abi/json_abi_oracle_new_encoding.json | 2 +- .../array_of_structs_caller/src/main.sw | 2 +- .../require_contract_deployment/asset_ops_test/src/main.sw | 2 +- .../require_contract_deployment/bal_opcode/src/main.sw | 2 +- .../call_abi_with_tuples/src/main.sw | 2 +- .../require_contract_deployment/call_basic_storage/src/main.sw | 2 +- .../call_contract_with_type_aliases/src/main.sw | 2 +- .../call_increment_contract/src/main.sw | 2 +- .../require_contract_deployment/call_storage_enum/src/main.sw | 2 +- .../require_contract_deployment/caller_auth_test/src/main.sw | 2 +- .../require_contract_deployment/caller_context_test/src/main.sw | 2 +- .../nested_struct_args_caller/src/main.sw | 2 +- .../storage_access_caller/src/main.sw | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json index d2552663ebd..a982fde719b 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json @@ -7,7 +7,7 @@ "typeArguments": null }, "name": "SOME_U256", - "offset": 816 + "offset": 808 } ], "encoding": "1", diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw index cd5a22ec0a3..b6cce9fb794 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/array_of_structs_caller/src/main.sw @@ -6,7 +6,7 @@ use std::hash::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0x14ed3cd06c2947248f69d54bfa681fe40d26267be84df7e19e253622b7921bbe; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x316c03d37b53eaeffe22c2d2df50d675e2b2ee07bd8b73f852e686129aeba462; // AUTO-CONTRACT-ID ../../test_contracts/array_of_structs_contract --release +const CONTRACT_ID = 0xe8034b7b83eb42bd3c52757f5f1cd5808bef1594804ed4f0f4a4bbb6514fe411; // AUTO-CONTRACT-ID ../../test_contracts/array_of_structs_contract --release fn main() -> u64 { let addr = abi(TestContract, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw index 376ccd9b332..2796aac89fa 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/asset_ops_test/src/main.sw @@ -14,7 +14,7 @@ const FUEL_COIN_CONTRACT_ID = 0x04126a159537ffa66c60c062904e0408e92fa044f61742ae #[cfg(experimental_new_encoding = false)] const BALANCE_CONTRACT_ID = 0xf6cd545152ac83225e8e7df2efb5c6fa6e37bc9b9e977b5ea8103d28668925df; #[cfg(experimental_new_encoding = true)] -const BALANCE_CONTRACT_ID = 0x51088b17e33a9fbbcac387cd3e462571dfce54e340579d7130c5b6fe08793ea9; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release +const BALANCE_CONTRACT_ID = 0x0f5c045cc0b1dc371e0513fd0173a90aeb386629ae0511d90e400c57d65d6328; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release fn main() -> bool { let default_gas = 1_000_000_000_000; diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw index 79c9b83161e..096e15715f4 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/bal_opcode/src/main.sw @@ -5,7 +5,7 @@ use balance_test_abi::BalanceTest; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xf6cd545152ac83225e8e7df2efb5c6fa6e37bc9b9e977b5ea8103d28668925df; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x51088b17e33a9fbbcac387cd3e462571dfce54e340579d7130c5b6fe08793ea9; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release +const CONTRACT_ID = 0x0f5c045cc0b1dc371e0513fd0173a90aeb386629ae0511d90e400c57d65d6328; // AUTO-CONTRACT-ID ../../test_contracts/balance_test_contract --release fn main() -> bool { let balance_test_contract = abi(BalanceTest, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw index a326a415454..6c9d0685336 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_abi_with_tuples/src/main.sw @@ -6,7 +6,7 @@ use abi_with_tuples::{MyContract, Location, Person}; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xfdc14550c8aee742cd556d0ab7f378b7be0d3b1e6e086c097352e94590d4ed02; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x3e643ef32d855086058951d13730fc9cb891dba39147c545e4b2e8df0ed42f19; // AUTO-CONTRACT-ID ../../test_contracts/abi_with_tuples_contract --release +const CONTRACT_ID = 0x8c5c7f3e4f466f55d2ee66c264d52dcbaa721e120b9fe9e321ed1426511af663; // AUTO-CONTRACT-ID ../../test_contracts/abi_with_tuples_contract --release fn main() -> bool { let the_abi = abi(MyContract, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw index 6647c07e1b9..75ef2fe7ead 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw @@ -4,7 +4,7 @@ use basic_storage_abi::{BasicStorage, Quad}; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0x94db39f409a31b9f2ebcadeea44378e419208c20de90f5d8e1e33dc1523754cb; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0xcaaa4d2e98f4cea99de2ab3a79d160a01b5bd39796a3419c29f97faf27c8c717; // AUTO-CONTRACT-ID ../../test_contracts/basic_storage --release +const CONTRACT_ID = 0x14dcde44157de2afab6253cc27033c25febef024cbd11eb0f27b68e7b2ea81f0; // AUTO-CONTRACT-ID ../../test_contracts/basic_storage --release fn main() -> u64 { let addr = abi(BasicStorage, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_contract_with_type_aliases/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_contract_with_type_aliases/src/main.sw index 8dc12a1b8f5..008f5ae7d7b 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_contract_with_type_aliases/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_contract_with_type_aliases/src/main.sw @@ -5,7 +5,7 @@ use contract_with_type_aliases_abi::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0x0cbeb6efe3104b460be769bdc4ea101ebf16ccc16f2d7b667ec3e1c7f5ce35b5; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0xba120652492747e60222b5a37946c840a5bb91ba2665e11d000b9bd3063db237; // AUTO-CONTRACT-ID ../../test_contracts/contract_with_type_aliases --release +const CONTRACT_ID = 0xb805a413bff66e2130bac4cc63e8f54e655503dc629072f64ad8fd6601890814; // AUTO-CONTRACT-ID ../../test_contracts/contract_with_type_aliases --release fn main() { let caller = abi(MyContract, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_increment_contract/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_increment_contract/src/main.sw index 4716a5e8da8..86b094cc73f 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_increment_contract/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_increment_contract/src/main.sw @@ -6,7 +6,7 @@ use dynamic_contract_call::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xd1b4047af7ef111c023ab71069e01dc2abfde487c0a0ce1268e4f447e6c6e4c2; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x6f2d0d8ffc753e2efecf66dd00e258520e6a7bba294e302d239eb1f4d81efc4e; // AUTO-CONTRACT-ID ../../test_contracts/increment_contract --release +const CONTRACT_ID = 0x39606c9d9d395f231cc76e5b90826a8788ca25999df72e1ece4aafe6fe33ec07; // AUTO-CONTRACT-ID ../../test_contracts/increment_contract --release fn main() -> bool { let the_abi = abi(Incrementor, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_storage_enum/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_storage_enum/src/main.sw index 1c950a32e0f..e0d1e8af974 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_storage_enum/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_storage_enum/src/main.sw @@ -5,7 +5,7 @@ use storage_enum_abi::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xc601d11767195485a6654d566c67774134668863d8c797a8c69e8778fb1f89e9; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x90691afac688061184bffdf134ed06119d6187371d550a095a1d09d8406bc104; // AUTO-CONTRACT-ID ../../test_contracts/storage_enum_contract --release +const CONTRACT_ID = 0x7ebcca623be0e2ae91c08a3fcbd7eb90b9e31a2cdfb93f996dbce0e0c44464ff; // AUTO-CONTRACT-ID ../../test_contracts/storage_enum_contract --release fn main() -> u64 { let caller = abi(StorageEnum, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_auth_test/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_auth_test/src/main.sw index 8b8bed75d36..bcd2fe6e757 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_auth_test/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_auth_test/src/main.sw @@ -5,7 +5,7 @@ use auth_testing_abi::AuthTesting; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xc2eec20491b53aab7232cbd27c31d15417b4e9daf0b89c74cc242ef1295f681f; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x17078468cc937c3653ad0d2df73255aae6fc95934d4765897f3950773cb32199; // AUTO-CONTRACT-ID ../../test_contracts/auth_testing_contract --release +const CONTRACT_ID = 0xd4529187df29ad2f22bdefec7b9d3bebb81ade2d67ddef8f263da80c4f2b6fe8; // AUTO-CONTRACT-ID ../../test_contracts/auth_testing_contract --release // should be false in the case of a script fn main() -> bool { diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_context_test/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_context_test/src/main.sw index b5fc50227fd..21433cf8870 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_context_test/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/caller_context_test/src/main.sw @@ -6,7 +6,7 @@ use context_testing_abi::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0x6054c11cda000f5990373a4d61929396165be4dfdd61d5b7bd26da60ab0d8577; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x54f18a2b2e131fb8f0774c8ca368292101755a195cc609ea2fc88e708921b0a3; // AUTO-CONTRACT-ID ../../test_contracts/context_testing_contract --release +const CONTRACT_ID = 0x9fdffb1c0274daa03de1245907a657ae2832a1530a0127572dbc64562e50128f; // AUTO-CONTRACT-ID ../../test_contracts/context_testing_contract --release fn main() -> bool { let gas: u64 = u64::max(); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/nested_struct_args_caller/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/nested_struct_args_caller/src/main.sw index 715860df705..7c787662077 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/nested_struct_args_caller/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/nested_struct_args_caller/src/main.sw @@ -5,7 +5,7 @@ use nested_struct_args_abi::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0xe63d33a1b3a6903808b379f6a41a72fa8a370e8b76626775e7d9d2f9c4c5da40; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0x118efc99167f16e4f9b916ac6b3a85ca08e579cad533d1fca14fb3b460df691e; // AUTO-CONTRACT-ID ../../test_contracts/nested_struct_args_contract --release +const CONTRACT_ID = 0x99fbde6a68019774d28c05d2682124fe2dedb5343e1e1e140c11e5bbbcbc775b; // AUTO-CONTRACT-ID ../../test_contracts/nested_struct_args_contract --release fn main() -> bool { let caller = abi(NestedStructArgs, CONTRACT_ID); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw index f720cbddea9..1366011b2d3 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw @@ -6,7 +6,7 @@ use std::hash::*; #[cfg(experimental_new_encoding = false)] const CONTRACT_ID = 0x3bc28acd66d327b8c1b9624c1fabfc07e9ffa1b5d71c2832c3bfaaf8f4b805e9; #[cfg(experimental_new_encoding = true)] -const CONTRACT_ID = 0xa6026cfed3ba7a4ab3aa17908da17248f0489c042acc7ffaefb8b6c6536b069a; // AUTO-CONTRACT-ID ../../test_contracts/storage_access_contract --release +const CONTRACT_ID = 0xa138fa6272a72eb7e796db53f89e7c1862cb128882f18bf2029b4b7446795e1e; // AUTO-CONTRACT-ID ../../test_contracts/storage_access_contract --release fn main() -> bool { let caller = abi(StorageAccess, CONTRACT_ID); From d1e035c28fdd1335eba3d66a0e0757bb0da6aaa2 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Mon, 19 Aug 2024 07:19:02 +0530 Subject: [PATCH 09/19] remove dbg prints --- sway-ir/src/optimize/cse.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/sway-ir/src/optimize/cse.rs b/sway-ir/src/optimize/cse.rs index 2d239bcdeb1..7423cc2fa62 100644 --- a/sway-ir/src/optimize/cse.rs +++ b/sway-ir/src/optimize/cse.rs @@ -12,9 +12,9 @@ use std::{ }; use crate::{ - function_print, AnalysisResults, BinaryOpKind, Context, DebugWithContext, DomTree, Function, - InstOp, IrError, Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, - Value, DOMINATORS_NAME, POSTORDER_NAME, + AnalysisResults, BinaryOpKind, Context, DebugWithContext, DomTree, Function, InstOp, IrError, + Pass, PassMutability, PostOrder, Predicate, ScopedPass, Type, UnaryOpKind, Value, + DOMINATORS_NAME, POSTORDER_NAME, }; pub const CSE_NAME: &str = "cse"; @@ -179,7 +179,6 @@ fn dominates(context: &Context, dom_tree: &DomTree, inst1: Value, inst2: Value) let block2 = match &context.values[inst2.0].value { crate::ValueDatum::Argument(arg) => arg.block, crate::ValueDatum::Constant(_) => { - dbg!(inst1, inst2); panic!("Shouldn't be querying dominance info for constants") } crate::ValueDatum::Instruction(i) => i.parent, @@ -207,9 +206,6 @@ pub fn cse( analyses: &AnalysisResults, function: Function, ) -> Result { - if function.get_name(context) == "sha256_6" { - function_print(context, function); - } let mut vntable = VNTable::default(); // Function arg values map to themselves. @@ -352,8 +348,6 @@ pub fn cse( vntable.expr_map.clear(); } - dbg!(&vntable); - // create a partition of congruent (equal) values. let mut partition = FxHashMap::>::default(); vntable.value_map.iter().for_each(|(v, vn)| { From a90df95ceca4cad03e90357a1afb6009cb8c0a5f Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Mon, 9 Sep 2024 11:17:32 +0530 Subject: [PATCH 10/19] Fix tests and run cargo fmt --- sway-ir/src/pass_manager.rs | 12 ++++++- sway-ir/tests/tests.rs | 9 +++++- .../json_abi_oracle_new_encoding.json | 32 +++++++++---------- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index d86b19b9b31..9edb21626c3 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -1,5 +1,15 @@ use crate::{ - create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module, ARG_DEMOTION_NAME, CCP_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME + create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, + create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, + create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, + create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, + create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, + create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, + create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, + IrError, Module, ARG_DEMOTION_NAME, CCP_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, + CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, + FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, + SIMPLIFY_CFG_NAME, SROA_NAME, }; use downcast_rs::{impl_downcast, Downcast}; use rustc_hash::FxHashMap; diff --git a/sway-ir/tests/tests.rs b/sway-ir/tests/tests.rs index 58bd60e8bbb..197d67d400e 100644 --- a/sway-ir/tests/tests.rs +++ b/sway-ir/tests/tests.rs @@ -2,7 +2,14 @@ use std::path::PathBuf; use itertools::Itertools; use sway_ir::{ - create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, metadata_to_inline, optimize as opt, register_known_passes, Context, ExperimentalFlags, Function, IrError, PassGroup, PassManager, Value, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, MEM2REG_NAME, SROA_NAME + create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, + create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, + create_dominators_pass, create_escaped_symbols_pass, create_mem2reg_pass, + create_memcpyopt_pass, create_misc_demotion_pass, create_postorder_pass, + create_ret_demotion_pass, create_simplify_cfg_pass, metadata_to_inline, optimize as opt, + register_known_passes, Context, ExperimentalFlags, Function, IrError, PassGroup, PassManager, + Value, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, + MEM2REG_NAME, SROA_NAME, }; use sway_types::SourceEngine; diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json index 0662cd80d0d..b5e886d1a19 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json @@ -62,82 +62,82 @@ { "concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903", "name": "BOOL", - "offset": 7208 + "offset": 7216 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "U8", - "offset": 7400 + "offset": 7408 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "ANOTHER_U8", - "offset": 7136 + "offset": 7144 }, { "concreteTypeId": "29881aad8730c5ab11d275376323d8e4ff4179aae8ccb6c13fe4902137e162ef", "name": "U16", - "offset": 7344 + "offset": 7352 }, { "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc", "name": "U32", - "offset": 7384 + "offset": 7392 }, { "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc", "name": "U64", - "offset": 7392 + "offset": 7400 }, { "concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e", "name": "U256", - "offset": 7352 + "offset": 7360 }, { "concreteTypeId": "7c5ee1cecf5f8eacd1284feb5f0bf2bdea533a51e2f0c9aabe9236d335989f3b", "name": "B256", - "offset": 7176 + "offset": 7184 }, { "concreteTypeId": "81fc10c4681a3271cf2d66b2ec6fbc8ed007a442652930844fcf11818c295bff", "name": "CONFIGURABLE_STRUCT", - "offset": 7296 + "offset": 7304 }, { "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c", "name": "CONFIGURABLE_ENUM_A", - "offset": 7216 + "offset": 7224 }, { "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c", "name": "CONFIGURABLE_ENUM_B", - "offset": 7256 + "offset": 7264 }, { "concreteTypeId": "4926d35d1a5157936b0a29bc126b8aace6d911209a5c130e9b716b0c73643ea6", "name": "ARRAY_BOOL", - "offset": 7144 + "offset": 7152 }, { "concreteTypeId": "776fb5a3824169d6736138565fdc20aad684d9111266a5ff6d5c675280b7e199", "name": "ARRAY_U64", - "offset": 7152 + "offset": 7160 }, { "concreteTypeId": "c998ca9a5f221fe7b5c66ae70c8a9562b86d964408b00d17f883c906bc1fe4be", "name": "TUPLE_BOOL_U64", - "offset": 7328 + "offset": 7336 }, { "concreteTypeId": "94f0fa95c830be5e4f711963e83259fe7e8bc723278ab6ec34449e791a99b53a", "name": "STR_4", - "offset": 7320 + "offset": 7328 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "NOT_USED", - "offset": 7312 + "offset": 7320 } ], "encodingVersion": "1", From e794e64a8769dc5f94d381ac1cc0f7a699221d35 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 20 Sep 2024 14:31:15 +0530 Subject: [PATCH 11/19] group demonomophizable functions --- sway-core/src/lib.rs | 7 +- sway-ir/src/optimize/fn_dedup.rs | 116 +++++++++++++++++++++++++++---- sway-ir/src/pass_manager.rs | 13 +--- 3 files changed, 106 insertions(+), 30 deletions(-) diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index c744255156d..5071eda9f74 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -44,10 +44,7 @@ use std::sync::Arc; use sway_ast::AttributeDecl; use sway_error::handler::{ErrorEmitted, Handler}; use sway_ir::{ - create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, - PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, FN_DCE_NAME, - FN_DEDUP_DEBUG_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, - RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME, + create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_DEMONOMORPHIZE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME }; use sway_types::constants::DOC_COMMENT_ATTRIBUTE_NAME; use sway_types::SourceEngine; @@ -950,6 +947,8 @@ pub(crate) fn compile_ast_to_ir_to_asm( pass_group.append_pass(SROA_NAME); pass_group.append_pass(MEM2REG_NAME); pass_group.append_pass(DCE_NAME); + pass_group.append_pass(FN_DEDUP_DEMONOMORPHIZE_NAME); + pass_group.append_pass(FN_DCE_NAME); } OptLevel::Opt0 => {} } diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index 257db379a72..3ad59d5c72b 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -12,13 +12,14 @@ use std::hash::{Hash, Hasher}; use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; use crate::{ - build_call_graph, callee_first_order, AnalysisResults, Block, Context, Function, InstOp, - Instruction, IrError, MetadataIndex, Metadatum, Module, Pass, PassMutability, ScopedPass, - Value, + build_call_graph, callee_first_order, AnalysisResults, Block, Context, DebugWithContext, + Function, InstOp, Instruction, IrError, MetadataIndex, Metadatum, Module, Pass, PassMutability, + ScopedPass, Type, Value, }; pub const FN_DEDUP_DEBUG_PROFILE_NAME: &str = "fn-dedup-debug"; pub const FN_DEDUP_RELEASE_PROFILE_NAME: &str = "fn-dedup-release"; +pub const FN_DEDUP_DEMONOMORPHIZE_NAME: &str = "fn-dedup-demonomorphize"; pub fn create_fn_dedup_release_profile_pass() -> Pass { Pass { @@ -38,6 +39,15 @@ pub fn create_fn_dedup_debug_profile_pass() -> Pass { } } +pub fn create_fn_dedup_demonomorphize_pass() -> Pass { + Pass { + name: FN_DEDUP_DEMONOMORPHIZE_NAME, + descr: "Function deduplication via demonomorphization", + deps: vec![], + runner: ScopedPass::ModulePass(PassMutability::Transform(dedup_fn_demonomorphize)), + } +} + // Functions that are equivalent are put in the same set. struct EqClass { // Map a function hash to its equivalence class. @@ -51,6 +61,7 @@ fn hash_fn( function: Function, eq_class: &mut EqClass, ignore_metadata: bool, + ignore_pointee_type: bool, ) -> u64 { let state = &mut FxHasher::default(); @@ -91,6 +102,14 @@ fn hash_fn( } } + fn hash_type(context: &Context, hasher: &mut FxHasher, t: Type, ignore_pointee_type: bool) { + if t.is_ptr(context) && ignore_pointee_type { + std::mem::discriminant(t.get_content(context)).hash(hasher); + } else { + t.hash(hasher); + } + } + fn hash_metadata( context: &Context, m: MetadataIndex, @@ -141,7 +160,12 @@ fn hash_fn( } // Start with the function return type. - function.get_return_type(context).hash(state); + hash_type( + context, + state, + function.get_return_type(context), + ignore_pointee_type, + ); // ... and local variables. for (local_name, local_var) in function.locals_iter(context) { @@ -149,7 +173,15 @@ fn hash_fn( if let Some(init) = local_var.get_initializer(context) { init.hash(state); } - local_var.get_type(context).hash(state); + // Locals are pointers, so if we should ignore the pointee type, ignore the type of locals also. + if !ignore_pointee_type { + hash_type( + context, + state, + local_var.get_type(context), + ignore_pointee_type, + ); + } } // Process every block, first its arguments and then the instructions. @@ -157,7 +189,12 @@ fn hash_fn( get_localised_id(block, localised_block_id).hash(state); for &arg in block.arg_iter(context) { get_localised_id(arg, localised_value_id).hash(state); - arg.get_argument(context).unwrap().ty.hash(state); + hash_type( + context, + state, + arg.get_argument(context).unwrap().ty, + ignore_pointee_type, + ); } for inst in block.instruction_iter(context) { get_localised_id(inst, localised_value_id).hash(state); @@ -187,7 +224,7 @@ fn hash_fn( if let Some(return_name) = &asm_block.return_name { return_name.as_str().hash(state); } - asm_block.return_type.hash(state); + hash_type(context, state, asm_block.return_type, ignore_pointee_type); for asm_inst in &asm_block.body { asm_inst.op_name.as_str().hash(state); for arg in &asm_inst.args { @@ -200,7 +237,9 @@ fn hash_fn( } crate::InstOp::UnaryOp { op, .. } => op.hash(state), crate::InstOp::BinaryOp { op, .. } => op.hash(state), - crate::InstOp::BitCast(_, ty) => ty.hash(state), + crate::InstOp::BitCast(_, ty) => { + hash_type(context, state, *ty, ignore_pointee_type) + } crate::InstOp::Branch(b) => { get_localised_id(b.block, localised_block_id).hash(state) } @@ -216,7 +255,9 @@ fn hash_fn( } } } - crate::InstOp::CastPtr(_, ty) => ty.hash(state), + crate::InstOp::CastPtr(_, ty) => { + hash_type(context, state, *ty, ignore_pointee_type) + } crate::InstOp::Cmp(p, _, _) => p.hash(state), crate::InstOp::ConditionalBranch { cond_value: _, @@ -235,7 +276,9 @@ fn hash_fn( crate::FuelVmInstruction::Gtf { tx_field_id, .. } => { tx_field_id.hash(state) } - crate::FuelVmInstruction::Log { log_ty, .. } => log_ty.hash(state), + crate::FuelVmInstruction::Log { log_ty, .. } => { + hash_type(context, state, *log_ty, ignore_pointee_type) + } crate::FuelVmInstruction::ReadRegister(reg) => reg.hash(state), crate::FuelVmInstruction::Revert(_) | crate::FuelVmInstruction::JmpMem @@ -260,13 +303,19 @@ fn hash_fn( .unwrap() .hash(state), crate::InstOp::GetConfig(_, name) => name.hash(state), - crate::InstOp::GetElemPtr { elem_ptr_ty, .. } => elem_ptr_ty.hash(state), - crate::InstOp::IntToPtr(_, ty) => ty.hash(state), + crate::InstOp::GetElemPtr { elem_ptr_ty, .. } => { + hash_type(context, state, *elem_ptr_ty, ignore_pointee_type) + } + crate::InstOp::IntToPtr(_, ty) => { + hash_type(context, state, *ty, ignore_pointee_type) + } crate::InstOp::Load(_) => (), crate::InstOp::MemCopyBytes { byte_len, .. } => byte_len.hash(state), crate::InstOp::MemCopyVal { .. } | crate::InstOp::Nop => (), - crate::InstOp::PtrToInt(_, ty) => ty.hash(state), - crate::InstOp::Ret(_, ty) => ty.hash(state), + crate::InstOp::PtrToInt(_, ty) => { + hash_type(context, state, *ty, ignore_pointee_type) + } + crate::InstOp::Ret(_, ty) => hash_type(context, state, *ty, ignore_pointee_type), crate::InstOp::Store { .. } => (), } } @@ -289,7 +338,7 @@ pub fn dedup_fns( let cg = build_call_graph(context, &context.modules.get(module.0).unwrap().functions); let callee_first = callee_first_order(&cg); for function in callee_first { - let hash = hash_fn(context, function, eq_class, ignore_metadata); + let hash = hash_fn(context, function, eq_class, ignore_metadata, false); eq_class .hash_set_map .entry(hash) @@ -358,3 +407,40 @@ fn dedup_fn_release_profile( ) -> Result { dedup_fns(context, analysis_results, module, true) } + +fn dedup_fn_demonomorphize( + context: &mut Context, + _analysis_results: &AnalysisResults, + module: Module, +) -> Result { + let modified = false; + let eq_class = &mut EqClass { + hash_set_map: FxHashMap::default(), + function_hash_map: FxHashMap::default(), + }; + let cg = build_call_graph(context, &context.modules.get(module.0).unwrap().functions); + let callee_first = callee_first_order(&cg); + for function in callee_first { + let hash = hash_fn(context, function, eq_class, true, true); + eq_class + .hash_set_map + .entry(hash) + .and_modify(|class| { + class.insert(function); + }) + .or_insert(vec![function].into_iter().collect()); + eq_class.function_hash_map.insert(function, hash); + } + + for (class_id, class) in &eq_class.hash_set_map { + if class.len() > 1 { + print!("{}: ", class_id); + for f in class { + print!("{}, ", f.get_name(context)); + } + println!(""); + } + } + + Ok(modified) +} diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs index 9edb21626c3..15575b5565e 100644 --- a/sway-ir/src/pass_manager.rs +++ b/sway-ir/src/pass_manager.rs @@ -1,15 +1,5 @@ use crate::{ - create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, - create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, - create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, - create_fn_dedup_debug_profile_pass, create_fn_dedup_release_profile_pass, - create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, - create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, - create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, - IrError, Module, ARG_DEMOTION_NAME, CCP_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, - CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, - FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, - SIMPLIFY_CFG_NAME, SROA_NAME, + create_arg_demotion_pass, create_ccp_pass, create_const_demotion_pass, create_const_folding_pass, create_cse_pass, create_dce_pass, create_dom_fronts_pass, create_dominators_pass, create_escaped_symbols_pass, create_fn_dce_pass, create_fn_dedup_debug_profile_pass, create_fn_dedup_demonomorphize_pass, create_fn_dedup_release_profile_pass, create_fn_inline_pass, create_mem2reg_pass, create_memcpyopt_pass, create_misc_demotion_pass, create_module_printer_pass, create_module_verifier_pass, create_postorder_pass, create_ret_demotion_pass, create_simplify_cfg_pass, create_sroa_pass, Context, Function, IrError, Module, ARG_DEMOTION_NAME, CCP_NAME, CONST_DEMOTION_NAME, CONST_FOLDING_NAME, CSE_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_DEMONOMORPHIZE_NAME, FN_DEDUP_RELEASE_PROFILE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME }; use downcast_rs::{impl_downcast, Downcast}; use rustc_hash::FxHashMap; @@ -393,6 +383,7 @@ pub fn register_known_passes(pm: &mut PassManager) { // Optimization passes. pm.register(create_fn_dedup_release_profile_pass()); pm.register(create_fn_dedup_debug_profile_pass()); + pm.register(create_fn_dedup_demonomorphize_pass()); pm.register(create_mem2reg_pass()); pm.register(create_sroa_pass()); pm.register(create_fn_inline_pass()); From 2be2bbf1e19d6780ce9260c58303de6294224ac6 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 1 Nov 2024 08:19:02 +0530 Subject: [PATCH 12/19] [WIP] demonomorphization --- sway-ir/src/optimize/fn_dedup.rs | 70 ++++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index 3ad59d5c72b..3de9d4bbcb9 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -12,9 +12,9 @@ use std::hash::{Hash, Hasher}; use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; use crate::{ - build_call_graph, callee_first_order, AnalysisResults, Block, Context, DebugWithContext, - Function, InstOp, Instruction, IrError, MetadataIndex, Metadatum, Module, Pass, PassMutability, - ScopedPass, Type, Value, + build_call_graph, callee_first_order, function_print, AnalysisResults, Block, Context, + DebugWithContext, Function, InstOp, Instruction, IrError, LocalVar, MetadataIndex, Metadatum, + Module, Pass, PassMutability, ScopedPass, Type, Value, }; pub const FN_DEDUP_DEBUG_PROFILE_NAME: &str = "fn-dedup-debug"; @@ -432,13 +432,65 @@ fn dedup_fn_demonomorphize( eq_class.function_hash_map.insert(function, hash); } - for (class_id, class) in &eq_class.hash_set_map { - if class.len() > 1 { - print!("{}: ", class_id); - for f in class { - print!("{}, ", f.get_name(context)); + for (_class_id, class) in &eq_class.hash_set_map { + if class.len() <= 1 { + continue; + } + struct OthersTracker<'a> { + locals_iter: Box + 'a>, + instr_iter: Box + 'a>, + } + let mut class_iter = class.iter(); + let leader = class_iter.next().unwrap(); + let mut others: FxHashMap<_, _> = class_iter + .map(|f| { + ( + *f, + OthersTracker { + locals_iter: Box::new(f.locals_iter(context)), + instr_iter: Box::new(f.instruction_iter(context)), + }, + ) + }) + .collect(); + + // Collect those locals that need to be shifted to an argument. + // The key is a local from the leader and the value is a list of + // corresponding locals from others in the class. + let mut locals_to_args = FxHashMap::default(); + for local in leader.locals_iter(context) { + let mut other_locals = Vec::new(); + let mut shift_to_arg = false; + // If this local differs from a corresponding one in others in the class, + // we'll need to shift it to be a caller allocated parameter with an opaque + // pointer passed as parameter. + let ty = local.1.get_inner_type(context); + for other_func in others.iter_mut() { + let other_local = other_func.1.locals_iter.next().unwrap(); + assert!( + local.0 == other_local.0, + "If names differed, then the functions wouldn't be in the same class" + ); + other_locals.push(other_local.1); + let other_local_ty = other_local.1.get_inner_type(context); + if ty != other_local_ty { + shift_to_arg = true; + } + } + if shift_to_arg { + locals_to_args.insert(local.1, other_locals); } - println!(""); + } + for (idx_in_block, (inst, block)) in leader + .block_iter(context) + .map(|b| { + b.instruction_iter(context) + .map(move |inst| (inst, b)) + .enumerate() + }) + .flatten() + { + } } From 24f07ff54096fef82a36379952c5a18a27e3ad71 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Thu, 5 Dec 2024 13:57:36 +0530 Subject: [PATCH 13/19] Add a demonomorphization pass --- .../src/asm_generation/evm/evm_asm_builder.rs | 4 +- .../asm_generation/fuel/fuel_asm_builder.rs | 51 +- sway-core/src/ir_generation/function.rs | 18 +- sway-ir/src/instruction.rs | 4 +- sway-ir/src/optimize/fn_dedup.rs | 492 +++++++++++++++++- sway-ir/src/optimize/memcpyopt.rs | 69 ++- sway-ir/src/parser.rs | 6 +- sway-ir/src/printer.rs | 2 +- sway-ir/src/verify.rs | 5 +- 9 files changed, 597 insertions(+), 54 deletions(-) diff --git a/sway-core/src/asm_generation/evm/evm_asm_builder.rs b/sway-core/src/asm_generation/evm/evm_asm_builder.rs index ecddcc69ac5..20c35ca6ee0 100644 --- a/sway-core/src/asm_generation/evm/evm_asm_builder.rs +++ b/sway-core/src/asm_generation/evm/evm_asm_builder.rs @@ -350,7 +350,7 @@ impl<'ir, 'eng> EvmAsmBuilder<'ir, 'eng> { dst_val_ptr, src_val_ptr, byte_len, - } => self.compile_mem_copy_bytes(instr_val, dst_val_ptr, src_val_ptr, *byte_len), + } => self.compile_mem_copy_bytes(instr_val, dst_val_ptr, src_val_ptr, byte_len), InstOp::MemCopyVal { dst_val_ptr, src_val_ptr, @@ -506,7 +506,7 @@ impl<'ir, 'eng> EvmAsmBuilder<'ir, 'eng> { instr_val: &Value, dst_val_ptr: &Value, src_val_ptr: &Value, - byte_len: u64, + byte_len: &Value, ) { todo!(); } diff --git a/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs b/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs index a97ff923635..c0edcd9924a 100644 --- a/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs +++ b/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs @@ -484,7 +484,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> { dst_val_ptr, src_val_ptr, byte_len, - } => self.compile_mem_copy_bytes(instr_val, dst_val_ptr, src_val_ptr, *byte_len), + } => self.compile_mem_copy_bytes(instr_val, dst_val_ptr, src_val_ptr, byte_len), InstOp::MemCopyVal { dst_val_ptr, src_val_ptr, @@ -1441,6 +1441,40 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> { } fn compile_mem_copy_bytes( + &mut self, + instr_val: &Value, + dst_val_ptr: &Value, + src_val_ptr: &Value, + byte_len: &Value, + ) -> Result<(), CompileError> { + if let Some(byte_len_const) = byte_len + .get_constant(self.context) + .and_then(|c| c.as_uint()) + { + return self.compile_mem_copy_const_bytes( + instr_val, + dst_val_ptr, + src_val_ptr, + byte_len_const, + ); + } + + let owning_span = self.md_mgr.val_to_span(self.context, *instr_val); + + let dst_reg = self.value_to_register(dst_val_ptr)?; + let src_reg = self.value_to_register(src_val_ptr)?; + let len_reg = self.value_to_register(byte_len)?; + + self.cur_bytecode.push(Op { + opcode: Either::Left(VirtualOp::MCP(dst_reg, src_reg, len_reg)), + comment: "copy memory".into(), + owning_span, + }); + + Ok(()) + } + + fn compile_mem_copy_const_bytes( &mut self, instr_val: &Value, dst_val_ptr: &Value, @@ -1457,6 +1491,18 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> { let dst_reg = self.value_to_register(dst_val_ptr)?; let src_reg = self.value_to_register(src_val_ptr)?; + // If we can use an MCPI instead of MOVI + MCP, do that. + if let Ok(byte_len_imm) = + VirtualImmediate12::new(byte_len, owning_span.clone().unwrap_or(Span::dummy())) + { + self.cur_bytecode.push(Op { + opcode: Either::Left(VirtualOp::MCPI(dst_reg, src_reg, byte_len_imm)), + comment: "copy memory".into(), + owning_span, + }); + return Ok(()); + } + let len_reg = self.reg_seqr.next(); self.cur_bytecode.push(Op { opcode: Either::Left(VirtualOp::MOVI( @@ -1496,7 +1542,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> { ) })?; let byte_len = dst_ty.size(self.context).in_bytes(); - self.compile_mem_copy_bytes(instr_val, dst_val_ptr, src_val_ptr, byte_len) + self.compile_mem_copy_const_bytes(instr_val, dst_val_ptr, src_val_ptr, byte_len) } fn compile_log( @@ -2128,6 +2174,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> { }) }) .ok_or_else(|| { + dbg!(value); let span = self.md_mgr.val_to_span(self.context, *value); CompileError::Internal( "An attempt to get register for unknown Value.", diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs index 7c333f55185..fc79ff1aa88 100644 --- a/sway-core/src/ir_generation/function.rs +++ b/sway-core/src/ir_generation/function.rs @@ -460,9 +460,10 @@ impl<'eng> FnCompiler<'eng> { .add_metadatum(context, span_md_idx); // copy the value of the struct variable into the slice + let c_16 = Constant::get_uint(context, 64, 16); self.current_block .append(context) - .mem_copy_bytes(slice_val, struct_val, 16); + .mem_copy_bytes(slice_val, struct_val, c_16); // return the slice Ok(TerminatorValue::new(slice_val, context)) @@ -1720,9 +1721,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); + let len = Constant::get_uint(context, 64, 8 - offset); s.current_block .append(context) - .mem_copy_bytes(addr, item_ptr, 8 - offset); + .mem_copy_bytes(addr, item_ptr, len); Ok(increase_len(&mut s.current_block, context, len, 8 - offset)) } @@ -1987,9 +1989,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); + let len = Constant::get_uint(context, 64, 32); self.current_block .append(context) - .mem_copy_bytes(addr, item_ptr, 32); + .mem_copy_bytes(addr, item_ptr, len); increase_len(&mut self.current_block, context, len, 32) } TypeInfo::StringArray(string_len) => { @@ -2002,11 +2005,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); - self.current_block.append(context).mem_copy_bytes( - addr, - item_ptr, - string_len.val() as u64, - ); + let len = Constant::get_uint(context, 64, string_len.val() as u64); + self.current_block + .append(context) + .mem_copy_bytes(addr, item_ptr, len); increase_len( &mut self.current_block, context, diff --git a/sway-ir/src/instruction.rs b/sway-ir/src/instruction.rs index 9e7b92fb5e0..85b0243e81f 100644 --- a/sway-ir/src/instruction.rs +++ b/sway-ir/src/instruction.rs @@ -101,7 +101,7 @@ pub enum InstOp { MemCopyBytes { dst_val_ptr: Value, src_val_ptr: Value, - byte_len: u64, + byte_len: Value, }, /// Copy a value from one pointer to another. MemCopyVal { @@ -1062,7 +1062,7 @@ impl<'a, 'eng> InstructionInserter<'a, 'eng> { ) } - pub fn mem_copy_bytes(self, dst_val_ptr: Value, src_val_ptr: Value, byte_len: u64) -> Value { + pub fn mem_copy_bytes(self, dst_val_ptr: Value, src_val_ptr: Value, byte_len: Value) -> Value { insert_instruction!( self, InstOp::MemCopyBytes { diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index 3de9d4bbcb9..32da81a72ed 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -7,14 +7,20 @@ //! generating a new function for each instantiation even when the exact //! same instantiation exists. -use std::hash::{Hash, Hasher}; +use std::{ + hash::{Hash, Hasher}, + iter, +}; +use itertools::Itertools; +use prettydiff::format_table::new; use rustc_hash::{FxHashMap, FxHashSet, FxHasher}; use crate::{ - build_call_graph, callee_first_order, function_print, AnalysisResults, Block, Context, - DebugWithContext, Function, InstOp, Instruction, IrError, LocalVar, MetadataIndex, Metadatum, - Module, Pass, PassMutability, ScopedPass, Type, Value, + build_call_graph, callee_first_order, function, function_print, AnalysisResults, Block, + BlockArgument, Constant, Context, DebugWithContext, Function, InstOp, Instruction, + InstructionInserter, IrError, LocalVar, MetadataIndex, Metadatum, Module, Pass, PassMutability, + ScopedPass, Type, Value, ValueDatum, }; pub const FN_DEDUP_DEBUG_PROFILE_NAME: &str = "fn-dedup-debug"; @@ -413,6 +419,8 @@ fn dedup_fn_demonomorphize( _analysis_results: &AnalysisResults, module: Module, ) -> Result { + // println!("{}", context); + let modified = false; let eq_class = &mut EqClass { hash_set_map: FxHashMap::default(), @@ -471,26 +479,482 @@ fn dedup_fn_demonomorphize( local.0 == other_local.0, "If names differed, then the functions wouldn't be in the same class" ); - other_locals.push(other_local.1); + other_locals.push(*other_local.1); let other_local_ty = other_local.1.get_inner_type(context); if ty != other_local_ty { shift_to_arg = true; } } if shift_to_arg { - locals_to_args.insert(local.1, other_locals); + locals_to_args.insert(*local.1, other_locals); + } + } + + let mut can_optimize = true; + + #[derive(Default)] + struct ChangeInstrs { + // all the CastPtr/IntToPtr that need change in the leader + cast_to_ptr: FxHashSet, + // all the GetLocal that need change in the leader + get_local: FxHashSet, + // All the GEPs Map that need to become a + // "add pointer + offset", where offset if parameterized instruction. + gep: FxHashMap>, + // All the MemCopyVals Map that need to become + // MemCopyBytes with the size parameterized. + mem_copy_val: FxHashMap>, + } + + let mut change_instrs = ChangeInstrs::default(); + 'leader_loop: for (_, inst) in leader.instruction_iter(context) { + for other_func in others.iter_mut() { + let (_other_block, other_instr) = other_func.1.instr_iter.next().unwrap(); + // Throughout this loop we check only for differing types between the leader and + // its followers. Other differences aren't checked for because then the hashes would + // be different and they wouldn't be in the same class. + match &inst.get_instruction(context).unwrap().op { + InstOp::AsmBlock(asm_block, _args) => { + let InstOp::AsmBlock(other_asm_block, _) = + &other_instr.get_instruction(context).unwrap().op + else { + panic!("Leader and follower are different instructions in same class"); + }; + if asm_block.return_type != other_asm_block.return_type { + can_optimize = false; + break 'leader_loop; + } + } + InstOp::UnaryOp { .. } => {} + InstOp::BinaryOp { .. } => {} + InstOp::BitCast(_value, ty) => { + let InstOp::BitCast(_other_value, other_ty) = + &other_instr.get_instruction(context).unwrap().op + else { + panic!("Leader and follower are different instructions in same class"); + }; + if ty != other_ty { + can_optimize = false; + break 'leader_loop; + } + } + InstOp::Branch(..) => {} + InstOp::Call(..) => {} + InstOp::CastPtr(_, target_ty) | InstOp::IntToPtr(_, target_ty) => { + match &other_instr.get_instruction(context).unwrap().op { + InstOp::CastPtr(_, other_target_ty) + | InstOp::IntToPtr(_, other_target_ty) => { + if target_ty != other_target_ty { + change_instrs.cast_to_ptr.insert(inst); + } + } + _ => { + panic!( + "Leader and follower are different instructions in same class" + ); + } + } + } + InstOp::Cmp(..) => {} + InstOp::ConditionalBranch { .. } => {} + InstOp::ContractCall { .. } => {} + InstOp::FuelVm(_fuel_vm_instruction) => {} + InstOp::GetLocal(local_var) => { + if locals_to_args.contains_key(local_var) { + change_instrs.get_local.insert(inst); + } + } + InstOp::GetConfig(..) => {} + InstOp::GetElemPtr { + elem_ptr_ty: _, + indices, + base, + } => { + let InstOp::GetElemPtr { + elem_ptr_ty: _, + indices: other_indices, + base: other_base, + } = &other_instr.get_instruction(context).unwrap().op + else { + panic!("Leader and follower are different instructions in same class"); + }; + let base_ty = base + .get_type(context) + .unwrap() + .get_pointee_type(context) + .unwrap(); + let other_base_ty = other_base + .get_type(context) + .unwrap() + .get_pointee_type(context) + .unwrap(); + if base_ty != other_base_ty { + // If we can't determine the offset to a compile time constant, + // we cannot do the optimization. + if base_ty.get_value_indexed_offset(context, indices).is_none() + || other_base_ty + .get_value_indexed_offset(context, &other_indices) + .is_none() + { + can_optimize = false; + break 'leader_loop; + } + change_instrs + .gep + .entry(inst) + .and_modify(|others| others.push(other_instr)) + .or_insert(vec![other_instr]); + } + } + InstOp::Load(_value) => {} + InstOp::MemCopyBytes { .. } => {} + InstOp::MemCopyVal { dst_val_ptr, .. } => { + let InstOp::MemCopyVal { + dst_val_ptr: other_dst_val_ptr, + .. + } = &other_instr.get_instruction(context).unwrap().op + else { + panic!("Leader and follower are different instructions in same class"); + }; + let copied_ty = dst_val_ptr.get_type(context).unwrap(); + let other_copied_ty = other_dst_val_ptr.get_type(context).unwrap(); + if copied_ty != other_copied_ty { + change_instrs + .mem_copy_val + .entry(inst) + .and_modify(|others| others.push(other_instr)) + .or_insert(vec![other_instr]); + } + } + InstOp::Nop => {} + InstOp::PtrToInt(..) => {} + InstOp::Ret(..) => {} + InstOp::Store { .. } => {} + } } } - for (idx_in_block, (inst, block)) in leader - .block_iter(context) - .map(|b| { - b.instruction_iter(context) - .map(move |inst| (inst, b)) - .enumerate() + + if !can_optimize { + continue; + } + + if change_instrs.cast_to_ptr.is_empty() + && change_instrs.gep.is_empty() + && change_instrs.get_local.is_empty() + && change_instrs.mem_copy_val.is_empty() + { + continue; + } + + // Map every function in the class to an index. Useful later on. + let class_fn_to_idx: FxHashMap<_, _> = iter::once(leader) + .chain(others.keys()) + .enumerate() + .map(|(idx, f)| (*f, idx)) + .collect(); + + // Note down all call sites for later use. + let call_sites = context + .module_iter() + .flat_map(|module| module.function_iter(context)) + .flat_map(|ref call_from_func| { + call_from_func + .block_iter(context) + .flat_map(|ref block| { + block + .instruction_iter(context) + .filter_map(|instr_val| { + if let Instruction { + op: InstOp::Call(call_to_func, _), + .. + } = instr_val + .get_instruction(context) + .expect("`instruction_iter()` must return instruction values.") + { + iter::once(leader) + .chain(others.keys()) + .contains(call_to_func) + .then_some((*call_from_func, *block, instr_val)) + } else { + None + } + }) + .collect::>() + }) + .collect::>() }) - .flatten() + .collect::>(); + + // `others` captures `context`, so let's drop it now. + drop(others); + + let unit_ptr_ty = Type::new_ptr(context, Type::get_unit(context)); + + // Track the additional arguments we're adding. + #[derive(Clone)] + enum NewArg { + // A local which is now allocated in the caller and + // whose pointer is passed as parameter. + CallerAllocatedLocal(LocalVar), + // A u64 value + Size(u64), + } + // New arguments for the leader followed by every other. + let mut new_args: Vec> = vec![Vec::new(); class.len()]; + // Argument number for a local + let mut local_to_argno = FxHashMap::default(); + + // We'll collect all the new arguments first, + // and then actually add them and modify the instructions. + for (local, other_locals) in locals_to_args { + new_args[0].push(NewArg::CallerAllocatedLocal(local)); + for (i, ty) in other_locals + .iter() + .map(|other_local| NewArg::CallerAllocatedLocal(*other_local)) + .enumerate() + { + new_args[i + 1].push(ty); + } + local_to_argno.insert(local, new_args[0].len() - 1); + } + + // Map a GEP or MemCopyVal to the new size parameter. + let mut gep_memcpyval_to_argno = FxHashMap::default(); + + for (inst, other_insts) in change_instrs + .gep + .iter() + .chain(change_instrs.mem_copy_val.iter()) { - + let mut this_params: Vec = Vec::new(); + for inst in std::iter::once(inst).chain(other_insts) { + match &inst.get_instruction(context).unwrap().op { + InstOp::GetElemPtr { + elem_ptr_ty: _, + indices, + base, + } => { + let base_ty = base + .get_type(context) + .unwrap() + .get_pointee_type(context) + .unwrap(); + let offset = base_ty.get_value_indexed_offset(context, indices).unwrap(); + this_params.push(offset); + } + InstOp::MemCopyVal { + dst_val_ptr, + src_val_ptr: _, + } => { + let copied_ty = dst_val_ptr.get_type(context).unwrap(); + let size_copied_type_bytes = copied_ty.size(context).in_bytes(); + this_params.push(size_copied_type_bytes); + } + _ => { + unreachable!("Expected only GEPs or MemCopyVals") + } + } + } + assert!(this_params.len() == class.len()); + // Check if any row in new_args is already the same as this_params, + // in which case we can reuse that parameter. + let argno = (0..new_args[0].len()).find_map(|i| { + if matches!(new_args[0][i], NewArg::CallerAllocatedLocal(..)) { + return None; + } + let ith_params: Vec<_> = new_args + .iter() + .map(|params| { + let NewArg::Size(size_param) = params[i] else { + panic!("We just filtered for Size parameters above"); + }; + size_param + }) + .collect(); + (this_params == ith_params).then_some(i) + }); + if let Some(argno) = argno { + gep_memcpyval_to_argno.insert(inst, argno); + } else { + let argno = new_args[0].len(); + gep_memcpyval_to_argno.insert(inst, argno); + + // Let's add a new row to new_args. + for (i, param) in this_params.iter().enumerate() { + new_args[i].push(NewArg::Size(*param)); + } + } + } + + // We are now equipped to actually modify the program. + // 1. Add the new arguments. + let mut new_arg_values = Vec::with_capacity(new_args[0].len()); + let entry_block = leader.get_entry_block(context); + for new_arg in &new_args[0] { + let (new_block_arg, new_arg_name) = match new_arg { + NewArg::CallerAllocatedLocal(..) => ( + BlockArgument { + block: entry_block, + idx: leader.num_args(context), + ty: unit_ptr_ty, + }, + "demonomorphize_alloca_arg", + ), + NewArg::Size(_) => ( + BlockArgument { + block: entry_block, + idx: leader.num_args(context), + ty: Type::get_uint64(context), + }, + "demonomorphize_size_arg", + ), + }; + let new_arg_value = Value::new_argument(context, new_block_arg); + leader.add_arg(context, new_arg_name, new_arg_value); + entry_block.add_arg(context, new_arg_value); + new_arg_values.push(new_arg_value); + } + + // 2. Modify pointer casts. + for cast_to_ptr in change_instrs.cast_to_ptr { + let instr = cast_to_ptr.get_instruction(context).unwrap(); + let new_instr = match &instr.op { + InstOp::CastPtr(source, _target_ty) => InstOp::CastPtr(*source, unit_ptr_ty), + InstOp::IntToPtr(source, _target_ty) => InstOp::IntToPtr(*source, unit_ptr_ty), + _ => unreachable!(), + }; + let new_instr = ValueDatum::Instruction(Instruction { + op: new_instr, + parent: instr.parent, + }); + cast_to_ptr.replace(context, new_instr); + } + + // 3. Modify GEPs. + for (gep, _) in &change_instrs.gep { + let instr = gep.get_instruction(context).unwrap(); + let InstOp::GetElemPtr { + elem_ptr_ty, + indices: _, + base, + } = instr.op + else { + panic!("Should be GEP"); + }; + let arg_idx = gep_memcpyval_to_argno[gep]; + let arg_value = new_arg_values[arg_idx]; + let parent_block = instr.parent; + + let replacement_add = Value::new_instruction( + context, + parent_block, + InstOp::BinaryOp { + op: crate::BinaryOpKind::Add, + arg1: base, + arg2: arg_value, + }, + ); + let mut inserter = InstructionInserter::new( + context, + parent_block, + crate::InsertionPosition::Before(*gep), + ); + inserter.insert(replacement_add); + let ptr_cast = ValueDatum::Instruction(Instruction { + parent: parent_block, + op: InstOp::CastPtr(replacement_add, elem_ptr_ty), + }); + + gep.replace(context, ptr_cast); + } + + // 4. Modify MemCopyVals + for (mem_copy_val, _) in &change_instrs.mem_copy_val { + let instr = mem_copy_val.get_instruction(context).unwrap(); + let InstOp::MemCopyVal { + dst_val_ptr, + src_val_ptr, + } = &instr.op + else { + panic!("Should be MemCopyVal"); + }; + let arg_idx = gep_memcpyval_to_argno[mem_copy_val]; + let arg_value = new_arg_values[arg_idx]; + let replacement_memcpybyte = ValueDatum::Instruction(Instruction { + op: InstOp::MemCopyBytes { + dst_val_ptr: *dst_val_ptr, + src_val_ptr: *src_val_ptr, + byte_len: arg_value, + }, + parent: instr.parent, + }); + mem_copy_val.replace(context, replacement_memcpybyte); + } + + // 5. Update the uses of get_local instructions to directly use the argument. + let mut replacements = FxHashMap::default(); + for get_local in &change_instrs.get_local { + let InstOp::GetLocal(local_var) = get_local.get_instruction(context).unwrap().op else { + panic!("Expected GetLocal"); + }; + let arg = local_to_argno.get(&local_var).unwrap(); + replacements.insert(*get_local, new_arg_values[*arg]); + } + leader.replace_values(context, &replacements, None); + + // 6. Finally modify calls to each function in the class. + for (caller, call_block, call_inst) in call_sites { + // Update the callee in call_inst first, all calls go to the leader now. + let InstOp::Call(callee, _) = &mut call_inst.get_instruction_mut(context).unwrap().op + else { + panic!("Expected Call"); + }; + *callee = *leader; + let callee = *callee; + + // Now add the new args. + let callee_idx = class_fn_to_idx[&callee]; + let new_args = &new_args[callee_idx]; + for new_arg in new_args { + match new_arg { + NewArg::CallerAllocatedLocal(original_local) => { + let name = callee + .lookup_local_name(context, original_local) + .cloned() + .unwrap_or("".to_string()) + + "_demonomorphized"; + let new_local = caller.new_unique_local_var( + context, + name, + original_local.get_type(context), + original_local.get_initializer(context).cloned(), + original_local.is_mutable(context), + ); + let inserter = InstructionInserter::new( + context, + call_block, + crate::InsertionPosition::Before(call_inst), + ); + let new_local_ptr = inserter.get_local(new_local); + let InstOp::Call(_, args) = + &mut call_inst.get_instruction_mut(context).unwrap().op + else { + panic!("Expected Call"); + }; + args.push(new_local_ptr); + } + NewArg::Size(val) => { + let new_size_const = Constant::new_uint(context, 64, *val); + let new_size_arg = Value::new_constant(context, new_size_const); + let InstOp::Call(_, args) = + &mut call_inst.get_instruction_mut(context).unwrap().op + else { + panic!("Expected Call"); + }; + args.push(new_size_arg); + } + } + } } } diff --git a/sway-ir/src/optimize/memcpyopt.rs b/sway-ir/src/optimize/memcpyopt.rs index 7046ec094f1..37ff23ca753 100644 --- a/sway-ir/src/optimize/memcpyopt.rs +++ b/sway-ir/src/optimize/memcpyopt.rs @@ -7,7 +7,7 @@ use sway_types::{FxIndexMap, FxIndexSet}; use crate::{ get_gep_symbol, get_referred_symbol, get_referred_symbols, get_stored_symbols, memory_utils, - AnalysisResults, Block, Context, EscapedSymbols, Function, InstOp, Instruction, + AnalysisResults, Block, Constant, Context, EscapedSymbols, Function, InstOp, Instruction, InstructionInserter, IrError, LocalVar, Pass, PassMutability, ReferredSymbols, ScopedPass, Symbol, Type, Value, ValueDatum, ESCAPED_SYMBOLS_NAME, }; @@ -285,7 +285,7 @@ fn local_copy_prop( fn kill_defined_symbol( context: &Context, value: Value, - len: u64, + len: Option, available_copies: &mut FxHashSet, src_to_copies: &mut FxIndexMap>, dest_to_copies: &mut FxIndexMap>, @@ -296,7 +296,16 @@ fn local_copy_prop( if let Some(copies) = src_to_copies.get_mut(&sym) { for copy in &*copies { let (_, src_ptr, copy_size) = deconstruct_memcpy(context, *copy); - if memory_utils::may_alias(context, value, len, src_ptr, copy_size) { + if len.is_none() + || copy_size.is_none() + || memory_utils::may_alias( + context, + value, + len.unwrap(), + src_ptr, + copy_size.unwrap(), + ) + { available_copies.remove(copy); } } @@ -314,7 +323,10 @@ fn local_copy_prop( byte_len, }, .. - } => (*dst_val_ptr, *byte_len), + } => ( + *dst_val_ptr, + byte_len.get_constant(context).and_then(|c| c.as_uint()), + ), Instruction { op: InstOp::MemCopyVal { @@ -324,11 +336,22 @@ fn local_copy_prop( .. } => ( *dst_val_ptr, - memory_utils::pointee_size(context, *dst_val_ptr), + Some(memory_utils::pointee_size(context, *dst_val_ptr)), ), _ => panic!("Unexpected copy instruction"), }; - if memory_utils::may_alias(context, value, len, dest_ptr, copy_size) { + // If we don't know the copy size or there's a possible alias, + // we decide that this copy won't be available. + if len.is_none() + || copy_size.is_none() + || memory_utils::may_alias( + context, + value, + len.unwrap(), + dest_ptr, + copy_size.unwrap(), + ) + { available_copies.remove(copy); } } @@ -380,7 +403,7 @@ fn local_copy_prop( } // Deconstruct a memcpy into (dst_val_ptr, src_val_ptr, copy_len). - fn deconstruct_memcpy(context: &Context, inst: Value) -> (Value, Value, u64) { + fn deconstruct_memcpy(context: &Context, inst: Value) -> (Value, Value, Option) { match inst.get_instruction(context).unwrap() { Instruction { op: @@ -390,7 +413,11 @@ fn local_copy_prop( byte_len, }, .. - } => (*dst_val_ptr, *src_val_ptr, *byte_len), + } => ( + *dst_val_ptr, + *src_val_ptr, + byte_len.get_constant(context).and_then(|c| c.as_uint()), + ), Instruction { op: InstOp::MemCopyVal { @@ -401,7 +428,7 @@ fn local_copy_prop( } => ( *dst_val_ptr, *src_val_ptr, - memory_utils::pointee_size(context, *dst_val_ptr), + Some(memory_utils::pointee_size(context, *dst_val_ptr)), ), _ => unreachable!("Only memcpy instructions handled"), } @@ -444,13 +471,15 @@ fn local_copy_prop( // matches. This isn't really needed as the copy happens and the // data we want is safe to access. But we just don't know how to // generate the right GEP always. So that's left for another day. - if memory_utils::must_alias( - context, - src_val_ptr, - memory_utils::pointee_size(context, src_val_ptr), - dst_ptr_memcpy, - copy_len, - ) { + if copy_len.is_some_and(|copy_len| { + memory_utils::must_alias( + context, + src_val_ptr, + memory_utils::pointee_size(context, src_val_ptr), + dst_ptr_memcpy, + copy_len, + ) + }) { // Replace src_val_ptr with src_ptr_memcpy. if src_val_ptr.get_type(context) == src_ptr_memcpy.get_type(context) { replacements.insert(inst, Replacement::OldGep(src_ptr_memcpy)); @@ -473,7 +502,9 @@ fn local_copy_prop( .get_pointee_type(context) .unwrap(); if memcpy_src_sym_type == memcpy_dst_sym_type - && memcpy_dst_sym_type.size(context).in_bytes() == copy_len + && copy_len.is_some_and(|copy_len| { + memcpy_dst_sym_type.size(context).in_bytes() == copy_len + }) { replacements.insert( inst, @@ -532,7 +563,7 @@ fn local_copy_prop( kill_defined_symbol( context, *arg, - max_size, + Some(max_size), available_copies, src_to_copies, dest_to_copies, @@ -643,7 +674,7 @@ fn local_copy_prop( kill_defined_symbol( context, *dst_val_ptr, - memory_utils::pointee_size(context, *dst_val_ptr), + Some(memory_utils::pointee_size(context, *dst_val_ptr)), &mut available_copies, &mut src_to_copies, &mut dest_to_copies, diff --git a/sway-ir/src/parser.rs b/sway-ir/src/parser.rs index caa439c3fb7..e020e15b65c 100644 --- a/sway-ir/src/parser.rs +++ b/sway-ir/src/parser.rs @@ -357,7 +357,7 @@ mod ir_builder { } rule op_mem_copy_bytes() -> IrAstOperation - = "mem_copy_bytes" _ dst_name:id() comma() src_name:id() comma() len:decimal() { + = "mem_copy_bytes" _ dst_name:id() comma() src_name:id() comma() len:id() { IrAstOperation::MemCopyBytes(dst_name, src_name, len) } @@ -753,7 +753,7 @@ mod ir_builder { IntToPtr(String, IrAstTy), Load(String), Log(IrAstTy, String, String), - MemCopyBytes(String, String, u64), + MemCopyBytes(String, String, String), MemCopyVal(String, String), Nop, PtrToInt(String, IrAstTy), @@ -1343,7 +1343,7 @@ mod ir_builder { .mem_copy_bytes( *val_map.get(&dst_name).unwrap(), *val_map.get(&src_name).unwrap(), - len, + *val_map.get(&len).unwrap(), ) .add_metadatum(context, opt_metadata), IrAstOperation::MemCopyVal(dst_name, src_name) => block diff --git a/sway-ir/src/printer.rs b/sway-ir/src/printer.rs index 2478b3ef4f9..cb10326aea6 100644 --- a/sway-ir/src/printer.rs +++ b/sway-ir/src/printer.rs @@ -998,7 +998,7 @@ fn instruction_to_doc<'a>( "mem_copy_bytes {}, {}, {}", namer.name(context, dst_val_ptr), namer.name(context, src_val_ptr), - byte_len, + namer.name(context, byte_len), )) .append(md_namer.md_idx_to_doc(context, metadata)), ), diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index f5bbb39cea9..a1f2a0da6d4 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -333,8 +333,8 @@ impl<'a, 'eng> InstructionVerifier<'a, 'eng> { InstOp::MemCopyBytes { dst_val_ptr, src_val_ptr, - byte_len, - } => self.verify_mem_copy_bytes(dst_val_ptr, src_val_ptr, byte_len)?, + byte_len: _, + } => self.verify_mem_copy_bytes(dst_val_ptr, src_val_ptr)?, InstOp::MemCopyVal { dst_val_ptr, src_val_ptr, @@ -892,7 +892,6 @@ impl<'a, 'eng> InstructionVerifier<'a, 'eng> { &self, dst_val_ptr: &Value, src_val_ptr: &Value, - _byte_len: &u64, ) -> Result<(), IrError> { // Just confirm both values are pointers. self.get_ptr_type(dst_val_ptr, IrError::VerifyMemcopyNonPointer) From a2d7f76188513dfed5de672e4cb9441785f80bf0 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Thu, 5 Dec 2024 21:10:23 +0530 Subject: [PATCH 14/19] fix printing of new MemCopyBytes --- sway-ir/src/printer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sway-ir/src/printer.rs b/sway-ir/src/printer.rs index d87020523a1..12845db14f9 100644 --- a/sway-ir/src/printer.rs +++ b/sway-ir/src/printer.rs @@ -993,7 +993,7 @@ fn instruction_to_doc<'a>( dst_val_ptr, src_val_ptr, byte_len, - } => Doc::line( + } => maybe_constant_to_doc(context, md_namer, namer, byte_len).append(Doc::line( Doc::text(format!( "mem_copy_bytes {}, {}, {}", namer.name(context, dst_val_ptr), @@ -1001,7 +1001,7 @@ fn instruction_to_doc<'a>( namer.name(context, byte_len), )) .append(md_namer.md_idx_to_doc(context, metadata)), - ), + )), InstOp::MemCopyVal { dst_val_ptr, src_val_ptr, From be7759aa35ff7278119771ddff7555fecdb3165c Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 6 Dec 2024 11:50:06 +0530 Subject: [PATCH 15/19] fix bug in ir generation changes --- sway-core/src/ir_generation/function.rs | 8 ++++---- sway-core/src/lib.rs | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs index 8fc2725b63d..c4193f7366a 100644 --- a/sway-core/src/ir_generation/function.rs +++ b/sway-core/src/ir_generation/function.rs @@ -1726,10 +1726,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); - let len = Constant::get_uint(context, 64, 8 - offset); + let len_const = Constant::get_uint(context, 64, 8 - offset); s.current_block .append(context) - .mem_copy_bytes(addr, item_ptr, len); + .mem_copy_bytes(addr, item_ptr, len_const); Ok(increase_len(&mut s.current_block, context, len, 8 - offset)) } @@ -1998,10 +1998,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); - let len = Constant::get_uint(context, 64, 32); + let len_32 = Constant::get_uint(context, 64, 32); self.current_block .append(context) - .mem_copy_bytes(addr, item_ptr, len); + .mem_copy_bytes(addr, item_ptr, len_32); increase_len(&mut self.current_block, context, len, 32) } TypeInfo::StringArray(string_len) => { diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index f79336f27cb..4d671c323d0 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -45,7 +45,10 @@ use sway_ast::AttributeDecl; use sway_error::handler::{ErrorEmitted, Handler}; use sway_features::ExperimentalFeatures; use sway_ir::{ - create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, FN_DCE_NAME, FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_DEMONOMORPHIZE_NAME, FN_INLINE_NAME, MEM2REG_NAME, MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME + create_o1_pass_group, register_known_passes, Context, Kind, Module, PassGroup, PassManager, + PrintPassesOpts, ARG_DEMOTION_NAME, CONST_DEMOTION_NAME, DCE_NAME, FN_DCE_NAME, + FN_DEDUP_DEBUG_PROFILE_NAME, FN_DEDUP_DEMONOMORPHIZE_NAME, FN_INLINE_NAME, MEM2REG_NAME, + MEMCPYOPT_NAME, MISC_DEMOTION_NAME, RET_DEMOTION_NAME, SIMPLIFY_CFG_NAME, SROA_NAME, }; use sway_types::constants::DOC_COMMENT_ATTRIBUTE_NAME; use sway_types::SourceEngine; From 3ad36cdc0e208f1f3c77b8dd2427cc3c945db962 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 6 Dec 2024 12:23:09 +0530 Subject: [PATCH 16/19] fix bug missed in previous bugfix --- sway-core/src/ir_generation/function.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs index c4193f7366a..9ff6d468ecf 100644 --- a/sway-core/src/ir_generation/function.rs +++ b/sway-core/src/ir_generation/function.rs @@ -2014,10 +2014,10 @@ impl<'eng> FnCompiler<'eng> { len, Type::get_uint8(context), ); - let len = Constant::get_uint(context, 64, string_len.val() as u64); + let len_const = Constant::get_uint(context, 64, string_len.val() as u64); self.current_block .append(context) - .mem_copy_bytes(addr, item_ptr, len); + .mem_copy_bytes(addr, item_ptr, len_const); increase_len( &mut self.current_block, context, From da757d217f54ab890161e3c3218717656cf91d90 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 13 Dec 2024 09:04:04 +0530 Subject: [PATCH 17/19] some bugfixes --- sway-core/src/lib.rs | 1 + sway-ir/src/function.rs | 5 + sway-ir/src/optimize/fn_dedup.rs | 193 ++++++++++++++++++++++++++++--- sway-ir/src/verify.rs | 13 ++- 4 files changed, 193 insertions(+), 19 deletions(-) diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index 4d671c323d0..87a06f82d35 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -950,6 +950,7 @@ pub(crate) fn compile_ast_to_ir_to_asm( pass_group.append_pass(MEM2REG_NAME); pass_group.append_pass(DCE_NAME); pass_group.append_pass(FN_DEDUP_DEMONOMORPHIZE_NAME); + pass_group.append_pass(DCE_NAME); pass_group.append_pass(FN_DCE_NAME); } OptLevel::Opt0 => {} diff --git a/sway-ir/src/function.rs b/sway-ir/src/function.rs index cedfee85930..a478a723fe5 100644 --- a/sway-ir/src/function.rs +++ b/sway-ir/src/function.rs @@ -358,6 +358,11 @@ impl Function { .copied() } + /// Get the i'th arg value + pub fn get_ith_arg(&self, context: &Context, i: usize) -> Value { + context.functions[self.0].arguments[i].1 + } + /// Append an extra argument to the function signature. /// /// NOTE: `arg` must be a `BlockArgument` value with the correct index otherwise `add_arg` will diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index 5e98ba38844..b7b8d82d106 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -478,6 +478,7 @@ fn dedup_fn_demonomorphize( struct OthersTracker<'a> { locals_iter: Box + 'a>, instr_iter: Box + 'a>, + args_iter: Box + 'a>, } let mut class_iter = class.iter(); let leader = class_iter.next().unwrap(); @@ -488,11 +489,55 @@ fn dedup_fn_demonomorphize( OthersTracker { locals_iter: Box::new(f.locals_iter(context)), instr_iter: Box::new(f.instruction_iter(context)), + args_iter: Box::new(f.args_iter(context).cloned()), }, ) }) .collect(); + // Note down arguments and retun value that need to be type erased. + let mut type_erase_args = vec![]; + let mut type_erase_ret = false; + for (arg_idx, arg) in leader.args_iter(context).enumerate() { + let ty = arg.1.get_type(context).unwrap(); + if !ty.is_ptr(context) { + continue; + } + for other_func in others.iter_mut() { + let other_arg = other_func.1.args_iter.next().unwrap(); + let other_arg_ty = other_arg.1.get_type(context).unwrap(); + assert!( + other_arg_ty.is_ptr(context), + "Functions wouldn't be in the same class if args differ" + ); + if ty.get_pointee_type(context).unwrap() + != other_arg_ty.get_pointee_type(context).unwrap() + { + type_erase_args.push(arg_idx); + break; + } + } + } + + let ret_ty = leader.get_return_type(context); + let mut ret_ty_map = FxHashMap::default(); + ret_ty_map.insert(*leader, ret_ty); + if ret_ty.is_ptr(context) { + for other_func in others.iter_mut() { + let other_ret_ty = other_func.0.get_return_type(context); + ret_ty_map.insert(*other_func.0, other_ret_ty); + assert!( + other_ret_ty.is_ptr(context), + "Function't wouldn't be in the same class if ret type differs" + ); + if ret_ty.get_pointee_type(context).unwrap() + != other_ret_ty.get_pointee_type(context).unwrap() + { + type_erase_ret = true; + } + } + } + // Collect those locals that need to be shifted to an argument. // The key is a local from the leader and the value is a list of // corresponding locals from others in the class. @@ -537,10 +582,36 @@ fn dedup_fn_demonomorphize( mem_copy_val: FxHashMap>, } + let mut type_erase_block_args = FxHashSet::default(); let mut change_instrs = ChangeInstrs::default(); - 'leader_loop: for (_, inst) in leader.instruction_iter(context) { + 'leader_loop: for (block, inst) in leader.instruction_iter(context) { + let mut block_args_checked = false; for other_func in others.iter_mut() { - let (_other_block, other_instr) = other_func.1.instr_iter.next().unwrap(); + let (other_block, other_instr) = other_func.1.instr_iter.next().unwrap(); + // Check if any of the block args (except for the entry block) need their type erased. + if !block_args_checked && leader.get_entry_block(context) != block { + block_args_checked = true; + for (arg_idx, arg) in block.arg_iter(context).enumerate() { + let ty = arg.get_type(context).unwrap(); + if !ty.is_ptr(context) { + continue; + } + let other_ty = other_block + .get_arg(context, arg_idx) + .unwrap() + .get_type(context) + .unwrap(); + assert!( + other_ty.is_ptr(context), + "If this isn't a pointer, functions shouldn't be in same class" + ); + if ty.get_pointee_type(context).unwrap() + != other_ty.get_pointee_type(context).unwrap() + { + type_erase_block_args.insert(*arg); + } + } + } // Throughout this loop we check only for differing types between the leader and // its followers. Other differences aren't checked for because then the hashes would // be different and they wouldn't be in the same class. @@ -777,7 +848,11 @@ fn dedup_fn_demonomorphize( dst_val_ptr, src_val_ptr: _, } => { - let copied_ty = dst_val_ptr.get_type(context).unwrap(); + let copied_ty = dst_val_ptr + .get_type(context) + .unwrap() + .get_pointee_type(context) + .unwrap(); let size_copied_type_bytes = copied_ty.size(context).in_bytes(); this_params.push(size_copied_type_bytes); } @@ -818,10 +893,24 @@ fn dedup_fn_demonomorphize( } // We are now equipped to actually modify the program. - // 1. Add the new arguments. + // 1(a) Type erase existing arguments / return type if necessary + for arg_idx in &type_erase_args { + let arg_val = leader.get_ith_arg(context, *arg_idx); + let arg = arg_val.get_argument_mut(context).unwrap(); + arg.ty = unit_ptr_ty; + } + for block_arg in type_erase_block_args { + let arg = block_arg.get_argument_mut(context).unwrap(); + arg.ty = unit_ptr_ty; + } + if type_erase_ret { + leader.set_return_type(context, unit_ptr_ty); + } + + // 1(b) Add the new arguments. let mut new_arg_values = Vec::with_capacity(new_args[0].len()); let entry_block = leader.get_entry_block(context); - for new_arg in &new_args[0] { + for (arg_idx, new_arg) in (&new_args[0]).iter().enumerate() { let (new_block_arg, new_arg_name) = match new_arg { NewArg::CallerAllocatedLocal(..) => ( BlockArgument { @@ -829,7 +918,7 @@ fn dedup_fn_demonomorphize( idx: leader.num_args(context), ty: unit_ptr_ty, }, - "demonomorphize_alloca_arg", + "demonomorphize_alloca_arg_".to_string() + &arg_idx.to_string(), ), NewArg::Size(_) => ( BlockArgument { @@ -837,7 +926,7 @@ fn dedup_fn_demonomorphize( idx: leader.num_args(context), ty: Type::get_uint64(context), }, - "demonomorphize_size_arg", + "demonomorphize_size_arg_".to_string() + &arg_idx.to_string(), ), }; let new_arg_value = Value::new_argument(context, new_block_arg); @@ -936,12 +1025,41 @@ fn dedup_fn_demonomorphize( // 6. Finally modify calls to each function in the class. for (caller, call_block, call_inst) in call_sites { // Update the callee in call_inst first, all calls go to the leader now. - let InstOp::Call(callee, _) = &mut call_inst.get_instruction_mut(context).unwrap().op + let (callee, params) = { + let InstOp::Call(callee, params) = + &mut call_inst.get_instruction_mut(context).unwrap().op + else { + panic!("Expected Call"); + }; + let original_callee = *callee; + *callee = *leader; + (original_callee, params) + }; + + // Update existing params to erase type, if necessary. + let mut new_params = params.clone(); + let mut new_instrs = vec![]; + for arg_idx in &type_erase_args { + let new_param = Value::new_instruction( + context, + call_block, + InstOp::CastPtr(new_params[*arg_idx], unit_ptr_ty), + ); + new_instrs.push(new_param); + new_params[*arg_idx] = new_param; + } + let mut inserter = InstructionInserter::new( + context, + call_block, + crate::InsertionPosition::Before(call_inst), + ); + inserter.insert_slice(&new_instrs); + let InstOp::Call(_callee, params) = + &mut call_inst.get_instruction_mut(context).unwrap().op else { panic!("Expected Call"); }; - *callee = *leader; - let callee = *callee; + *params = new_params; // Now add the new args. let callee_idx = class_fn_to_idx[&callee]; @@ -957,22 +1075,35 @@ fn dedup_fn_demonomorphize( let new_local = caller.new_unique_local_var( context, name, - original_local.get_type(context), + original_local + .get_type(context) + .get_pointee_type(context) + .unwrap(), original_local.get_initializer(context).cloned(), original_local.is_mutable(context), ); - let inserter = InstructionInserter::new( + let new_local_ptr = Value::new_instruction( + context, + call_block, + InstOp::GetLocal(new_local), + ); + let new_local_ptr_casted = Value::new_instruction( + context, + call_block, + InstOp::CastPtr(new_local_ptr, unit_ptr_ty), + ); + let mut inserter = InstructionInserter::new( context, call_block, crate::InsertionPosition::Before(call_inst), ); - let new_local_ptr = inserter.get_local(new_local); + inserter.insert_slice(&[new_local_ptr, new_local_ptr_casted]); let InstOp::Call(_, args) = &mut call_inst.get_instruction_mut(context).unwrap().op else { panic!("Expected Call"); }; - args.push(new_local_ptr); + args.push(new_local_ptr_casted); } NewArg::Size(val) => { let new_size_const = Constant::new_uint(context, 64, *val); @@ -986,6 +1117,40 @@ fn dedup_fn_demonomorphize( } } } + if type_erase_ret { + let inserter = InstructionInserter::new( + context, + call_block, + crate::InsertionPosition::After(call_inst), + ); + let ret_cast = inserter.cast_ptr(call_inst, ret_ty_map[&callee]); + caller.replace_value(context, call_inst, ret_cast, None); + // caller.replace_value will replace call_inst in the just inserted cast. Fix it. + let Instruction { + op: InstOp::CastPtr(ptr, _), + .. + } = ret_cast.get_instruction_mut(context).unwrap() + else { + panic!("We just created this to be a Castptr"); + }; + *ptr = call_inst; + + // Modify all return instructions + for (_, ret) in leader + .instruction_iter(context) + .filter(|inst| { + matches!(inst.1.get_instruction(context).unwrap().op, InstOp::Ret(..)) + }) + .collect::>() + { + let InstOp::Ret(__entry, ty) = + &mut ret.get_instruction_mut(context).unwrap().op + else { + panic!("We just filtered for Rets") + }; + *ty = unit_ptr_ty; + } + } } } diff --git a/sway-ir/src/verify.rs b/sway-ir/src/verify.rs index a1f2a0da6d4..57caa009865 100644 --- a/sway-ir/src/verify.rs +++ b/sway-ir/src/verify.rs @@ -524,11 +524,14 @@ impl<'a, 'eng> InstructionVerifier<'a, 'eng> { return Err(IrError::VerifyBinaryOpIncorrectArgType); } } - BinaryOpKind::Add - | BinaryOpKind::Sub - | BinaryOpKind::Mul - | BinaryOpKind::Div - | BinaryOpKind::Mod => { + BinaryOpKind::Add => { + if !arg1_ty.eq(self.context, &arg2_ty) || !arg1_ty.is_uint(self.context) { + if !(arg1_ty.is_ptr(self.context) && arg2_ty.is_uint64(self.context)) { + return Err(IrError::VerifyBinaryOpIncorrectArgType); + } + } + } + BinaryOpKind::Sub | BinaryOpKind::Mul | BinaryOpKind::Div | BinaryOpKind::Mod => { if !arg1_ty.eq(self.context, &arg2_ty) || !arg1_ty.is_uint(self.context) { return Err(IrError::VerifyBinaryOpIncorrectArgType); } From 1326792b3f5eb9dd24936ef7800657eadaa6a54c Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 13 Dec 2024 11:13:12 +0530 Subject: [PATCH 18/19] bug fixes and test updates --- sway-ir/src/optimize/fn_dedup.rs | 6 ++-- .../json_abi_oracle_new_encoding.json | 32 +++++++++---------- .../json_abi_oracle_new_encoding.json | 2 +- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index b7b8d82d106..6cb4d4c7f1f 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -478,7 +478,6 @@ fn dedup_fn_demonomorphize( struct OthersTracker<'a> { locals_iter: Box + 'a>, instr_iter: Box + 'a>, - args_iter: Box + 'a>, } let mut class_iter = class.iter(); let leader = class_iter.next().unwrap(); @@ -489,7 +488,6 @@ fn dedup_fn_demonomorphize( OthersTracker { locals_iter: Box::new(f.locals_iter(context)), instr_iter: Box::new(f.instruction_iter(context)), - args_iter: Box::new(f.args_iter(context).cloned()), }, ) }) @@ -504,8 +502,8 @@ fn dedup_fn_demonomorphize( continue; } for other_func in others.iter_mut() { - let other_arg = other_func.1.args_iter.next().unwrap(); - let other_arg_ty = other_arg.1.get_type(context).unwrap(); + let other_arg = other_func.0.get_ith_arg(context, arg_idx); + let other_arg_ty = other_arg.get_type(context).unwrap(); assert!( other_arg_ty.is_ptr(context), "Functions wouldn't be in the same class if args differ" diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json index 71cf1f22ff7..d8195619956 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json @@ -62,82 +62,82 @@ { "concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903", "name": "BOOL", - "offset": 7048 + "offset": 6704 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "U8", - "offset": 7240 + "offset": 6896 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "ANOTHER_U8", - "offset": 6976 + "offset": 6632 }, { "concreteTypeId": "29881aad8730c5ab11d275376323d8e4ff4179aae8ccb6c13fe4902137e162ef", "name": "U16", - "offset": 7184 + "offset": 6840 }, { "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc", "name": "U32", - "offset": 7224 + "offset": 6880 }, { "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc", "name": "U64", - "offset": 7232 + "offset": 6888 }, { "concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e", "name": "U256", - "offset": 7192 + "offset": 6848 }, { "concreteTypeId": "7c5ee1cecf5f8eacd1284feb5f0bf2bdea533a51e2f0c9aabe9236d335989f3b", "name": "B256", - "offset": 7016 + "offset": 6672 }, { "concreteTypeId": "81fc10c4681a3271cf2d66b2ec6fbc8ed007a442652930844fcf11818c295bff", "name": "CONFIGURABLE_STRUCT", - "offset": 7136 + "offset": 6792 }, { "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c", "name": "CONFIGURABLE_ENUM_A", - "offset": 7056 + "offset": 6712 }, { "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c", "name": "CONFIGURABLE_ENUM_B", - "offset": 7096 + "offset": 6752 }, { "concreteTypeId": "4926d35d1a5157936b0a29bc126b8aace6d911209a5c130e9b716b0c73643ea6", "name": "ARRAY_BOOL", - "offset": 6984 + "offset": 6640 }, { "concreteTypeId": "776fb5a3824169d6736138565fdc20aad684d9111266a5ff6d5c675280b7e199", "name": "ARRAY_U64", - "offset": 6992 + "offset": 6648 }, { "concreteTypeId": "c998ca9a5f221fe7b5c66ae70c8a9562b86d964408b00d17f883c906bc1fe4be", "name": "TUPLE_BOOL_U64", - "offset": 7168 + "offset": 6824 }, { "concreteTypeId": "94f0fa95c830be5e4f711963e83259fe7e8bc723278ab6ec34449e791a99b53a", "name": "STR_4", - "offset": 7160 + "offset": 6816 }, { "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b", "name": "NOT_USED", - "offset": 7152 + "offset": 6808 } ], "encodingVersion": "1", diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json index c58838960ad..3246e38ae49 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/u256/u256_abi/json_abi_oracle_new_encoding.json @@ -9,7 +9,7 @@ { "concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e", "name": "SOME_U256", - "offset": 872 + "offset": 760 } ], "encodingVersion": "1", From f6c4caa5c7eeb7f2c255625bca124e5fe73ff5e7 Mon Sep 17 00:00:00 2001 From: Vaivaswatha Nagaraj Date: Fri, 13 Dec 2024 14:26:18 +0530 Subject: [PATCH 19/19] another bugfix --- sway-ir/src/optimize/fn_dedup.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs index 6cb4d4c7f1f..e322bf78c6c 100644 --- a/sway-ir/src/optimize/fn_dedup.rs +++ b/sway-ir/src/optimize/fn_dedup.rs @@ -706,7 +706,12 @@ fn dedup_fn_demonomorphize( .or_insert(vec![other_instr]); } } - InstOp::Load(_value) => {} + InstOp::Load(_value) => { + if inst.get_type(context) != other_instr.get_type(context) { + can_optimize = false; + break 'leader_loop; + } + } InstOp::MemCopyBytes { .. } => {} InstOp::MemCopyVal { dst_val_ptr, .. } => { let InstOp::MemCopyVal { @@ -729,7 +734,19 @@ fn dedup_fn_demonomorphize( InstOp::Nop => {} InstOp::PtrToInt(..) => {} InstOp::Ret(..) => {} - InstOp::Store { .. } => {} + InstOp::Store { stored_val, .. } => { + let InstOp::Store { + stored_val: other_stored_val, + .. + } = &other_instr.get_instruction(context).unwrap().op + else { + panic!("Leader and follower are different instructions in same class"); + }; + if stored_val.get_type(context) != other_stored_val.get_type(context) { + can_optimize = false; + break 'leader_loop; + } + } } } }