Skip to content

Commit

Permalink
Fix wrong safemath lowering
Browse files Browse the repository at this point in the history
  • Loading branch information
Y-Nak authored and cburgdorf committed May 27, 2022
1 parent f810961 commit 4211cd4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
19 changes: 15 additions & 4 deletions crates/codegen/src/yul/isel/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ impl<'db, 'a> FuncLowerHelper<'db, 'a> {
}
)}
}

fn lower_assign(&mut self, lhs: &AssignableValue, rhs: ValueId) -> yul::Statement {
match lhs {
AssignableValue::Value(value) => {
Expand Down Expand Up @@ -494,14 +495,23 @@ impl<'db, 'a> FuncLowerHelper<'db, 'a> {
}

fn lower_unary(&mut self, op: UnOp, value: ValueId) -> yul::Expression {
let value = self.value_expr(value);
let value_expr = self.value_expr(value);
match op {
UnOp::Not => expression! { iszero([value])},
UnOp::Not => expression! { iszero([value_expr])},
UnOp::Neg => {
let zero = literal_expression! {0};
expression! { sub([zero], [value])}
if self.body.store.value_data(value).is_imm() {
// Literals are checked at compile time (e.g. -128) so there's no point
// in adding a runtime check.
expression! {sub([zero], [value_expr])}
} else {
let value_ty = self.body.store.value_ty(value);
self.ctx
.runtime
.safe_sub(self.db, zero, value_expr, value_ty)
}
}
UnOp::Inv => expression! { not([value])},
UnOp::Inv => expression! { not([value_expr])},
}
}

Expand Down Expand Up @@ -535,6 +545,7 @@ impl<'db, 'a> FuncLowerHelper<'db, 'a> {
BinOp::Mod => self.ctx.runtime.safe_mod(self.db, lhs, rhs, inst_result_ty),
BinOp::Pow => self.ctx.runtime.safe_pow(self.db, lhs, rhs, inst_result_ty),
BinOp::Shl => expression! {shl([rhs], [lhs])},
BinOp::Shr if is_signed => expression! {sar([rhs], [lhs])},
BinOp::Shr => expression! {shr([rhs], [lhs])},
BinOp::BitOr | BinOp::LogicalOr => expression! {or([lhs], [rhs])},
BinOp::BitXor => expression! {xor([lhs], [rhs])},
Expand Down
4 changes: 4 additions & 0 deletions crates/mir/src/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ impl Value {
| Self::Constant { ty, .. } => *ty,
}
}

pub fn is_imm(&self) -> bool {
matches!(self, Self::Immediate { .. })
}
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
Expand Down
2 changes: 2 additions & 0 deletions newsfragments/723.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* Properly lower right shift operation to yul's `sar` if operand is signed type
* Properly lower negate operation to call `safe_sub`

0 comments on commit 4211cd4

Please sign in to comment.