Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more order of operation comparisons #13

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/subset/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,10 @@ impl Expr {
}

pub fn new_ipath_with_index(path: &str, index: &str) -> Expr {
Expr::IPath(InstancePath::new(path), Some(Rc::new(Expr::new_ref(index))))
Expr::IPath(
InstancePath::new(path),
Some(Rc::new(Expr::new_ref(index))),
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a formatting change. In general, we try to only commit the required functional changes for a particular commit. Can you remove this?

}

pub fn new_call(name: &str, params: Vec<Expr>) -> Expr {
Expand Down
127 changes: 89 additions & 38 deletions src/subset/pretty_print.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::subset::ast::*;
use crate::util::pretty_print::{block, intersperse, PrettyHelper, PrettyPrint};
use crate::util::pretty_print::{
block, intersperse, PrettyHelper, PrettyPrint,
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting change

use core::cmp::Ordering;
use itertools::Itertools;
use pretty::RcDoc;
Expand All @@ -8,32 +10,32 @@ use pretty::RcDoc;
/// operator with stronger binding.
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
enum ParenCtx {
Op,
Not,
And,
Or,
Index,
Mul,
AddSub,
Shift,
Cmp,
Equal,
BAnd,
BOr,
And,
Or,
}

impl From<&Binop> for ParenCtx {
fn from(s: &Binop) -> Self {
match s {
Binop::BitOr => ParenCtx::BOr,
Binop::IndexBit => ParenCtx::Index,
Binop::Mul => ParenCtx::Mul,
Binop::Add | Binop::Sub => ParenCtx::AddSub,
Binop::ShiftLeft => ParenCtx::Shift,
Binop::Lt | Binop::Gt | Binop::Geq | Binop::Leq => ParenCtx::Cmp,
Binop::Equal | Binop::NotEqual => ParenCtx::Equal,
Binop::BitAnd => ParenCtx::BAnd,
Binop::LogOr => ParenCtx::Or,
Binop::BitOr => ParenCtx::BOr,
Binop::LogAnd => ParenCtx::And,
Binop::Add
| Binop::Sub
| Binop::Mul
| Binop::Lt
| Binop::Gt
| Binop::Geq
| Binop::Leq
| Binop::Equal
| Binop::NotEqual
| Binop::IndexBit
| Binop::ShiftLeft => ParenCtx::Op,
Binop::LogOr => ParenCtx::Or,
}
}
}
Expand All @@ -47,18 +49,63 @@ impl Ord for ParenCtx {
match (self, other) {
(P::Not, _) => Ordering::Greater,

(P::Op, P::Not) => Ordering::Less,
(P::Op, _) => Ordering::Greater,
(P::Index, P::Not) => Ordering::Less,
(P::Index, _) => Ordering::Greater,

(P::Mul, P::Not) | (P::Mul, P::Index) => Ordering::Less,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this code was written, rust started supporting nested patterns so you can write something like:

(P::Mul, P::Not | P::Index)

Which might be easier to read

(P::Mul, _) => Ordering::Greater,

(P::AddSub, P::Not)
| (P::AddSub, P::Index)
| (P::AddSub, P::Mul) => Ordering::Less,
(P::AddSub, _) => Ordering::Greater,

(P::Shift, P::Not)
| (P::Shift, P::Index)
| (P::Shift, P::Mul)
| (P::Shift, P::AddSub) => Ordering::Less,
(P::Shift, _) => Ordering::Greater,

(P::BAnd, P::Not) | (P::BAnd, P::Op) => Ordering::Less,
(P::Cmp, P::Not)
| (P::Cmp, P::Index)
| (P::Cmp, P::Mul)
| (P::Cmp, P::AddSub)
| (P::Cmp, P::Shift) => Ordering::Greater,
(P::Cmp, _) => Ordering::Less,

(P::Equal, P::Not)
| (P::Equal, P::Index)
| (P::Equal, P::Mul)
| (P::Equal, P::AddSub)
| (P::Equal, P::Shift)
| (P::Equal, P::Cmp) => Ordering::Greater,
(P::Equal, _) => Ordering::Less,

(P::BAnd, P::Not)
| (P::BAnd, P::Mul)
| (P::BAnd, P::AddSub)
| (P::BAnd, P::Shift)
| (P::BAnd, P::Cmp)
| (P::BAnd, P::Equal) => Ordering::Less,
(P::BAnd, _) => Ordering::Greater,

(P::BOr, P::Not) | (P::BOr, P::Op) | (P::BOr, P::BAnd) => Ordering::Less,
(P::BOr, P::Not)
| (P::BOr, P::Mul)
| (P::BOr, P::AddSub)
| (P::BOr, P::Shift)
| (P::BOr, P::Cmp)
| (P::BOr, P::Equal)
| (P::BOr, P::BAnd) => Ordering::Less,
(P::BOr, _) => Ordering::Greater,

(P::And, P::Not) | (P::And, P::Op) | (P::And, P::BAnd) | (P::And, P::BOr) => {
Ordering::Less
}
(P::And, P::Not)
| (P::And, P::Mul)
| (P::And, P::AddSub)
| (P::And, P::Shift)
| (P::And, P::Cmp)
| (P::And, P::Equal)
| (P::And, P::BOr)
| (P::And, P::BAnd) => Ordering::Less,
(P::And, _) => Ordering::Greater,

(P::Or, _) => Ordering::Less,
Expand Down Expand Up @@ -174,7 +221,9 @@ impl PrettyPrint for Expr {
path.to_doc()
}
}
Expr::Unop(op, input) => op.to_doc().append(print_expr(input, ParenCtx::Not)),
Expr::Unop(op, input) => {
op.to_doc().append(print_expr(input, ParenCtx::Not))
}
Expr::Binop(Binop::IndexBit, lhs, rhs) => {
print_expr(lhs, ParenCtx::Not).append(rhs.to_doc().brackets())
}
Expand Down Expand Up @@ -207,15 +256,17 @@ impl PrettyPrint for Expr {
.append(lo.to_doc())
.brackets(),
),
Expr::Terop(Terop::IndexSlice, var, lo, width) => var.to_doc().append(
lo.to_doc()
.append(RcDoc::space())
.append(RcDoc::text("+"))
.append(RcDoc::text(":"))
.append(RcDoc::space())
.append(width.to_doc())
.brackets(),
),
Expr::Terop(Terop::IndexSlice, var, lo, width) => {
var.to_doc().append(
lo.to_doc()
.append(RcDoc::space())
.append(RcDoc::text("+"))
.append(RcDoc::text(":"))
.append(RcDoc::space())
.append(width.to_doc())
.brackets(),
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting change

Expr::Concat(concat) => concat.to_doc(),
Expr::Repeat(times, expr) => RcDoc::text(times.to_string())
.append(expr.to_doc().braces())
Expand Down Expand Up @@ -269,13 +320,13 @@ impl PrettyPrint for AssignTy {
impl PrettyPrint for Map {
fn to_doc(&self) -> RcDoc<()> {
intersperse(
self.iter()
.sorted_by_key(|(id, _)| (*id).clone())
.map(|(id, expr)| {
self.iter().sorted_by_key(|(id, _)| (*id).clone()).map(
|(id, expr)| {
RcDoc::text(".")
.append(RcDoc::as_string(id))
.append(expr.to_doc().parens())
}),
},
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting change

RcDoc::text(",").append(RcDoc::hardline()),
)
}
Expand Down
5 changes: 4 additions & 1 deletion src/util/pretty_print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ impl<'a, A> PrettyHelper<'a> for RcDoc<'a, A> {
}
}

pub fn intersperse<'a>(iter: impl Iterator<Item = RcDoc<'a>>, separator: RcDoc<'a>) -> RcDoc<'a> {
pub fn intersperse<'a>(
iter: impl Iterator<Item = RcDoc<'a>>,
separator: RcDoc<'a>,
) -> RcDoc<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting change

RcDoc::intersperse(iter, separator)
}

Expand Down
11 changes: 9 additions & 2 deletions src/v05/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,18 @@ impl Decl {
}

pub fn new_array(name: &str, width: u64, depth: u64) -> Decl {
Decl::Array(name.to_string(), Ty::new_width(width), Ty::new_width(depth))
Decl::Array(
name.to_string(),
Ty::new_width(width),
Ty::new_width(depth),
)
}

pub fn new_param_uint(name: &str, value: u32) -> Decl {
Decl::Param(name.to_string(), Expr::new_ulit_dec(32, &value.to_string()))
Decl::Param(
name.to_string(),
Expr::new_ulit_dec(32, &value.to_string()),
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are formatting changes


pub fn new_param_str(name: &str, value: &str) -> Decl {
Expand Down
8 changes: 6 additions & 2 deletions src/v05/pretty_print.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// use crate::util::pretty_print::{PrettyHelper, PrettyPrint, PRETTY_INDENT};
use crate::util::pretty_print::{block, block_with_parens, intersperse, PrettyHelper, PrettyPrint};
use crate::util::pretty_print::{
block, block_with_parens, intersperse, PrettyHelper, PrettyPrint,
};
use crate::v05::ast::*;
use pretty::RcDoc;

Expand Down Expand Up @@ -114,7 +116,9 @@ impl PrettyPrint for Sequential {
match self {
// wildcard for sensitivity list
Sequential::Wildcard => RcDoc::text("*"),
Sequential::Event(ty, expr) => ty.to_doc().append(RcDoc::space()).append(expr.to_doc()),
Sequential::Event(ty, expr) => {
ty.to_doc().append(RcDoc::space()).append(expr.to_doc())
}
Sequential::IfElse(ifelse) => ifelse.to_doc(),
Sequential::Assign(lexpr, rexpr, ty) => lexpr
.to_doc()
Expand Down
3 changes: 2 additions & 1 deletion src/v17/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ pub type Instance = subset::ast::Instance;
pub type CaseBranch = subset::ast::GenericCaseBranch<Sequential>;
pub type CaseDefault = subset::ast::GenericCaseDefault<Sequential>;
pub type Case = subset::ast::GenericCase<Sequential>;
pub type Function = subset::ast::GenericFunction<FunctionTy, Decl, Sequential, Ty>;
pub type Function =
subset::ast::GenericFunction<FunctionTy, Decl, Sequential, Ty>;
pub type Stmt = subset::ast::GenericStmt<Decl, Parallel>;
pub type Port = subset::ast::GenericPort<Decl>;
pub type Module = subset::ast::GenericModule<Decl, Parallel>;
Expand Down
13 changes: 10 additions & 3 deletions src/v17/pretty_print.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::subset::ast::Terop;
use crate::util::pretty_print::{block, block_with_parens, intersperse, PrettyHelper, PrettyPrint};
use crate::util::pretty_print::{
block, block_with_parens, intersperse, PrettyHelper, PrettyPrint,
};
use crate::v17::ast::*;
use pretty::RcDoc;

Expand Down Expand Up @@ -106,7 +108,10 @@ impl PrettyPrint for Function {
let body = if self.body().is_empty() {
RcDoc::nil()
} else {
intersperse(self.body().iter().map(|x| x.to_doc()), RcDoc::hardline())
intersperse(
self.body().iter().map(|x| x.to_doc()),
RcDoc::hardline(),
)
};
let args = RcDoc::space()
.append(self.ret.to_doc())
Expand Down Expand Up @@ -195,7 +200,9 @@ impl PrettyPrint for Sequential {
.append(RcDoc::text(";")),
Sequential::SeqCase(case) => case.to_doc(),
Sequential::Call(call) => call.to_doc().append(RcDoc::text(";")),
Sequential::Event(ty, expr) => ty.to_doc().append(RcDoc::space()).append(expr.to_doc()),
Sequential::Event(ty, expr) => {
ty.to_doc().append(RcDoc::space()).append(expr.to_doc())
}
Sequential::Assert(expr, branch) => {
let cond = RcDoc::text("assert").append(expr.to_doc().parens());
if let Some(block) = branch {
Expand Down
18 changes: 16 additions & 2 deletions tests/v05.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,18 @@ fn test_decl_array() {
check!(decl, exp);
}

#[test]
fn test_bit_shift_order_of_operations() {
let bin1 = Expr::new_ulit_bin(4, "1000");
let bin2 = Expr::new_ulit_bin(4, "0100");
let shift_by: i32 = 2;
let shift = Expr::new_shift_left(bin1, shift_by);
let add = Expr::new_add(bin2, shift);
let exp = "4'b0100 + (4'b1000 << 2)".to_string();
let res = add.to_string();
check!(res, exp);
}

#[test]
fn test_event_ty_posedge() {
let event = EventTy::Posedge;
Expand All @@ -184,7 +196,8 @@ fn test_event_ty_negedge() {

#[test]
fn test_sequential_event_posedge_clock() {
let seq = Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let seq =
Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let exp = "posedge clock".to_string();
let res = seq.to_string();
check!(res, exp);
Expand Down Expand Up @@ -280,7 +293,8 @@ end"#;
fn test_attribute_decl() {
let mut attr = Attribute::default();
attr.add_stmt("ram_style", "block");
let decl = Decl::new_attribute_decl(attr, Decl::new_reg("ram", 32)).to_string();
let decl =
Decl::new_attribute_decl(attr, Decl::new_reg("ram", 32)).to_string();
let gold = r#"(*ram_style = "block"*) reg [31:0] ram"#;
check!(decl, gold);
}
Expand Down
6 changes: 4 additions & 2 deletions tests/v17.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ end"#;

#[test]
fn test_seq_event_posedge_clock() {
let event = Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let event =
Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let res = event.to_string();
let exp = "posedge clock".to_string();
check!(res, exp);
Expand Down Expand Up @@ -623,7 +624,8 @@ fn test_module_with_always_comb() {
#[test]
fn test_module_with_always_ff() {
let exp = read_to_string("regression/v17/module_with_always_ff.v");
let event = Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let event =
Sequential::Event(EventTy::Posedge, Expr::Ref("clock".to_string()));
let mut always = ParallelProcess::new_always_ff();
always.add_seq(Sequential::new_display("hello sync world"));
always.set_event(event);
Expand Down