Share Block instead of Block.ops (#49)

Removes one `Shared` (`Arc`) to simplify the API and replaces trivial
getters and setters by `pub` (#44).
```diff
  pub struct Block {
      pub label: BlockName,
      arguments: Values,
-     ops: Shared<Vec<Shared<dyn Op>>>,
+     pub ops: Vec<Shared<dyn Op>>,
      parent: Option<Shared<Region>>,
  }
```

xrcf/src/canonicalize.rs

@@ -46,7 +46,7 @@ impl Rewrite for DeadCodeElimination {
             Users::OpOperands(users) => {
                 if users.is_empty() {
                     let parent = operation.parent().unwrap();
-                    parent.rd().remove(readonly.operation().clone());
+                    parent.wr().remove(readonly.operation().clone());
                     Ok(RewriteResult::Changed(ChangedOp::new(op.clone())))
                 } else {
                     Ok(RewriteResult::Unchanged)

xrcf/src/convert/mlir_to_llvmir.rs

@@ -406,8 +406,7 @@ fn set_phi_result(phi: Shared<dyn Op>, argument: &Shared<Value>) {
 
 /// Replace the only argument of the block by a `phi` instruction.
 fn insert_phi(block: Shared<Block>) {
-    let block_read = block.rd();
-    let arguments = block_read.arguments().vec();
+    let arguments = block.rd().arguments().vec();
     let mut arguments = arguments.wr();
     assert!(
         arguments.len() == 1,
@@ -426,7 +425,7 @@ fn insert_phi(block: Shared<Block>) {
     let phi = Shared::new(phi.into());
     set_phi_result(phi.clone(), argument);
     arguments.clear();
-    block_read.insert_op(phi, 0);
+    block.wr().insert_op(phi, 0);
 }
 
 /// Remove the operands of the callers that call given block.
@@ -463,8 +462,7 @@ fn one_child_block_has_argument(op: &dyn Op) -> Result<bool> {
         return Ok(false);
     }
     for block in operation.rd().blocks().into_iter() {
-        let block = block.rd();
-        let has_argument = !block.arguments().vec().rd().is_empty();
+        let has_argument = !block.rd().arguments().vec().rd().is_empty();
         if has_argument {
             return Ok(true);
         }
@@ -485,8 +483,7 @@ impl Rewrite for MergeLowering {
         }
         let blocks = op.rd().operation().rd().region().unwrap().rd().blocks();
         for block in blocks.into_iter() {
-            let block_read = block.rd();
-            let has_argument = !block_read.arguments().vec().rd().is_empty();
+            let has_argument = !block.rd().arguments().vec().rd().is_empty();
             if has_argument {
                 insert_phi(block.clone());
                 remove_caller_operands(block.clone());

xrcf/src/convert/scf_to_cf.rs

@@ -73,8 +73,7 @@ fn branch_op(after: Shared<Block>) -> Shared<dyn Op> {
 
 /// Add a `cf.br` to the end of `block` with destination `after`.
 fn add_branch_to_after(block: Shared<Block>, after: Shared<Block>) {
-    let ops = block.rd().ops();
-    let mut ops = ops.wr();
+    let ops = &mut block.wr().ops;
     let ops_clone = ops.clone();
     let last_op = ops_clone.last().unwrap();
     let last_op = last_op.rd();
@@ -118,14 +117,12 @@ fn move_successors_to_exit_block(op: &dialect::scf::IfOp, exit_block: Shared<Blo
         .rd()
         .index_of(&op.operation().rd())
         .expect("Expected index");
-    let ops = if_op_parent.rd().ops();
-    let mut ops = ops.wr();
+    let ops = &mut if_op_parent.wr().ops;
     let return_ops = ops[if_op_index + 1..].to_vec();
     for op in return_ops.iter() {
-        let op = op.rd();
-        op.set_parent(exit_block.clone());
+        op.rd().set_parent(exit_block.clone());
     }
-    exit_block.wr().set_ops(Shared::new(return_ops.into()));
+    exit_block.wr().ops = return_ops;
     ops.drain(if_op_index + 1..);
     Ok(())
 }
@@ -149,9 +146,7 @@ fn add_merge_block(
     merge_op.set_dest(operand);
 
     let merge_op: Shared<dyn Op> = Shared::new(merge_op.into());
-    merge
-        .wr()
-        .set_ops(Shared::new(vec![merge_op.clone()].into()));
+    merge.wr().ops = vec![merge_op.clone()];
     Ok((merge, merge_block_arguments))
 }
 

xrcf/src/dialect/func/op.rs

@@ -249,12 +249,11 @@ impl FuncOp {
                 panic!("Expected region to be empty");
             }
             let ops = vec![op.clone()];
-            let ops = Shared::new(ops.into());
             let region = Region::default();
             let without_parent = region.add_empty_block();
             let region = Shared::new(region.into());
             let block = without_parent.set_parent(Some(region.clone()));
-            block.wr().set_ops(ops);
+            block.wr().ops = ops;
             operation.wr().set_region(Some(region));
         } else {
             ops.last().unwrap().rd().insert_after(op.clone());

xrcf/src/frontend/parser.rs

@@ -222,7 +222,7 @@ fn replace_block_labels(block: Shared<Block>) {
     let parent = block.rd().parent().expect("no parent");
     // Assumes the current block was not yet added to the parent region.
     for predecessor in parent.rd().blocks().into_iter() {
-        for op in predecessor.rd().ops().rd().iter() {
+        for op in predecessor.rd().ops.iter() {
             for operand in op.rd().operation().rd().operands().into_iter() {
                 let mut operand = operand.wr();
                 if let Value::BlockLabel(curr) = &*operand.value().rd() {
@@ -316,9 +316,7 @@ impl<T: ParserDispatch> Parser<T> {
             (label, values)
         };
 
-        let ops = vec![];
-        let ops = Shared::new(ops.into());
-        let block = Block::new(label, arguments.clone(), ops.clone(), parent);
+        let block = Block::new(label, arguments.clone(), vec![], parent);
         let block = Shared::new(block.into());
         replace_block_labels(block.clone());
         for argument in arguments.vec().rd().iter() {
@@ -331,14 +329,14 @@ impl<T: ParserDispatch> Parser<T> {
         while !self.is_region_end() && !self.is_block_definition() {
             let parent = Some(block.clone());
             let op = T::parse_op(self, parent)?;
-            ops.wr().push(op.clone());
+            block.wr().ops.push(op.clone());
         }
-        if ops.rd().is_empty() {
+        if block.rd().ops.is_empty() {
             let token = self.peek();
             let msg = self.error(token, "Could not find operations in block");
             return Err(anyhow::anyhow!(msg));
         }
-        for op in block.rd().ops().rd().iter() {
+        for op in block.rd().ops.iter() {
             op.rd().operation().wr().set_parent(Some(block.clone()));
         }
         Ok(block)
@@ -422,13 +420,12 @@ impl<T: ParserDispatch> Parser<T> {
                     ops.push(op.clone());
                 }
             }
-            let ops = Shared::new(ops.into());
             let arguments = Values::default();
             let label = BlockName::Unnamed;
             let block = Block::new(label, arguments, ops.clone(), Some(module_region.clone()));
             let block = Shared::new(block.into());
             {
-                for child_op in ops.rd().iter() {
+                for child_op in ops.iter() {
                     child_op
                         .rd()
                         .operation()

xrcf/src/ir/block.rs

@@ -29,7 +29,7 @@ pub struct Block {
     /// this block. During printing, a new name is generated.
     pub label: BlockName,
     arguments: Values,
-    ops: Shared<Vec<Shared<dyn Op>>>,
+    pub ops: Vec<Shared<dyn Op>>,
     /// Region has to be `Shared` to allow the parent to be moved.
     parent: Option<Shared<Region>>,
 }
@@ -52,7 +52,7 @@ impl Block {
     pub fn new(
         label: BlockName,
         arguments: Values,
-        ops: Shared<Vec<Shared<dyn Op>>>,
+        ops: Vec<Shared<dyn Op>>,
         parent: Option<Shared<Region>>,
     ) -> Self {
         Self {
@@ -65,18 +65,9 @@ impl Block {
     pub fn arguments(&self) -> Values {
         self.arguments.clone()
     }
-    pub fn ops(&self) -> Shared<Vec<Shared<dyn Op>>> {
-        self.ops.clone()
-    }
-    pub fn set_ops(&mut self, ops: Shared<Vec<Shared<dyn Op>>>) {
-        self.ops = ops;
-    }
     pub fn set_parent(&mut self, parent: Option<Shared<Region>>) {
         self.parent = parent;
     }
-    pub fn ops_mut(&mut self) -> &mut Shared<Vec<Shared<dyn Op>>> {
-        &mut self.ops
-    }
     pub fn parent(&self) -> Option<Shared<Region>> {
         self.parent.clone()
     }
@@ -100,7 +91,7 @@ impl Block {
         };
         let mut callers = vec![];
         for p in self.predecessors().expect("no predecessors") {
-            for op in p.rd().ops().rd().iter() {
+            for op in p.rd().ops.iter() {
                 for operand in op.rd().operation().rd().operands().into_iter() {
                     match &*operand.rd().value().rd() {
                         Value::BlockPtr(block_ptr) => {
@@ -200,7 +191,7 @@ impl Block {
         None
     }
     pub fn assignment_in_ops(&self, name: &str) -> Option<Shared<Value>> {
-        for op in self.ops().rd().iter() {
+        for op in self.ops.iter() {
             for value in op.rd().assignments().unwrap().into_iter() {
                 match &*value.rd() {
                     Value::BlockArgument(_block_argument) => {
@@ -289,7 +280,7 @@ impl Block {
     ///
     /// Returns `None` if `op` is not found in `self`.
     pub fn index_of(&self, op: &Operation) -> Option<usize> {
-        self.ops().rd().iter().position(|current| {
+        self.ops.iter().position(|current| {
             let current = current.rd();
             let current = current.operation();
             let current = &*current.rd();
@@ -307,32 +298,32 @@ impl Block {
             .blocks()
             .splice(self, region.rd().blocks());
     }
-    pub fn insert_op(&self, op: Shared<dyn Op>, index: usize) {
-        self.ops.wr().insert(index, op);
+    pub fn insert_op(&mut self, op: Shared<dyn Op>, index: usize) {
+        self.ops.insert(index, op);
     }
-    pub fn insert_after(&self, earlier: Shared<Operation>, later: Shared<dyn Op>) {
+    pub fn insert_after(&mut self, earlier: Shared<Operation>, later: Shared<dyn Op>) {
         match self.index_of(&earlier.rd()) {
             Some(index) => self.insert_op(later, index + 1),
             None => {
                 panic!("Could not find op in block during insert_after");
             }
         }
     }
-    pub fn insert_before(&self, earlier: Shared<dyn Op>, later: Shared<Operation>) {
+    pub fn insert_before(&mut self, earlier: Shared<dyn Op>, later: Shared<Operation>) {
         match self.index_of(&later.rd()) {
             Some(index) => self.insert_op(earlier, index),
             None => panic!("could not find op in block"),
         }
     }
-    pub fn replace(&self, old: Shared<Operation>, new: Shared<dyn Op>) {
+    pub fn replace(&mut self, old: Shared<Operation>, new: Shared<dyn Op>) {
         match self.index_of(&old.rd()) {
-            Some(index) => self.ops().wr()[index] = new,
+            Some(index) => self.ops[index] = new,
             None => panic!("could not find op in block"),
         }
     }
-    pub fn remove(&self, op: Shared<Operation>) {
+    pub fn remove(&mut self, op: Shared<Operation>) {
         match self.index_of(&op.rd()) {
-            Some(index) => self.ops().wr().remove(index),
+            Some(index) => self.ops.remove(index),
             None => panic!("could not find op in block"),
         };
     }
@@ -349,7 +340,7 @@ impl Block {
             }
             writeln!(f, ":")?;
         }
-        for op in self.ops().rd().iter() {
+        for op in self.ops.iter() {
             write!(f, "{}", crate::ir::spaces(indent))?;
             op.rd().display(f, indent)?;
             writeln!(f)?;
@@ -362,7 +353,7 @@ impl Default for Block {
     fn default() -> Self {
         let label = BlockName::Unnamed;
         let arguments = Values::default();
-        let ops = Shared::new(vec![].into());
+        let ops = vec![];
         let parent = None;
         Self::new(label, arguments, ops, parent)
     }

xrcf/src/ir/module.rs

@@ -57,7 +57,7 @@ impl ModuleOp {
             Some(block) => block,
             None => return Err(anyhow::anyhow!("Expected 1 block in module, got 0")),
         };
-        match block.rd().ops().rd().first() {
+        match block.rd().ops.first() {
             #[allow(clippy::needless_return)]
             None => return Err(anyhow::anyhow!("Expected 1 op, got 0")),
             #[allow(clippy::needless_return)]

xrcf/src/ir/op.rs

@@ -129,7 +129,7 @@ pub trait Op: Send + Sync {
         };
         let later = operation.clone();
         earlier.rd().set_parent(parent.clone());
-        parent.rd().insert_before(earlier, later);
+        parent.wr().insert_before(earlier, later);
     }
     /// Insert `later` after `self` inside `self`'s parent block.
     ///
@@ -142,13 +142,13 @@ pub trait Op: Send + Sync {
         };
         let earlier = operation.clone();
         later.rd().set_parent(parent.clone());
-        parent.rd().insert_after(earlier, later);
+        parent.wr().insert_after(earlier, later);
     }
     /// Remove the operation from its parent block.
     fn remove(&self) {
         let operation = self.operation();
         let parent = operation.rd().parent().expect("no parent");
-        parent.rd().remove(operation.clone());
+        parent.wr().remove(operation.clone());
     }
     /// Replace self with `new`.
     ///
@@ -172,7 +172,7 @@ pub trait Op: Send + Sync {
         // Root ops do not have a parent, so in that case we don't need to
         // update the parent.
         if let Some(parent) = self.operation().rd().parent() {
-            parent.rd().replace(self.operation().clone(), new.clone())
+            parent.wr().replace(self.operation().clone(), new.clone())
         }
     }
     /// Return ops that are children of this op (inside blocks that are inside

xrcf/src/ir/operation.rs

@@ -322,7 +322,7 @@ impl Operation {
     pub fn predecessors(&self) -> Vec<Shared<dyn Op>> {
         let parent = self.parent().expect("no parent");
         match parent.clone().rd().index_of(self) {
-            Some(index) => parent.rd().ops().rd()[..index].to_vec(),
+            Some(index) => parent.rd().ops[..index].to_vec(),
             None => {
                 panic!(
                     "Expected index. Is the parent set correctly for the following op?\n{}",
@@ -334,7 +334,7 @@ impl Operation {
     pub fn successors(&self) -> Vec<Shared<dyn Op>> {
         let parent = self.parent().expect("no parent");
         match parent.clone().rd().index_of(self) {
-            Some(index) => parent.rd().ops().rd()[index + 1..].to_vec(),
+            Some(index) => parent.rd().ops[index + 1..].to_vec(),
             None => {
                 panic!(
                     "Expected index. Is the parent set correctly for the following op?\n{}",

xrcf/src/ir/region.rs

@@ -107,7 +107,7 @@ fn set_fresh_ssa_names(prefix: &str, blocks: &[Shared<Block>]) {
     // SSA names stay in scope until the end of the region I think.
     let mut name_index: usize = 0;
     for block in blocks.iter() {
-        for op in block.rd().ops().rd().iter() {
+        for op in block.rd().ops.iter() {
             for result in op.rd().operation().rd().results() {
                 if let Value::OpResult(op_result) = &mut *result.wr() {
                     let name = format!("{prefix}{name_index}");
@@ -135,7 +135,7 @@ impl Region {
     pub fn ops(&self) -> Vec<Shared<dyn Op>> {
         let mut result = Vec::new();
         for block in self.blocks().into_iter() {
-            for op in block.rd().ops().rd().iter() {
+            for op in block.rd().ops.iter() {
                 result.push(op.clone());
             }
         }
@@ -175,15 +175,7 @@ impl Region {
         // the module have to be rewritten for the output to be correct so will
         // know the right prefixes, see also the [Op::prefixes] docstring for
         // more information.
-        let prefixes = self
-            .block(0)
-            .rd()
-            .ops()
-            .rd()
-            .first()
-            .unwrap()
-            .rd()
-            .prefixes();
+        let prefixes = self.block(0).rd().ops.first().unwrap().rd().prefixes();
         set_fresh_argument_names(prefixes.argument, &blocks);
         set_fresh_block_labels(prefixes.block, &blocks);
         set_fresh_ssa_names(prefixes.ssa, &blocks);

xrcf/src/ir/value.rs

@@ -460,9 +460,9 @@ impl Value {
         } else {
             panic!("BlockArgument {arg} has no parent operation");
         };
-        let mut ops = parent.rd().ops().rd().clone();
+        let mut ops = parent.rd().ops.clone();
         for successor in parent.rd().successors().unwrap().iter() {
-            ops.extend(successor.rd().ops().rd().clone());
+            ops.extend(successor.rd().ops.clone());
         }
         self.find_users(&ops)
     }