Skip to content

Commit

Permalink
Introduce a __revert intrinsic a disallow rvrt, ret, and retd
Browse files Browse the repository at this point in the history
… in `asm` blocks (FuelLabs#2890)

Closes FuelLabs#2884

Main changes:
1. Disallowing `rvrt`, `ret`, and `retd` in `asm` blocks.
2. Remove all checks that look for the opcodes above.
3. Add `__revert` intrinsic and make its type `Unknown` for now. It will
be `Never` in the future.
4. Add an IR instruction `revert` with no type. There are still some
things that should be ironed out related to that.

Also closes FuelLabs#2822.

FuelLabs#2876 is not fixed yet though.
This is related to the issues mentioned in (4) above.

Co-authored-by: Toby Hutton <[email protected]>
Co-authored-by: Alex Hansen <[email protected]>
  • Loading branch information
3 people authored Oct 5, 2022
1 parent 4e842c1 commit 85d4f94
Show file tree
Hide file tree
Showing 26 changed files with 266 additions and 119 deletions.
3 changes: 0 additions & 3 deletions sway-ast/src/expr/op_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ define_op_codes!(
(Ctmv, CtmvOpcode, "ctmv", (ret: reg, maturity: reg)),
(Ji, JiOpcode, "ji", (offset: imm)),
(Jnei, JneiOpcode, "jnei", (lhs: reg, rhs: reg, offset: imm)),
(Ret, RetOpcode, "ret", (value: reg)),
(Aloc, AlocOpcode, "aloc", (size: reg)),
(Cfei, CfeiOpcode, "cfei", (size: imm)),
(Cfsi, CfsiOpcode, "cfsi", (size: imm)),
Expand Down Expand Up @@ -258,8 +257,6 @@ define_op_codes!(
(reg_a: reg, reg_b: reg, addr: reg, size: reg)
),
(Mint, MintOpcode, "mint", (coins: reg)),
(Retd, RetdOpcode, "retd", (addr: reg, size: reg)),
(Rvrt, RvrtOpcode, "rvrt", (value: reg)),
(
Sldc,
SldcOpcode,
Expand Down
3 changes: 3 additions & 0 deletions sway-ast/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub enum Intrinsic {
Sub,
Mul,
Div,
Revert,
}

impl fmt::Display for Intrinsic {
Expand All @@ -39,6 +40,7 @@ impl fmt::Display for Intrinsic {
Intrinsic::Sub => "sub",
Intrinsic::Mul => "mul",
Intrinsic::Div => "div",
Intrinsic::Revert => "revert",
};
write!(f, "{}", s)
}
Expand All @@ -64,6 +66,7 @@ impl Intrinsic {
"__sub" => Sub,
"__mul" => Mul,
"__div" => Div,
"__revert" => Revert,
_ => return None,
})
}
Expand Down
12 changes: 12 additions & 0 deletions sway-core/src/asm_generation/asm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ impl<'ir> AsmBuilder<'ir> {
self.compile_ret_from_call(instr_val, ret_val, ty)
}
}
Instruction::Revert(revert_val) => self.compile_revert(instr_val, revert_val),
Instruction::StateLoadQuadWord { load_val, key } => check!(
self.compile_state_access_quad_word(
instr_val,
Expand Down Expand Up @@ -1308,6 +1309,17 @@ impl<'ir> AsmBuilder<'ir> {
}
}

fn compile_revert(&mut self, instr_val: &Value, revert_val: &Value) {
let owning_span = self.md_mgr.val_to_span(self.context, *instr_val);
let revert_reg = self.value_to_register(revert_val);

self.cur_bytecode.push(Op {
owning_span,
opcode: Either::Left(VirtualOp::RVRT(revert_reg)),
comment: "".into(),
});
}

fn offset_reg(
&mut self,
base_reg: &VirtualRegister,
Expand Down
4 changes: 3 additions & 1 deletion sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ fn compile_fn_with_args(
ret_val = Constant::get_unit(context);
}

let already_returns = compiler.current_block.is_terminated_by_ret(context);
let already_returns = compiler
.current_block
.is_terminated_by_ret_or_revert(context);

// Another special case: if the last expression in a function is a return then we don't want to
// add another implicit return instruction here, as `ret_val` will be unit regardless of the
Expand Down
14 changes: 13 additions & 1 deletion sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ impl FnCompiler {

match log_val.get_stripped_ptr_type(context) {
None => Err(CompileError::Internal(
"Unable to determine type for return statement expression.",
"Unable to determine type for logged value.",
span,
)),
Some(log_ty) => {
Expand Down Expand Up @@ -643,6 +643,18 @@ impl FnCompiler {
.ins(context)
.binary_op(op, lhs_value, rhs_value))
}
Intrinsic::Revert => {
let revert_code_val =
self.compile_expression(context, md_mgr, arguments[0].clone())?;

// The `revert` instruction
let span_md_idx = md_mgr.span_to_md(context, &span);
Ok(self
.current_block
.ins(context)
.revert(revert_code_val)
.add_metadatum(context, span_md_idx))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ impl fmt::Display for TypedIntrinsicFunctionKind {

impl DeterministicallyAborts for TypedIntrinsicFunctionKind {
fn deterministically_aborts(&self) -> bool {
self.arguments.iter().any(|x| x.deterministically_aborts())
matches!(self.kind, Intrinsic::Revert)
|| self.arguments.iter().any(|x| x.deterministically_aborts())
}
}

Expand Down Expand Up @@ -687,6 +688,59 @@ impl TypedIntrinsicFunctionKind {
insert_type(arg_ty),
)
}
Intrinsic::Revert => {
if arguments.len() != 1 {
errors.push(CompileError::IntrinsicIncorrectNumArgs {
name: kind.to_string(),
expected: 1,
span,
});
return err(warnings, errors);
}

if !type_arguments.is_empty() {
errors.push(CompileError::IntrinsicIncorrectNumTArgs {
name: kind.to_string(),
expected: 0,
span,
});
return err(warnings, errors);
}

// Type check the argument which is the revert code
let mut ctx = ctx
.by_ref()
.with_type_annotation(insert_type(TypeInfo::Unknown));
let revert_code = check!(
TypedExpression::type_check(ctx.by_ref(), arguments[0].clone()),
return err(warnings, errors),
warnings,
errors
);

// Make sure that the revert code is a `u64`
if !matches!(
to_typeinfo(revert_code.return_type, &revert_code.span).unwrap(),
TypeInfo::UnsignedInteger(IntegerBits::SixtyFour)
) {
errors.push(CompileError::IntrinsicUnsupportedArgType {
name: kind.to_string(),
span: revert_code.span.clone(),
hint: Hint::empty(),
});
}

(
TypedIntrinsicFunctionKind {
kind,
arguments: vec![revert_code],
type_arguments: vec![],
span,
},
insert_type(TypeInfo::Unknown), // TODO: change this to the `Never` type when
// available
)
}
};
ok((intrinsic_function, return_type), warnings, errors)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,23 +403,12 @@ impl DeterministicallyAborts for TypedExpression {
ArrayIndex { prefix, index } => {
prefix.deterministically_aborts() || index.deterministically_aborts()
}
AsmExpression {
registers, body, ..
} => {
// when asm expression parsing is handled earlier, this will be cleaner. For now,
// we rely on string comparison...
// jumps are not allowed in asm blocks, so we know this block deterministically
// aborts if these opcodes are present
let body_deterministically_aborts = body
.iter()
.any(|x| ["rvrt", "ret"].contains(&x.op_name.as_str().to_lowercase().as_str()));
registers.iter().any(|x| {
x.initializer
.as_ref()
.map(|x| x.deterministically_aborts())
.unwrap_or(false)
}) || body_deterministically_aborts
}
AsmExpression { registers, .. } => registers.iter().any(|x| {
x.initializer
.as_ref()
.map(|x| x.deterministically_aborts())
.unwrap_or(false)
}),
IfExp {
condition,
then,
Expand Down Expand Up @@ -1217,25 +1206,18 @@ impl TypedExpression {
.clone()
.map(|x| x.1)
.unwrap_or_else(|| asm.whole_block_span.clone());
let diverges = asm
.body
.iter()
.any(|asm_op| matches!(asm_op.op_name.as_str(), "rvrt" | "ret"));
let return_type = if diverges {
insert_type(TypeInfo::Unknown)
} else {
check!(
ctx.resolve_type_with_self(
insert_type(asm.return_type.clone()),
&asm_span,
EnforceTypeArguments::No,
None
),
insert_type(TypeInfo::ErrorRecovery),
warnings,
errors,
)
};
let return_type = check!(
ctx.resolve_type_with_self(
insert_type(asm.return_type.clone()),
&asm_span,
EnforceTypeArguments::No,
None
),
insert_type(TypeInfo::ErrorRecovery),
warnings,
errors,
);

// type check the initializers
let typed_registers = asm
.registers
Expand Down
12 changes: 12 additions & 0 deletions sway-core/tests/ir_to_asm/revert.asm
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
.program:
ji i4
noop
DATA_SECTION_OFFSET[0..32]
DATA_SECTION_OFFSET[32..64]
lw $ds $is 1
add $$ds $$ds $is
lw $r0 data_0 ; literal instantiation
rvrt $r0
noop ; word-alignment of data section
.data:
data_0 .word 42
7 changes: 7 additions & 0 deletions sway-core/tests/ir_to_asm/revert.ir
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
script {
fn main() -> () {
entry:
v0 = const u64 42
revert v0
}
}
8 changes: 0 additions & 8 deletions sway-ir/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,4 @@ impl AsmBlock {
pub fn get_content<'a>(&self, context: &'a Context) -> &'a AsmBlockContent {
&context.asm_blocks[self.0]
}

pub fn is_diverging(&self, context: &Context) -> bool {
let content = &context.asm_blocks[self.0];
content
.body
.iter()
.any(|asm_instruction| matches!(asm_instruction.name.as_str(), "rvrt" | "ret"))
}
}
7 changes: 4 additions & 3 deletions sway-ir/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,10 @@ impl Block {
}

/// Return whether this block is already terminated specifically by a Ret instruction.
pub fn is_terminated_by_ret(&self, context: &Context) -> bool {
self.get_terminator(context)
.map_or(false, |i| matches!(i, Instruction::Ret { .. }))
pub fn is_terminated_by_ret_or_revert(&self, context: &Context) -> bool {
self.get_terminator(context).map_or(false, |i| {
matches!(i, Instruction::Ret(..) | Instruction::Revert(..))
})
}

/// Replace a value within this block.
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 @@ -58,6 +58,7 @@ pub enum IrError {
VerifyInvalidGtfIndexType,
VerifyLogId,
VerifyMismatchedLoggedTypes,
VerifyRevertCodeBadType,
}

impl std::error::Error for IrError {}
Expand Down Expand Up @@ -310,6 +311,12 @@ impl fmt::Display for IrError {
"Verification failed: log type must match the type of the value being logged."
)
}
IrError::VerifyRevertCodeBadType => {
write!(
f,
"Verification failed: error code for revert must be a u64."
)
}
}
}
}
18 changes: 17 additions & 1 deletion sway-ir/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ pub enum Instruction {
ReadRegister(Register),
/// Return from a function.
Ret(Value, Type),
/// Revert VM execution.
Revert(Value),
/// Read a quad word from a storage slot. Type of `load_val` must be a B256 ptr.
StateLoadQuadWord {
load_val: Value,
Expand Down Expand Up @@ -232,6 +234,7 @@ impl Instruction {
Instruction::Branch(_) => None,
Instruction::ConditionalBranch { .. } => None,
Instruction::Ret(..) => None,
Instruction::Revert(..) => None,

Instruction::StateLoadQuadWord { .. } => Some(Type::Unit),
Instruction::StateStoreQuadWord { .. } => Some(Type::Unit),
Expand Down Expand Up @@ -354,6 +357,7 @@ impl Instruction {
Instruction::Phi(pairs) => pairs.iter_mut().for_each(|(_, val)| replace(val)),
Instruction::ReadRegister { .. } => (),
Instruction::Ret(ret_val, _) => replace(ret_val),
Instruction::Revert(revert_val) => replace(revert_val),
Instruction::StateLoadQuadWord { load_val, key } => {
replace(load_val);
replace(key);
Expand Down Expand Up @@ -406,14 +410,18 @@ impl Instruction {
| Instruction::Branch(_)
| Instruction::ConditionalBranch { .. }
| Instruction::Ret(..)
| Instruction::Revert(..)
| Instruction::Nop => false,
}
}

pub fn is_terminator(&self) -> bool {
matches!(
self,
Instruction::Branch(_) | Instruction::ConditionalBranch { .. } | Instruction::Ret(..)
Instruction::Branch(_)
| Instruction::ConditionalBranch { .. }
| Instruction::Ret(..)
| Instruction::Revert(..)
)
}
}
Expand Down Expand Up @@ -750,6 +758,14 @@ impl<'a> InstructionInserter<'a> {
ret_val
}

pub fn revert(self, value: Value) -> Value {
let revert_val = Value::new_instruction(self.context, Instruction::Revert(value));
self.context.blocks[self.block.0]
.instructions
.push(revert_val);
revert_val
}

pub fn state_load_quad_word(self, load_val: Value, key: Value) -> Value {
let state_load_val = Value::new_instruction(
self.context,
Expand Down
1 change: 1 addition & 0 deletions sway-ir/src/optimize/dce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub fn dce(context: &mut Context, function: &Function) -> Result<bool, IrError>
Instruction::Phi(ins) => ins.iter().map(|v| v.1).collect(),
Instruction::ReadRegister(_) => vec![],
Instruction::Ret(v, _) => vec![*v],
Instruction::Revert(v) => vec![*v],
Instruction::StateLoadQuadWord { load_val, key } => vec![*load_val, *key],
Instruction::StateLoadWord(key) => vec![*key],
Instruction::StateStoreQuadWord { stored_val, key } => vec![*stored_val, *key],
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 @@ -418,6 +418,7 @@ fn inline_instruction(
Instruction::Ret(val, _) => new_block
.ins(context)
.branch(*post_block, Some(map_value(val))),
Instruction::Revert(val) => new_block.ins(context).revert(map_value(val)),
Instruction::StateLoadQuadWord { load_val, key } => new_block
.ins(context)
.state_load_quad_word(map_value(load_val), map_value(key)),
Expand Down
Loading

0 comments on commit 85d4f94

Please sign in to comment.