From 731278aad86d4ee4fddc2f54f0e3ec1fb32be5a3 Mon Sep 17 00:00:00 2001
From: Denis Merigoux <denis.merigoux@gmail.com>
Date: Fri, 9 Jun 2017 16:20:38 -0700
Subject: [PATCH] Improved DFG API (#95)

* Improved DFG API with swap_remove_ebb_args and append_inst_arg
* Implemented EntityList::swap_remove
And used it for dfg::swap_remove_ebb_arg
---
 lib/cretonne/src/entity_list.rs | 18 ++++++++-
 lib/cretonne/src/ir/dfg.rs      | 66 ++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/lib/cretonne/src/entity_list.rs b/lib/cretonne/src/entity_list.rs
index 946bb7ba04..1ccddc3aed 100644
--- a/lib/cretonne/src/entity_list.rs
+++ b/lib/cretonne/src/entity_list.rs
@@ -416,7 +416,7 @@ impl<T: EntityRef> EntityList<T> {
         }
     }
 
-    /// Removes the element at position `index` from the list.
+    /// Removes the element at position `index` from the list. Potentially linear complexity.
     pub fn remove(&mut self, index: usize, pool: &mut ListPool<T>) {
         let len;
         {
@@ -448,6 +448,22 @@ impl<T: EntityRef> EntityList<T> {
         pool.data[block] = T::new(len - 1);
     }
 
+    /// Removes the element at `index` in constant time by switching it with the last element of
+    /// the list.
+    pub fn swap_remove(&mut self, index: usize, pool: &mut ListPool<T>) {
+        let len = self.len(pool);
+        assert!(index < len);
+        if index == len - 1 {
+            self.remove(index, pool);
+        } else {
+            {
+                let seq = self.as_mut_slice(pool);
+                seq.swap(index, len - 1);
+            }
+            self.remove(len - 1, pool);
+        }
+    }
+
     /// Grow the list by inserting `count` elements at `index`.
     ///
     /// The new elements are not initialized, they will contain whatever happened to be in memory.
diff --git a/lib/cretonne/src/ir/dfg.rs b/lib/cretonne/src/ir/dfg.rs
index a402274d5e..36607e76f1 100644
--- a/lib/cretonne/src/ir/dfg.rs
+++ b/lib/cretonne/src/ir/dfg.rs
@@ -127,7 +127,7 @@ fn resolve_aliases(values: &EntityMap<Value, ValueData>, value: Value) -> Value
 ///
 /// Values are either EBB arguments or instruction results.
 impl DataFlowGraph {
-    // Allocate an extended value entry.
+    /// Allocate an extended value entry.
     fn make_value(&mut self, data: ValueData) -> Value {
         self.values.push(data)
     }
@@ -562,6 +562,17 @@ impl DataFlowGraph {
                         })
     }
 
+    /// Append a new value argument to an instruction.
+    ///
+    /// Panics if the instruction doesn't support arguments.
+    pub fn append_inst_arg(&mut self, inst: Inst, new_arg: Value) {
+        let mut branch_values = self.insts[inst]
+            .take_value_list()
+            .expect("the instruction doesn't have value arguments");
+        branch_values.push(new_arg, &mut self.value_lists);
+        self.insts[inst].put_value_list(branch_values)
+    }
+
     /// Get the first result of an instruction.
     ///
     /// This function panics if the instruction doesn't have any result.
@@ -682,6 +693,35 @@ impl DataFlowGraph {
                         })
     }
 
+    /// Removes `val` from `ebb`'s argument by swapping it with the last argument of `ebb`.
+    /// Returns the position of `val` before removal.
+    ///
+    /// *Important*: to ensure O(1) deletion, this method swaps the removed argument with the
+    /// last `Ebb` argument. This can disrupt all the branch instructions jumping to this
+    /// `Ebb` for which you have to change the jump argument order if necessary.
+    ///
+    /// Panics if `val` is not an `Ebb` argument. Returns `true` if `Ebb` arguments have been
+    /// swapped.
+    pub fn swap_remove_ebb_arg(&mut self, val: Value) -> usize {
+        let (ebb, num) = if let ValueData::Arg { num, ebb, .. } = self.values[val] {
+            (ebb, num)
+        } else {
+            panic!("{} must be an EBB argument", val);
+        };
+        self.ebbs[ebb]
+            .args
+            .swap_remove(num as usize, &mut self.value_lists);
+        if let Some(last_arg_val) = self.ebbs[ebb].args.get(num as usize, &self.value_lists) {
+            // We update the position of the old last arg.
+            if let ValueData::Arg { num: ref mut old_num, .. } = self.values[last_arg_val] {
+                *old_num = num;
+            } else {
+                panic!("{} should be an Ebb argument but is not", last_arg_val);
+            }
+        }
+        num as usize
+    }
+
     /// Append an existing argument value to `ebb`.
     ///
     /// The appended value can't already be attached to something else.
@@ -899,6 +939,30 @@ mod tests {
         assert_eq!(dfg.ebb_args(ebb), &[new1, new3, arg1]);
     }
 
+    #[test]
+    fn swap_remove_ebb_arguments() {
+        let mut dfg = DataFlowGraph::new();
+
+        let ebb = dfg.make_ebb();
+        let arg1 = dfg.append_ebb_arg(ebb, types::F32);
+        let arg2 = dfg.append_ebb_arg(ebb, types::F32);
+        let arg3 = dfg.append_ebb_arg(ebb, types::F32);
+        assert_eq!(dfg.ebb_args(ebb), &[arg1, arg2, arg3]);
+
+        dfg.swap_remove_ebb_arg(arg1);
+        assert_eq!(dfg.value_is_attached(arg1), false);
+        assert_eq!(dfg.value_is_attached(arg2), true);
+        assert_eq!(dfg.value_is_attached(arg3), true);
+        assert_eq!(dfg.ebb_args(ebb), &[arg3, arg2]);
+        dfg.swap_remove_ebb_arg(arg2);
+        assert_eq!(dfg.value_is_attached(arg2), false);
+        assert_eq!(dfg.value_is_attached(arg3), true);
+        assert_eq!(dfg.ebb_args(ebb), &[arg3]);
+        dfg.swap_remove_ebb_arg(arg3);
+        assert_eq!(dfg.value_is_attached(arg3), false);
+        assert_eq!(dfg.ebb_args(ebb), &[]);
+    }
+
     #[test]
     fn aliases() {
         use ir::InstBuilder;