Skip to content

Commit

Permalink
Fix: Constant initializer expressions cannot use bitwise not (FuelLab…
Browse files Browse the repository at this point in the history
…s#4739)

## Description

Closes FuelLabs#4711.

This PR fixes the problem when initializing a constant unsigned integer
using "bitwise not". Main issue was that at `ops.sw` the bitwise
operator was implemented using `AsmFunctions`, which are ignored in
const eval.

I have created a new `__not` intrinsic which now works (almost) as
expected. We still have two issues surrounding this "const eval":

1 - All integers are `u64`, so inside the `const eval`, we do not have
enough context to correct do the bitwise. That is why `Not` for u8, u16
and u32 do a `__and` afterwards;

2 - `bool` is not using the new intrinsic, so it does not support
`bool`.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: IGI-111 <[email protected]>
  • Loading branch information
xunilrj and IGI-111 authored Jul 5, 2023
1 parent 91a795d commit bbb89d1
Show file tree
Hide file tree
Showing 25 changed files with 320 additions and 33 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions docs/book/src/reference/compiler_intrinsics.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,12 @@ __smo<T>(recipient: b256, data: T, coins: u64)
**Constraints:** None.

___

```sway
__not(op: T) -> T
```

**Description:** Bitwise NOT of `op`

**Constraints:** `T` is an integer type, i.e. `u8`, `u16`, `u32`, `u64`.
___
3 changes: 3 additions & 0 deletions sway-ast/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub enum Intrinsic {
PtrAdd,
PtrSub,
Smo,
Not,
}

impl fmt::Display for Intrinsic {
Expand Down Expand Up @@ -67,6 +68,7 @@ impl fmt::Display for Intrinsic {
Intrinsic::PtrAdd => "ptr_add",
Intrinsic::PtrSub => "ptr_sub",
Intrinsic::Smo => "smo",
Intrinsic::Not => "not",
};
write!(f, "{s}")
}
Expand Down Expand Up @@ -106,6 +108,7 @@ impl Intrinsic {
"__ptr_add" => PtrAdd,
"__ptr_sub" => PtrSub,
"__smo" => Smo,
"__not" => Not,
_ => return None,
})
}
Expand Down
5 changes: 5 additions & 0 deletions sway-core/src/asm_generation/evm/evm_asm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ impl<'ir, 'eng> EvmAsmBuilder<'ir, 'eng> {
)
}
Instruction::BitCast(val, ty) => self.compile_bitcast(instr_val, val, ty),
Instruction::UnaryOp { op, arg } => self.compile_unary_op(instr_val, op, arg),
Instruction::BinaryOp { op, arg1, arg2 } => {
self.compile_binary_op(instr_val, op, arg1, arg2)
}
Expand Down Expand Up @@ -403,6 +404,10 @@ impl<'ir, 'eng> EvmAsmBuilder<'ir, 'eng> {
todo!();
}

fn compile_unary_op(&mut self, instr_val: &Value, op: &UnaryOpKind, arg: &Value) {
todo!();
}

