Skip to content

Commit

Permalink
Remove Arc helpers such as GuardedOp (#38)
Browse files Browse the repository at this point in the history
The idea was to have special traits with convenience methods 

```rust
impl GuardedRegion for Arc<RwLock<Region>> {
    fn add_empty_block(&self) -> UnsetBlock {
        self.rd().add_empty_block()
    }
}
```
to allow writing
```rust
region.add_empty_block()
```
instead of
```rust
region.try_read().unwrap().add_empty_block()
```

This had two problems. One was that it of course takes extra code to
implement and clients would be needed to always include the trait. The
bigger problem though is that this layer of indirection hides the
docstring from the original method unless you copy the docstring.
  • Loading branch information
rikhuijzer authored Dec 21, 2024
1 parent 007c603 commit dea2aac
Show file tree
Hide file tree
Showing 25 changed files with 231 additions and 595 deletions.
6 changes: 3 additions & 3 deletions arnoldc/src/arnold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::sync::RwLock;
use xrcf::ir::APInt;
use xrcf::ir::Attribute;
use xrcf::ir::Block;
use xrcf::ir::GuardedOperation;
use xrcf::ir::IntegerAttr;
use xrcf::ir::IntegerType;
use xrcf::ir::Op;
Expand Down Expand Up @@ -372,11 +371,12 @@ impl PrintOp {
const TEXT_INDEX: usize = 0;
pub fn text(&self) -> Arc<RwLock<OpOperand>> {
self.operation
.rd()
.operand(Self::TEXT_INDEX)
.expect("Operand not set")
}
pub fn set_text(&mut self, text: Arc<RwLock<OpOperand>>) {
self.operation.set_operand(Self::TEXT_INDEX, text);
self.operation.wr().set_operand(Self::TEXT_INDEX, text);
}
}

Expand Down Expand Up @@ -425,7 +425,7 @@ pub struct SetInitialValueOp {

impl SetInitialValueOp {
pub fn value(&self) -> Arc<dyn Attribute> {
self.operation.attributes().get("value").unwrap()
self.operation.rd().attributes().get("value").unwrap()
}
}

Expand Down
35 changes: 16 additions & 19 deletions arnoldc/src/arnold_to_mlir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ use xrcf::dialect::scf;
use xrcf::ir::APInt;
use xrcf::ir::Attribute;
use xrcf::ir::Block;
use xrcf::ir::GuardedOp;
use xrcf::ir::GuardedOpOperand;
use xrcf::ir::GuardedOperation;
use xrcf::ir::IntegerAttr;
use xrcf::ir::IntegerType;
use xrcf::ir::Op;
Expand Down Expand Up @@ -78,9 +75,9 @@ impl Rewrite for DeclareIntLowering {
fn rewrite(&self, op: Arc<RwLock<dyn Op>>) -> Result<RewriteResult> {
let op = op.rd();
let op = op.as_any().downcast_ref::<arnold::DeclareIntOp>().unwrap();
op.operation().rename_variables(&RENAMER)?;
op.operation().rd().rename_variables(&RENAMER)?;

let successors = op.operation().successors();
let successors = op.operation().rd().successors();
let set_initial_value = successors.first().unwrap();
let set_initial_value = set_initial_value.rd();
let set_initial_value = set_initial_value
Expand All @@ -90,7 +87,7 @@ impl Rewrite for DeclareIntLowering {

let operation = Operation::default();
let new_op = arith::ConstantOp::from_operation(operation);
new_op.set_parent(op.operation().parent().clone().unwrap());
new_op.set_parent(op.operation().rd().parent().clone().unwrap());
new_op.set_value(set_initial_value.value());
set_initial_value.remove();
let new_op = Shared::new(new_op.into());
Expand Down Expand Up @@ -153,7 +150,6 @@ impl Rewrite for IfLowering {
let op = op.as_any().downcast_ref::<arnold::IfOp>().unwrap();
let operation = op.operation();
let mut new_op = scf::IfOp::from_operation_arc(operation.clone());
new_op.set_parent(operation.parent().clone().unwrap());
new_op.set_then(op.then().clone());
new_op.set_els(op.els().clone());
let new_op = Shared::new(new_op.into());
Expand Down Expand Up @@ -191,7 +187,7 @@ impl ModuleLowering {
ret.set_parent(Some(parent.clone()));
ret.set_name(func::ReturnOp::operation_name());
ret.set_anonymous_result(result_type).unwrap();
let value = constant.result(0);
let value = constant.rd().result(0);
let operand = OpOperand::new(value);
let operand = Shared::new(operand.into());
ret.set_operand(0, operand);
Expand All @@ -200,34 +196,35 @@ impl ModuleLowering {
ret
}
fn return_zero(func: Arc<RwLock<dyn Op>>) {
let operation = func.operation();
let typ = IntegerType::new(32);
operation
func.rd()
.operation()
.wr()
.set_anonymous_result(Shared::new(typ.into()))
.unwrap();

let ops = func.ops();
let ops = func.rd().ops();
if ops.is_empty() {
panic!("Expected ops to be non-empty");
}
let last = ops.last().unwrap();
let block = last.operation().parent();
let block = last.rd().operation().rd().parent();
let block = block.expect("no parent for operation");

let constant = Self::constant_op(&block);
last.insert_after(constant.clone());
last.rd().insert_after(constant.clone());

let ret = Self::return_op(&block, constant.clone());
constant.insert_after(ret.clone());
constant.rd().insert_after(ret.clone());
}
fn returns_something(func: Arc<RwLock<dyn Op>>) -> bool {
let func = func.rd();
let func_op = func.as_any().downcast_ref::<func::FuncOp>().unwrap();
let result = func_op.operation().results();
let result = func_op.operation().rd().results();
result.vec().rd().len() == 1
}
fn ensure_main_returns_zero(module: Arc<RwLock<dyn Op>>) -> Result<RewriteResult> {
let ops = module.ops();
let ops = module.rd().ops();
let last = ops.last().unwrap();
if !Self::returns_something(last.clone()) {
Self::return_zero(last.clone());
Expand Down Expand Up @@ -266,10 +263,10 @@ impl Rewrite for PrintLowering {
operation.set_name(experimental::PrintfOp::operation_name());
let operation = Shared::new(operation.into());
let mut new_op = experimental::PrintfOp::from_operation_arc(operation.clone());
new_op.set_parent(op.operation().parent().clone().unwrap());
new_op.set_parent(op.operation().rd().parent().clone().unwrap());

let operand = op.text();
let value = operand.value();
let value = operand.rd().value();
match &*value.rd() {
// printf("some text")
Value::Constant(constant) => {
Expand All @@ -281,7 +278,7 @@ impl Rewrite for PrintLowering {
Value::OpResult(_) => {
let text = StringAttr::from_str("%d");
new_op.set_text(text);
new_op.operation().set_operand(1, operand);
new_op.operation().wr().set_operand(1, operand);
}
_ => panic!("expected constant or op result"),
};
Expand Down
14 changes: 1 addition & 13 deletions developer-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,7 @@ For example, in my experience, declarative code is a beautiful idea, but in prac
This project uses `Arc<RwLock<T>>` for pretty much everything.
The reason is that the intermediate representation (IR) is essentially a tree with nodes.
This tree is traversed in multiple directions (up, down, and sideways), so we need to store pointers all over the place.
Also, this project uses `Arc` instead of `Rc` because at some point the compiler will be multi-threaded.

To simplify the usage, there are some helpers functions available via `GuardedOp`, `GuardedBlock`, etc.

In some cases, this can simplify code such as
```rust
let x = x.read().unwrap();
let parent = x.parent();
```
to
```rust
let x = x.parent();
```
Also, this project uses `Arc` instead of `Rc` because at some point the compiler will be multithreaded.

## LLVM's codebase

Expand Down
3 changes: 1 addition & 2 deletions xrcf/src/canonicalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::convert::ChangedOp;
use crate::convert::Pass;
use crate::convert::Rewrite;
use crate::convert::RewriteResult;
use crate::ir::GuardedBlock;
use crate::ir::Op;
use crate::ir::Users;
use crate::shared::SharedExt;
Expand Down Expand Up @@ -48,7 +47,7 @@ impl Rewrite for DeadCodeElimination {
Users::OpOperands(users) => {
if users.is_empty() {
let parent = operation.parent().unwrap();
parent.remove(readonly.operation().clone());
parent.rd().remove(readonly.operation().clone());
Ok(RewriteResult::Changed(ChangedOp::new(op.clone())))
} else {
Ok(RewriteResult::Unchanged)
Expand Down
34 changes: 16 additions & 18 deletions xrcf/src/convert/experimental_to_mlir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ use crate::dialect::llvm::PointerType;
use crate::ir::APInt;
use crate::ir::Block;
use crate::ir::BlockArgumentName;
use crate::ir::GuardedBlock;
use crate::ir::GuardedOp;
use crate::ir::GuardedOperation;
use crate::ir::IntegerAttr;
use crate::ir::IntegerType;
use crate::ir::Op;
Expand Down Expand Up @@ -72,7 +69,7 @@ impl PrintLowering {
let name = parent.rd().unique_value_name("%");
let result_type = Shared::new(typ.into());
let result = operation.add_new_op_result(&name, result_type);
let array_size = len.result(0);
let array_size = len.rd().result(0);
let array_size = OpOperand::new(array_size);
let array_size = Shared::new(array_size.into());
operation.set_operand(0, array_size);
Expand All @@ -93,11 +90,11 @@ impl PrintLowering {

let mut op = llvm::StoreOp::from_operation(operation);

let value = text.result(0);
let value = text.rd().result(0);
let value = OpOperand::new(value);
op.set_value(Shared::new(value.into()));

let addr = alloca.result(0);
let addr = alloca.rd().result(0);
let addr = OpOperand::new(addr);
op.set_addr(Shared::new(addr.into()));
Shared::new(op.into())
Expand All @@ -111,18 +108,18 @@ impl PrintLowering {
let mut operation = Operation::default();
operation.set_parent(Some(parent.clone()));
{
let text_addr = alloca.result(0);
let text_addr = alloca.rd().result(0);
let text_addr = OpOperand::new(text_addr);
let text_addr = Shared::new(text_addr.into());
operation.set_operand(0, text_addr);
}
if set_varargs {
let var = op.operation().operand(1);
let var = op.operation().rd().operand(1);
let var = var.expect("expected vararg");
operation.set_operand(1, var);
}
let typ = IntegerType::from_str("i32");
let name = parent.unique_value_name("%");
let name = parent.rd().unique_value_name("%");
let result_type = Shared::new(typ.into());
let result = operation.add_new_op_result(&name, result_type);

Expand All @@ -143,7 +140,7 @@ impl PrintLowering {
fn top_level_op(op: Arc<RwLock<dyn Op>>) -> Arc<RwLock<dyn Op>> {
let mut out = op.clone();
for i in 0..1000 {
let parent_op = out.parent_op();
let parent_op = out.rd().parent_op();
match parent_op {
Some(parent_op) => out = parent_op,
None => break,
Expand All @@ -156,7 +153,7 @@ impl PrintLowering {
}
/// Whether the parent operation of `op` contains a `printf` function.
fn contains_printf(top_level_op: Arc<RwLock<dyn Op>>) -> bool {
let ops = top_level_op.ops();
let ops = top_level_op.rd().ops();
for op in ops {
let op = op.rd();
if op.is_func() {
Expand Down Expand Up @@ -197,12 +194,12 @@ impl PrintLowering {
let value = Value::BlockArgument(argument);
let value = Shared::new(value.into());
let operation = op.operation();
operation.set_argument(0, value);
operation.wr().set_argument(0, value);
}
if set_varargs {
let value = Value::Variadic;
let value = Shared::new(value.into());
op.operation().set_argument(1, value);
op.operation().wr().set_argument(1, value);
}
let op = Shared::new(op.into());
Ok(op)
Expand All @@ -211,10 +208,11 @@ impl PrintLowering {
fn define_printf(op: Arc<RwLock<dyn Op>>, set_varargs: bool) -> Result<()> {
let top_level_op = Self::top_level_op(op.clone());
if !Self::contains_printf(top_level_op.clone()) {
let ops = top_level_op.ops();
let ops = top_level_op.rd().ops();
let op = ops[0].clone();
let parent = op.operation().parent().unwrap();
op.insert_before(Self::printf_func_def(parent, set_varargs)?);
let parent = op.rd().operation().rd().parent().unwrap();
op.rd()
.insert_before(Self::printf_func_def(parent, set_varargs)?);
}
Ok(())
}
Expand All @@ -229,13 +227,13 @@ impl Rewrite for PrintLowering {
}
fn rewrite(&self, op: Arc<RwLock<dyn Op>>) -> Result<RewriteResult> {
let op_clone = op.clone();
let set_varargs = 1 < op.operation().operands().vec().rd().len();
let set_varargs = 1 < op.rd().operation().rd().operands().vec().rd().len();
let op_rd = op_clone.rd();
let op_rd = op_rd
.as_any()
.downcast_ref::<dialect::experimental::PrintfOp>()
.unwrap();
let parent = op_rd.operation().parent();
let parent = op_rd.operation().rd().parent();
let parent = parent.expect("no parent");
let (text, len) = PrintLowering::text_constant(&parent, op_rd);
op_rd.insert_before(text.clone());
Expand Down
7 changes: 3 additions & 4 deletions xrcf/src/convert/func_to_llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::dialect::func;
use crate::dialect::func::Call;
use crate::dialect::func::Func;
use crate::dialect::llvm;
use crate::ir::GuardedOperation;
use crate::ir::Op;
use crate::shared::Shared;
use crate::shared::SharedExt;
Expand Down Expand Up @@ -89,11 +88,11 @@ impl Rewrite for FuncLowering {
let op = op.as_any().downcast_ref::<func::FuncOp>().unwrap();
let operation = op.operation();
{
let name = operation.name();
let name = operation.rd().name();
assert!(name == func::FuncOp::operation_name());
operation.set_name(name.clone());
operation.wr().set_name(name.clone());

let parent = operation.parent();
let parent = operation.rd().parent();
assert!(
parent.is_some(),
"maybe parent was not set during parsing for {name}?",
Expand Down
Loading

0 comments on commit dea2aac

Please sign in to comment.