chore: add comment for gvn and lcm pass

compilers/rustyc/optimizer/README.md

@@ -0,0 +1,18 @@
+# RustyC IR 
+
+This crate contain IR definition of RustyC, IR Converter and Optimizer.
+
+
+### IR Structure
+
+
+
+### Test Runner Structure
+
+Since we need traversal all folder for reading C code of test case, i create a custom test runner that
+can traverse folder and test all test structure according to folder structure.
+
+In the other hand, because some of test case need using function to create IR for optimizer, IR optimizer
+test runner still using rust build-in test runner.
+
+

compilers/rustyc/optimizer/src/ir_optimizer/pass/gvn/mod.rs

@@ -16,8 +16,14 @@ use std::collections::HashMap;
 
 pub struct GVNPass<'a> {
     dom_table: &'a DomTable,
+    /// Replace table is used to replace right hand side operand.
+    /// - if key is exist in table, mean value have same value as key
     replaceable_value_table: ScopeReplaceValueCacheTable,
+    /// Cache table is used to replace entire right hand side as a value.
+    /// - if expr key is exist in table, mean entire inst could be rewrite
+    ///   as a assignment expr.
     cache_inst_table: ScopeInstCacheTable,
+    /// Post process cache to reomve inst.
     need_remove_insts: Vec<(BasicBlock, Instruction)>,
 }
 
@@ -37,6 +43,11 @@ impl<'a> GVNPass<'a> {
             need_remove_insts: Default::default(),
         }
     }