fn compile_binary_op(
&mut self,
instr_val: &Value,
Expand Down
22 changes: 22 additions & 0 deletions sway-core/src/asm_generation/fuel/fuel_asm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {
match instruction {
Instruction::AsmBlock(..) => unreachable!("Handled immediately above."),
Instruction::BitCast(val, ty) => self.compile_bitcast(instr_val, val, ty),
Instruction::UnaryOp { op, arg } => self.compile_unary_op(instr_val, op, arg),
Instruction::BinaryOp { op, arg1, arg2 } => {
self.compile_binary_op(instr_val, op, arg1, arg2)
}
Expand Down Expand Up @@ -450,6 +451,27 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {
Ok(())
}

fn compile_unary_op(
&mut self,
instr_val: &Value,
op: &UnaryOpKind,
arg: &Value,
) -> Result<(), CompileError> {
let val_reg = self.value_to_register(arg)?;
let res_reg = self.reg_seqr.next();
let opcode = match op {
UnaryOpKind::Not => Either::Left(VirtualOp::NOT(res_reg.clone(), val_reg)),
};
self.cur_bytecode.push(Op {
opcode,
comment: String::new(),
owning_span: self.md_mgr.val_to_span(self.context, *instr_val),
});

self.reg_map.insert(*instr_val, res_reg);
Ok(())
}

fn compile_binary_op(
&mut self,
instr_val: &Value,
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/asm_generation/miden_vm/miden_vm_asm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ impl<'ir, 'eng> MidenVMAsmBuilder<'ir, 'eng> {
match instruction {
Instruction::AsmBlock(asm, args) => todo!(),
Instruction::BitCast(val, ty) => todo!(),
Instruction::UnaryOp { op, arg } => {
todo!()
}
Instruction::BinaryOp { op, arg1, arg2 } => {
todo!()
}
Expand Down
27 changes: 27 additions & 0 deletions sway-core/src/ir_generation/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,5 +743,32 @@ fn const_eval_intrinsic(
| sway_ast::Intrinsic::Log
| sway_ast::Intrinsic::Revert
| sway_ast::Intrinsic::Smo => Ok(None),
sway_ast::Intrinsic::Not => {
// Not works only with uint at the moment
// `bool` ops::Not implementation uses `__eq`.

assert!(args.len() == 1 && args[0].ty.is_uint(lookup.context));

let Some(arg) = args.into_iter().next() else {
unreachable!("Unexpected 'not' without any arguments");
};

let ConstantValue::Uint(v) = arg.value else {
unreachable!("Type checker allowed non integer value for Not");
};

let v = match arg.ty.get_uint_width(lookup.context) {
Some(8) => !(v as u8) as u64,
Some(16) => !(v as u16) as u64,
Some(32) => !(v as u32) as u64,
Some(64) => !v,
_ => unreachable!("Invalid unsigned integer width"),
};

Ok(Some(Constant {
ty: arg.ty,
value: ConstantValue::Uint(v),
}))
}
}
}
10 changes: 10 additions & 0 deletions sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,16 @@ impl<'eng> FnCompiler<'eng> {
.smo(recipient_var, message, user_message_size_val, coins)
.add_metadatum(context, span_md_idx))
}
Intrinsic::Not => {
assert!(arguments.len() == 1);

let op = &arguments[0];
let value = self.compile_expression_to_value(context, md_mgr, op)?;
Ok(self
.current_block
.ins(context)
.unary_op(UnaryOpKind::Not, value))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,83 @@ impl ty::TyIntrinsicFunctionKind {
type_check_ptr_ops(ctx, kind, arguments, type_arguments, span)
}
Intrinsic::Smo => type_check_smo(ctx, kind, arguments, type_arguments, span),
Intrinsic::Not => type_check_not(ctx, kind, arguments, type_arguments, span),
}
}
}

/// Signature: `__not(val: u64) -> u64`
/// Description: Return the bitwise negation of the operator.
/// Constraints: None.
fn type_check_not(
ctx: TypeCheckContext,
kind: sway_ast::Intrinsic,
arguments: Vec<Expression>,
_type_arguments: Vec<TypeArgument>,
span: Span,
) -> CompileResult<(ty::TyIntrinsicFunctionKind, TypeId)> {
let type_engine = ctx.engines.te();
let engines = ctx.engines();

let mut warnings = vec![];
let mut errors = vec![];

if arguments.len() != 1 {
errors.push(CompileError::IntrinsicIncorrectNumArgs {
name: kind.to_string(),
expected: 1,
span,
});
return err(warnings, errors);
}

let mut ctx = ctx
.with_help_text("")
.with_type_annotation(type_engine.insert(engines, TypeInfo::Unknown));

let operand = arguments[0].clone();
let operand_expr = check!(
ty::TyExpression::type_check(ctx.by_ref(), operand),
return err(warnings, errors),
warnings,
errors
);

let operand_typeinfo = check!(
CompileResult::from(
type_engine
.to_typeinfo(operand_expr.return_type, &operand_expr.span)
.map_err(CompileError::from)
),
TypeInfo::ErrorRecovery,
warnings,
errors
);
let is_valid_arg_ty = matches!(operand_typeinfo, TypeInfo::UnsignedInteger(_));
if !is_valid_arg_ty {
errors.push(CompileError::IntrinsicUnsupportedArgType {
name: kind.to_string(),
span: operand_expr.span,
hint: Hint::empty(),
});
return err(warnings, errors);
}

ok(
(
ty::TyIntrinsicFunctionKind {
kind,
arguments: vec![operand_expr],
type_arguments: vec![],
span,
},
type_engine.insert(engines, operand_typeinfo),
),
warnings,
errors,
)
}

/// Signature: `__size_of_val<T>(val: T) -> u64`
/// Description: Return the size of type `T` in bytes.
/// Constraints: None.
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/cei_pattern_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ fn effects_of_intrinsic(intr: &sway_ast::Intrinsic) -> HashSet<Effect> {
Smo => HashSet::from([Effect::OutputMessage]),
Revert | IsReferenceType | IsStrType | SizeOfType | SizeOfVal | SizeOfStr | Eq | Gt
| Lt | Gtf | AddrOf | Log | Add | Sub | Mul | Div | And | Or | Xor | Mod | Rsh | Lsh
| PtrAdd | PtrSub => HashSet::new(),
| PtrAdd | PtrSub | Not => HashSet::new(),
}
}

Expand Down
6 changes: 3 additions & 3 deletions sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,11 @@ fn item_to_ast_nodes(
ItemKind::Abi(item_abi) => decl(Declaration::AbiDeclaration(item_abi_to_abi_declaration(
context, handler, engines, item_abi, attributes,
)?)),
ItemKind::Const(item_const) => decl(Declaration::ConstantDeclaration(
ItemKind::Const(item_const) => decl(Declaration::ConstantDeclaration({
item_const_to_constant_declaration(
context, handler, engines, item_const, attributes, true,
)?,
)),
)?
})),
ItemKind::Storage(item_storage) => decl(Declaration::StorageDeclaration(
item_storage_to_storage_declaration(
context,
Expand Down
1 change: 1 addition & 0 deletions sway-ir/src/analysis/escaped_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub fn compute_escaped_symbols(context: &Context, function: &Function) -> Escape
add_from_val(&mut result, &arg_init)
}
}
Instruction::UnaryOp { .. } => (),
Instruction::BinaryOp { .. } => (),
Instruction::BitCast(_, _) => (),
Instruction::Branch(_) => (),
Expand Down
7 changes: 7 additions & 0 deletions sway-ir/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub enum IrError {
ValueNotFound(String),

VerifyArgumentValueIsNotArgument(String),
VerifyUnaryOpIncorrectArgType,
VerifyBinaryOpIncorrectArgType,
VerifyBitcastBetweenInvalidTypes(String, String),
VerifyBitcastUnknownSourceType,
Expand Down Expand Up @@ -116,6 +117,12 @@ impl fmt::Display for IrError {
f,
"Verification failed: Bitcast not allowed from a {from_ty} to a {to_ty}."
),
IrError::VerifyUnaryOpIncorrectArgType => {
write!(
f,
"Verification failed: Incorrect argument type for unary op"
)
}
IrError::VerifyBinaryOpIncorrectArgType => {
write!(
f,
Expand Down
19 changes: 18 additions & 1 deletion sway-ir/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub struct BranchToWithArgs {
pub enum Instruction {
/// An opaque list of ASM instructions passed directly to codegen.
AsmBlock(AsmBlock, Vec<AsmArg>),
/// Unary arithmetic operations
UnaryOp { op: UnaryOpKind, arg: Value },
/// Binary arithmetic operations
BinaryOp {
op: BinaryOpKind,
Expand Down Expand Up @@ -163,6 +165,11 @@ pub enum Predicate {
GreaterThan,
}

#[derive(Debug, Clone, Copy)]
pub enum UnaryOpKind {
Not,
}

#[derive(Debug, Clone, Copy)]
pub enum BinaryOpKind {
Add,
Expand Down Expand Up @@ -219,6 +226,7 @@ impl Instruction {
match self {
// These all return something in particular.
Instruction::AsmBlock(asm_block, _) => Some(asm_block.get_type(context)),
Instruction::UnaryOp { arg, .. } => arg.get_type(context),
Instruction::BinaryOp { arg1, .. } => arg1.get_type(context),
Instruction::BitCast(_, ty) => Some(*ty),
Instruction::Call(function, _) => Some(context.functions[function.0].return_type),
Expand Down Expand Up @@ -281,6 +289,7 @@ impl Instruction {
match self {
Instruction::AsmBlock(_, args) => args.iter().filter_map(|aa| aa.initializer).collect(),
Instruction::BitCast(v, _) => vec![*v],
Instruction::UnaryOp { op: _, arg } => vec![*arg],
Instruction::BinaryOp { op: _, arg1, arg2 } => vec![*arg1, *arg2],
Instruction::Branch(BranchToWithArgs { args, .. }) => args.clone(),
Instruction::Call(_, vs) => vs.clone(),
Expand Down Expand Up @@ -395,6 +404,9 @@ impl Instruction {
.for_each(|init_val| replace(init_val))
}),
Instruction::BitCast(value, _) => replace(value),
Instruction::UnaryOp { op: _, arg } => {
replace(arg);
}
Instruction::BinaryOp { op: _, arg1, arg2 } => {
replace(arg1);
replace(arg2);
Expand Down Expand Up @@ -540,7 +552,8 @@ impl Instruction {
| Instruction::Store { .. }
| Instruction::Ret(..) => true,

Instruction::BinaryOp { .. }
Instruction::UnaryOp { .. }
| Instruction::BinaryOp { .. }
| Instruction::BitCast(..)
| Instruction::Branch(_)
| Instruction::CastPtr { .. }
Expand Down Expand Up @@ -670,6 +683,10 @@ impl<'a, 'eng> InstructionInserter<'a, 'eng> {
make_instruction!(self, Instruction::BitCast(value, ty))
}

pub fn unary_op(self, op: UnaryOpKind, arg: Value) -> Value {
make_instruction!(self, Instruction::UnaryOp { op, arg })
}

pub fn binary_op(self, op: BinaryOpKind, arg1: Value, arg2: Value) -> Value {
make_instruction!(self, Instruction::BinaryOp { op, arg1, arg2 })
}
Expand Down
6 changes: 4 additions & 2 deletions sway-ir/src/optimize/dce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ fn can_eliminate_instruction(

fn get_loaded_symbols(context: &Context, val: Value) -> Vec<Symbol> {
match val.get_instruction(context).unwrap() {
Instruction::BinaryOp { .. }
Instruction::UnaryOp { .. }
| Instruction::BinaryOp { .. }
| Instruction::BitCast(_, _)
| Instruction::Branch(_)
| Instruction::ConditionalBranch { .. }
Expand Down Expand Up @@ -123,7 +124,8 @@ fn get_loaded_symbols(context: &Context, val: Value) -> Vec<Symbol> {

fn get_stored_symbols(context: &Context, val: Value) -> Vec<Symbol> {
match val.get_instruction(context).unwrap() {
Instruction::BinaryOp { .. }
Instruction::UnaryOp { .. }
| Instruction::BinaryOp { .. }
| Instruction::BitCast(_, _)
| Instruction::Branch(_)
| Instruction::ConditionalBranch { .. }
Expand Down
1 change: 1 addition & 0 deletions sway-ir/src/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ fn inline_instruction(
new_block.ins(context).asm_block_from_asm(asm, new_args)
}
Instruction::BitCast(value, ty) => new_block.ins(context).bitcast(map_value(value), ty),
Instruction::UnaryOp { op, arg } => new_block.ins(context).unary_op(op, map_value(arg)),
Instruction::BinaryOp { op, arg1, arg2 } => {
new_block
.ins(context)
Expand Down
Loading

0 comments on commit bbb89d1

Please sign in to comment.