Use unique names when printing

vaivaswatha · Nov 18, 2023 · 843eecb · 843eecb
1 parent c297c16
commit 843eecb
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 48 deletions.
diff --git a/README.md b/README.md
@@ -19,8 +19,8 @@ and written in safe Rust.
     block_0_0():
       builtin.func @foo: builtin.function <() -> (builtin.integer <si64>)> {
         entry():
-          c0 = builtin.constant 0x0: builtin.integer <si64>
-          llvm.return c0
+          c0_op_2_0_res0 = builtin.constant 0x0: builtin.integer <si64>;
+          llvm.return c0_op_2_0_res0
       }
   }
   ```

diff --git a/src/basic_block.rs b/src/basic_block.rs
@@ -47,12 +47,11 @@ impl BlockArgument {
 }
 
 impl Named for BlockArgument {
-    fn get_name(&self, ctx: &Context) -> String {
-        get_block_arg_name(ctx, self.get_def_block(), self.arg_idx).unwrap_or_else(|| {
-            let mut name = self.def_block.deref(ctx).get_name(ctx);
-            name.push_str(&format!("[{}]", self.arg_idx));
-            name
-        })
+    fn given_name(&self, ctx: &Context) -> Option<String> {
+        get_block_arg_name(ctx, self.get_def_block(), self.arg_idx)
+    }
+    fn id(&self, ctx: &Context) -> String {
+        format!("{}_arg{}", self.def_block.deref(ctx).id(ctx), self.arg_idx)
     }
 }
 
@@ -72,7 +71,7 @@ impl Printable for BlockArgument {
         _state: &printable::State,
         f: &mut core::fmt::Formatter<'_>,
     ) -> core::fmt::Result {
-        write!(f, "{}:{}", self.get_name(ctx), self.get_type().disp(ctx))
+        write!(f, "{}:{}", self.unique_name(ctx), self.get_type().disp(ctx))
     }
 }
 
@@ -126,11 +125,11 @@ pub struct BasicBlock {
 }
 
 impl Named for BasicBlock {
-    fn get_name(&self, _ctx: &Context) -> String {
-        self.label
-            .as_ref()
-            .cloned()
-            .unwrap_or_else(|| self.self_ptr.make_name("block"))
+    fn given_name(&self, _ctx: &Context) -> Option<String> {
+        self.label.as_ref().cloned()
+    }
+    fn id(&self, _ctx: &Context) -> String {
+        self.self_ptr.make_name("block")
     }
 }
 
@@ -309,7 +308,7 @@ impl Printable for BasicBlock {
         write!(
             f,
             "{}({}):",
-            self.get_name(ctx),
+            self.unique_name(ctx),
             self.args
                 .iter()
                 .iprint(ctx, state, ListSeparator::Char(','))

diff --git a/src/common_traits.rs b/src/common_traits.rs
@@ -9,10 +9,20 @@ pub trait Verify {
 
 /// Anything that has a name.
 pub trait Named {
-    fn get_name(&self, ctx: &Context) -> String;
+    // A (not necessarily unique) name.
+    fn given_name(&self, ctx: &Context) -> Option<String>;
+    // A Unique (within the context) ID.
+    fn id(&self, ctx: &Context) -> String;
+    // A unique name; concatenation of name and id.
+    fn unique_name(&self, ctx: &Context) -> String {
+        match self.given_name(ctx) {
+            Some(given_name) => given_name + "_" + &self.id(ctx),
+            None => self.id(ctx),
+        }
+    }
 }
 
-/// For types that contain a reference-counted container,
+/// For types that are a reference-counted container,
 /// provides methods to [share](Self::share) (i.e., [Rc::clone](std::rc::Rc::clone))
 /// and [deep copy](Self::replicate) inner data.
 /// This just avoids ambiguity over using `Rc::clone`, which doesn't clone

diff --git a/src/dialects/builtin/ops.rs b/src/dialects/builtin/ops.rs
@@ -281,7 +281,7 @@ impl Printable for ConstantOp {
         write!(
             f,
             "{} = {} {}",
-            self.get_result(ctx).get_name(ctx),
+            self.get_result(ctx).unique_name(ctx),
             self.get_opid().disp(ctx),
             self.get_value(ctx).disp(ctx)
         )

diff --git a/src/operation.rs b/src/operation.rs
@@ -53,9 +53,12 @@ impl From<&OpResult> for Value {
 }
 
 impl Named for OpResult {
-    fn get_name(&self, ctx: &Context) -> String {
+    fn given_name(&self, ctx: &Context) -> Option<String> {
         debug_info::get_operation_result_name(ctx, self.def_op, self.res_idx)
-            .unwrap_or_else(|| self.def_op.make_name("op") + &format!("[{}]", self.res_idx))
+    }
+
+    fn id(&self, _ctx: &Context) -> String {
+        format!("{}_res{}", self.def_op.make_name("op"), self.res_idx)
     }
 }
 
@@ -415,7 +418,7 @@ impl<T: DefUseParticipant + Named> Printable for Operand<T> {
         _state: &printable::State,
         f: &mut core::fmt::Formatter<'_>,
     ) -> core::fmt::Result {
-        write!(f, "{}", self.r#use.get_def().get_name(ctx))
+        write!(f, "{}", self.r#use.get_def().unique_name(ctx))
     }
 }
 

diff --git a/src/use_def_lists.rs b/src/use_def_lists.rs
@@ -171,18 +171,29 @@ impl Value {
 }
 
 impl Named for Value {
-    fn get_name(&self, ctx: &Context) -> String {
+    fn given_name(&self, ctx: &Context) -> Option<String> {
         match self {
             Value::OpResult { op, res_idx } => op
                 .deref(ctx)
                 .get_result_ref(*res_idx)
                 .unwrap()
-                .get_name(ctx),
+                .given_name(ctx),
             Value::BlockArgument { block, arg_idx } => block
                 .deref(ctx)
                 .get_argument_ref(*arg_idx)
                 .unwrap()
-                .get_name(ctx),
+                .given_name(ctx),
+        }
+    }
+
+    fn id(&self, ctx: &Context) -> String {
+        match self {
+            Value::OpResult { op, res_idx } => {
+                op.deref(ctx).get_result_ref(*res_idx).unwrap().id(ctx)
+            }
+            Value::BlockArgument { block, arg_idx } => {
+                block.deref(ctx).get_argument_ref(*arg_idx).unwrap().id(ctx)
+            }
         }
     }
 }
@@ -236,8 +247,11 @@ impl UseTrait for Value {
 }
 
 impl Named for Ptr<BasicBlock> {
-    fn get_name(&self, ctx: &Context) -> String {
-        self.deref(ctx).get_name(ctx)
+    fn given_name(&self, ctx: &Context) -> Option<String> {
+        self.deref(ctx).given_name(ctx)
+    }
+    fn id(&self, ctx: &Context) -> String {
+        self.deref(ctx).id(ctx)
     }
 }
 

diff --git a/tests/common/mod.rs b/tests/common/mod.rs
@@ -1,5 +1,4 @@
 use apint::ApInt;
-use expect_test::expect;
 use pliron::{
     common_traits::Verify,
     context::Context,
@@ -16,7 +15,6 @@ use pliron::{
     },
     error::Result,
     op::Op,
-    printable::Printable,
 };
 
 pub fn setup_context_dialects() -> Context {
@@ -47,18 +45,6 @@ pub fn const_ret_in_mod(ctx: &mut Context) -> Result<(ModuleOp, FuncOp, Constant
     let ret_op = ReturnOp::new_unlinked(ctx, const_op.get_result(ctx));
     ret_op.get_operation().insert_at_back(bb, ctx);
 
-    let printed = format!("{}", module.disp(ctx));
-    expect![[r#"
-        builtin.module @bar {
-          block_0_0():
-            builtin.func @foo: builtin.function <() -> (builtin.integer <si64>)> {
-              entry():
-                c0 = builtin.constant 0x0: builtin.integer <si64>;
-                llvm.return c0
-            }
-        }"#]]
-    .assert_eq(&printed);
-
     module.get_operation().verify(ctx)?;
 
     Ok((module, func, const_op, ret_op))

diff --git a/tests/ir_construct.rs b/tests/ir_construct.rs
@@ -86,10 +86,10 @@ fn replace_c0_with_c1_operand() -> Result<()> {
         builtin.module @bar {
           block_0_0():
             builtin.func @foo: builtin.function <() -> (builtin.integer <si64>)> {
-              entry():
-                c0 = builtin.constant 0x0: builtin.integer <si64>;
-                c1 = builtin.constant 0x1: builtin.integer <si64>;
-                llvm.return c0
+              entry_block_1_0():
+                c0_op_2_0_res0 = builtin.constant 0x0: builtin.integer <si64>;
+                c1_op_4_0_res0 = builtin.constant 0x1: builtin.integer <si64>;
+                llvm.return c0_op_2_0_res0
             }
         }"#]]
     .assert_eq(&printed);
@@ -105,9 +105,9 @@ fn replace_c0_with_c1_operand() -> Result<()> {
         builtin.module @bar {
           block_0_0():
             builtin.func @foo: builtin.function <() -> (builtin.integer <si64>)> {
-              entry():
-                c1 = builtin.constant 0x1: builtin.integer <si64>;
-                llvm.return c1
+              entry_block_1_0():
+                c1_op_4_0_res0 = builtin.constant 0x1: builtin.integer <si64>;
+                llvm.return c1_op_4_0_res0
             }
         }"#]]
     .assert_eq(&printed);
@@ -122,6 +122,17 @@ fn replace_c0_with_c1_operand() -> Result<()> {
 fn print_simple() -> Result<()> {
     let ctx = &mut setup_context_dialects();
     let module_op = const_ret_in_mod(ctx)?.0.get_operation();
-    println!("{}", module_op.disp(ctx));
+    let printed = format!("{}", module_op.disp(ctx));
+    expect![[r#"
+        builtin.module @bar {
+          block_0_0():
+            builtin.func @foo: builtin.function <() -> (builtin.integer <si64>)> {
+              entry_block_1_0():
+                c0_op_2_0_res0 = builtin.constant 0x0: builtin.integer <si64>;
+                llvm.return c0_op_2_0_res0
+            }
+        }"#]]
+    .assert_eq(&printed);
+    println!("{}", printed);
     Ok(())
 }