+    /// ## Main Algorithm of GVN
+    /// - for phi : remove it if meanless and redundant
+    /// - for other inst: remove it if redundant.
+    /// 1. A inst is redundant if and only if it's ExprKey is already computed.
+    /// 2. A meanless phi mean all the src value is same, like `a = phi b b1, b b2, b b3`.
     fn visit_block(
         &mut self,
         function: &mut Function,
@@ -71,11 +82,17 @@ impl<'a> GVNPass<'a> {
         self.replaceable_value_table.exit_scope();
         self.cache_inst_table.exit_scope();
     }
+    /// ## Post-process to remove inst
+    /// Because ownership problem, we can not remove inst while borrow it, we need
+    /// remove inst after gvn traversal.
     fn remove_redundant_insts(&mut self, function: &mut Function) {
         for (block_id, inst) in &self.need_remove_insts {
             function.remove_inst_from_block(&block_id, &inst);
         }
     }
+    /// ## Rewrite phi is meanless or redundant,
+    /// - return true when meanless and redundant.
+    /// - otherwise return false.
     fn rewrite_phi_by_cache_table(&mut self, dst: &Value, from: &Vec<(BasicBlock, Value)>) -> bool {
         // check is meanless
         let last_value = &from[0].1;
@@ -112,15 +129,15 @@ impl<'a> GVNPass<'a> {
         for inst in &successor_block_data.instructions {
             let inst_data = function.instructions.get_mut(inst).unwrap();
             if let InstructionData::Phi { from, .. } = inst_data {
-                self.rewrite_phi_operand_by_replaceable_table(from);
+                for (_, value) in from {
+                    self.rewrite_value_by_replaceable_table(value)
+                }
             }
         }
     }
-    fn rewrite_phi_operand_by_replaceable_table(&mut self, from: &mut Vec<(BasicBlock, Value)>) {
-        for (_, value) in from {
-            self.rewrite_value_by_replaceable_table(value)
-        }
-    }
+    /// ## Rewrite Inst by Cache Table
+    /// - return true if given inst need to remove
+    /// - otherwise, return false.
     fn rewrite_inst_by_cache_table(&mut self, inst_data: &InstructionData) -> bool {
         match get_right_hand_side_inst_key(inst_data) {
             Some((key, dst_value)) => {
@@ -136,6 +153,7 @@ impl<'a> GVNPass<'a> {
             _ => return false,
         }
     }
+    /// Wrapper of `rewrite_value_by_replaceable_table`, just unwind the inst values and dist.
     fn rewrite_inst_operand_by_replaceable_table(&mut self, inst_data: &mut InstructionData) {
         match inst_data {
             InstructionData::Add { src1, src2, .. }
@@ -177,6 +195,8 @@ impl<'a> GVNPass<'a> {
             _ => {}
         }
     }
+    /// ## Try to over write a value by replace table
+    /// if the given value exist in replaceable table, overwrite that value by table value.
     fn rewrite_value_by_replaceable_table(&mut self, value: &mut Value) {
         if let Some(replace_able_value) = self.replaceable_value_table.get(value) {
             *value = replace_able_value.clone();

compilers/rustyc/optimizer/src/ir_optimizer/pass/gvn/scope_table.rs

@@ -5,11 +5,14 @@ use crate::ir_optimizer::anaylsis::dfs_ordering::DFSOrdering;
 use crate::ir_optimizer::anaylsis::domtree::DomTable;
 use std::collections::HashMap;
 
+/// ## Scope Inst Cache Table
+/// cache table is used to store the expr-key map to the value that has already
+/// computed the inst right hand side.
+/// - scope means that we could enter and exit by dom-tree relationship
 pub struct ScopeInstCacheTable {
     current_index: usize,
     table: Vec<HashMap<RightHandSideInst, Value>>,
 }
-
 impl ScopeInstCacheTable {
     pub fn new() -> Self {
         Self {
@@ -38,7 +41,16 @@ impl ScopeInstCacheTable {
         self.table.pop();
     }
 }
-
+/// ## Scope Replace Value Cache Table
+/// replace table is used to store the value that is can be replaced by other value,
+/// for exapmple:
+/// ```ir
+/// a = b * c
+/// d = b * c
+/// e = d + d
+/// ```
+/// when d is rewrited by cache table to `d = a`, that for following inst `e = d + d`,
+/// d should be replace by a to remove the duplicate check for equal value  d = a.
 pub struct ScopeReplaceValueCacheTable {
     current_index: usize,
     table: Vec<HashMap<Value, Value>>,

compilers/rustyc/optimizer/src/ir_optimizer/pass/lcm/expr_key.rs

@@ -142,17 +142,24 @@ pub fn get_dst_value(instruction: &InstructionData) -> Option<Value> {
         _ => None,
     }
 }
-
+/// ## ExprKeyManager: Manage ExprKey for us.
+///
 pub(super) struct ExprKeyManager {
-    // values number set.
+    // cache set of all expr key
     all_expr_value_number: ExprValueNumberSet,
-    // 1 to 1 mapping
+    // 1-to-1 mapping of value number and exprkey
     value_number_map_expr_key: HashMap<ExprValueNumber, ExpreKey>,
+    // revserse mapping of expr key to value number.
     expr_key_map_value_number: HashMap<ExpreKey, ExprValueNumber>,
-
+    /// Mapping relation of ExprKey and Inst, ExprKey is constructed by right
+    /// hand side of inst, so multi inst might have same key, but for inst it
+    /// -self, it would only have a value number.
     inst_map_value_number: HashMap<Instruction, ExprValueNumber>,
+    /// Mapping of relation of ExprKey to it's inst, since ExprKey only take
+    /// right hand side to construct key, a key could map to multi inst.
     value_number_map_inst: HashMap<ExprValueNumber, HashSet<Instruction>>,
-
+    /// Mapping a dst value to the ExprKey it kill, used for build kill set
+    /// of LCM.
     ir_value_map_kill_exprs: HashMap<Value, ExprValueNumberSet>,
 }
 
@@ -207,21 +214,29 @@ impl ExprKeyManager {
             }
         }
     }
-    pub fn inst_get_expr_value_number(
+    /// ## Get expr value number from inst id
+    /// This function is used to build `expr_use` set of LCM.
+    pub fn get_expr_value_number_from_inst(
         &self,
         instruction: &Instruction,
     ) -> Option<&ExprValueNumber> {
         self.inst_map_value_number.get(instruction)
     }
-    pub fn dst_value_get_kill_expr_value_numbers(
+    /// ## Get expr value numbers from value (dst)
+    /// This function is used to build `expr_kil` set of LCM.
+    pub fn get_kill_expr_value_numbers_from_dst_value(
         &self,
         value: &Value,
     ) -> Option<&HashSet<ExprValueNumber>> {
         self.ir_value_map_kill_exprs.get(value)
     }
+    /// ## Get entire value number set
+    /// This function is used by every fixed point algorithm
+    /// to get the init set.
     pub fn get_value_number_set(&self) -> ExprValueNumberSet {
         self.all_expr_value_number.clone()
     }
+    /// ##
     pub fn get_expr_key_from_value_number(
         &self,
         value_number: &ExprValueNumber,

compilers/rustyc/optimizer/src/ir_optimizer/pass/lcm/pre_proc.rs

@@ -17,27 +17,26 @@ impl LCMPass {
         for (block_id, block_data) in &function.blocks {
             let mut expr_use = HashSet::new();
             for inst in &block_data.instructions {
-                if let Some(val_number) = self.key_manager.inst_get_expr_value_number(inst) {
+                if let Some(val_number) = self.key_manager.get_expr_value_number_from_inst(inst) {
                     expr_use.insert(val_number.clone());
                 }
             }
             self.expression_use.insert(block_id.clone(), expr_use);
-
-            for (block_id, block_data) in &function.blocks {
-                let mut expr_kill = HashSet::new();
-                for inst in &block_data.instructions {
-                    let inst_data = function.instructions.get(inst).unwrap();
-                    if let Some(value) = get_dst_value(inst_data) {
-                        if let Some(kill_set) = self
-                            .key_manager
-                            .dst_value_get_kill_expr_value_numbers(&value)
-                        {
-                            expr_kill.extend(kill_set.into_iter());
-                        }
+        }
+        for (block_id, block_data) in &function.blocks {
+            let mut expr_kill = HashSet::new();
+            for inst in &block_data.instructions {
+                let inst_data = function.instructions.get(inst).unwrap();
+                if let Some(value) = get_dst_value(inst_data) {
+                    if let Some(kill_set) = self
+                        .key_manager
+                        .get_kill_expr_value_numbers_from_dst_value(&value)
+                    {
+                        expr_kill.extend(kill_set.into_iter());
                     }
                 }
-                self.expression_kill.insert(block_id.clone(), expr_kill);
             }
+            self.expression_kill.insert(block_id.clone(), expr_kill);
         }
     }
     fn build_dfs_ordering(&mut self, function: &Function) {