Skip to content

Commit

Permalink
Return reference type values from calls. (FuelLabs#3055)
Browse files Browse the repository at this point in the history 
Implement changes to call to allow returning reference-type values.

Uses an auxiliary 'out' parameter to receive the value, uses `mem_copy`
to set it and then returns the pointer itself.
  • Loading branch information
@otrho
otrho authored Oct 18, 2022
1 parent 335f4b0 commit 9f31df0
Show file tree
Hide file tree
Showing 30 changed files with 559 additions and 348 deletions.
40 changes: 38 additions & 2 deletions sway-core/src/asm_generation/asm_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
};

use crate::{
asm_lang::{virtual_register::*, Label, Op, VirtualImmediate12, VirtualOp},
asm_lang::{virtual_register::*, Label, Op, VirtualImmediate12, VirtualImmediate18, VirtualOp},
error::*,
metadata::MetadataManager,
size_bytes_in_words, size_bytes_round_up_to_word_alignment,
Expand Down Expand Up @@ -212,13 +212,18 @@ impl<'ir> AsmBuilder<'ir> {
log_ty,
log_id,
} => self.compile_log(instr_val, log_val, log_ty, log_id),
Instruction::MemCopy {
dst_val,
src_val,
byte_len,
} => self.compile_mem_copy(instr_val, dst_val, src_val, *byte_len),
Instruction::Nop => (),
Instruction::ReadRegister(reg) => self.compile_read_register(instr_val, reg),
Instruction::Ret(ret_val, ty) => {
if func_is_entry {
self.compile_ret_from_entry(instr_val, ret_val, ty)
} else {
self.compile_ret_from_call(instr_val, ret_val, ty)
self.compile_ret_from_call(instr_val, ret_val)
}
}
Instruction::Revert(revert_val) => self.compile_revert(instr_val, revert_val),
Expand Down Expand Up @@ -1200,6 +1205,37 @@ impl<'ir> AsmBuilder<'ir> {
ok((), Vec::new(), Vec::new())
}

fn compile_mem_copy(
&mut self,
instr_val: &Value,
dst_val: &Value,
src_val: &Value,
byte_len: u64,
) {
let owning_span = self.md_mgr.val_to_span(self.context, *instr_val);

let dst_reg = self.value_to_register(dst_val);
let src_reg = self.value_to_register(src_val);

let len_reg = self.reg_seqr.next();
self.cur_bytecode.push(Op {
opcode: Either::Left(VirtualOp::MOVI(
len_reg.clone(),
VirtualImmediate18 {
value: byte_len as u32,
},
)),
comment: "get length for mcp".into(),
owning_span: owning_span.clone(),
});

self.cur_bytecode.push(Op {
opcode: Either::Left(VirtualOp::MCP(dst_reg, src_reg, len_reg)),
comment: "copy memory with mem_copy".into(),
owning_span,
});
}

fn compile_log(&mut self, instr_val: &Value, log_val: &Value, log_ty: &Type, log_id: &Value) {
let owning_span = self.md_mgr.val_to_span(self.context, *instr_val);
let log_val_reg = self.value_to_register(log_val);
Expand Down
26 changes: 1 addition & 25 deletions sway-core/src/asm_generation/asm_builder/functions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,6 @@ use either::Either;
impl<'ir> AsmBuilder<'ir> {
pub(super) fn compile_call(&mut self, instr_val: &Value, function: &Function, args: &[Value]) {
if !function.get_return_type(self.context).is_copy_type() {
// To implement non-copy type return values we will transform functions to return their
// value via an 'out' argument, either during IR generation or possibly with an IR
// transformation.
//
// This hasn't been done yet and will be addressed in a future change. Until then we
// will enforce functions returning non-copy type values are always inlined, and so
// we will not see them at this stage of the compiler.
unimplemented!(
"Can't do reference type return values yet (and should've been inlined). {}",
function.get_name(self.context)
)
}

// Put the args into the args registers.
for (idx, arg_val) in args.iter().enumerate() {
if idx < compiler_constants::NUM_ARG_REGISTERS as usize {
Expand Down Expand Up @@ -116,17 +102,7 @@ impl<'ir> AsmBuilder<'ir> {
self.reg_map.insert(*instr_val, ret_reg);
}

pub(super) fn compile_ret_from_call(
&mut self,
instr_val: &Value,
ret_val: &Value,
ret_type: &Type,
) {
if !ret_type.is_copy_type() {
// See above in compile_call().
unimplemented!("Can't do reference type return values yet. {ret_type:?}")
}

pub(super) fn compile_ret_from_call(&mut self, instr_val: &Value, ret_val: &Value) {
// Move the result into the return value register.
let owning_span = self.md_mgr.val_to_span(self.context, *instr_val);
let ret_reg = self.value_to_register(ret_val);
Expand Down
38 changes: 26 additions & 12 deletions sway-core/src/ir_generation/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,30 @@ fn compile_fn_with_args(
..
} = ast_fn_decl;

let args = args
let mut args = args
.into_iter()
.map(|(name, ty, span)| (name, ty, md_mgr.span_to_md(context, &span)))
.collect();
.collect::<Vec<_>>();

let ret_type = convert_resolved_typeid(context, &return_type, &return_type_span)?;

let is_entry = selector.is_some()
|| (matches!(module.get_kind(context), Kind::Script | Kind::Predicate)
&& name.as_str() == "main");
let returns_by_ref = !is_entry && !ret_type.is_copy_type();
if returns_by_ref {
// Instead of 'returning' a by-ref value we make the last argument an 'out' parameter.
args.push((
"__ret_value".to_owned(),
Type::Pointer(Pointer::new(context, ret_type, true, None)),
md_mgr.span_to_md(context, &return_type_span),
));
}

let span_md_idx = md_mgr.span_to_md(context, &span);
let storage_md_idx = md_mgr.purity_to_md(context, purity);
let metadata = md_combine(context, &span_md_idx, &storage_md_idx);

let func = Function::new(
context,
module,
Expand All @@ -215,25 +231,16 @@ fn compile_fn_with_args(
metadata,
);

// We clone the struct symbols here, as they contain the globals; any new local declarations
// may remain within the function scope.
let mut compiler = FnCompiler::new(context, module, func);

let mut compiler = FnCompiler::new(context, module, func, returns_by_ref);
let mut ret_val = compiler.compile_code_block(context, md_mgr, body)?;

// Special case: if the return type is unit but the return value type is not, then we have an
// implicit return from the last expression in the code block having a semi-colon. This isn't
// codified in the AST explicitly so we need to make a unit to return here.
if ret_type.eq(context, &Type::Unit) && !matches!(ret_val.get_type(context), Some(Type::Unit)) {
// NOTE: We're replacing the ret_val and throwing away whatever it used to be, as it is
// never actually used anyway.
ret_val = Constant::get_unit(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
// function return type actually is. This new RET will be going into an unreachable block
Expand All @@ -243,11 +250,18 @@ fn compile_fn_with_args(
// To tell if this is the case we can check that the current block is empty and has no
// predecessors (and isn't the entry block which has none by definition), implying the most
// recent instruction was a RET.
let already_returns = compiler
.current_block
.is_terminated_by_ret_or_revert(context);
if !already_returns
&& (compiler.current_block.num_instructions(context) > 0
|| compiler.current_block == compiler.function.get_entry_block(context)
|| compiler.current_block.num_predecessors(context) > 0)
{
if returns_by_ref {
// Need to copy ref-type return values to the 'out' parameter.
ret_val = compiler.compile_copy_to_last_arg(context, ret_val, None);
}
if ret_type.eq(context, &Type::Unit) {
ret_val = Constant::get_unit(context);
}
Expand Down
96 changes: 76 additions & 20 deletions sway-core/src/ir_generation/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,21 @@ pub(super) struct FnCompiler {
module: Module,
pub(super) function: Function,
pub(super) current_block: Block,
pub(super) block_to_break_to: Option<Block>,
pub(super) block_to_continue_to: Option<Block>,
pub(super) current_fn_param: Option<ty::TyFunctionParameter>,
block_to_break_to: Option<Block>,
block_to_continue_to: Option<Block>,
current_fn_param: Option<ty::TyFunctionParameter>,
returns_by_ref: bool,
lexical_map: LexicalMap,
recreated_fns: HashMap<(Span, Vec<TypeId>, Vec<TypeId>), Function>,
}

impl FnCompiler {
pub(super) fn new(context: &mut Context, module: Module, function: Function) -> Self {
pub(super) fn new(
context: &mut Context,
module: Module,
function: Function,
returns_by_ref: bool,
) -> Self {
let lexical_map = LexicalMap::from_iter(
function
.args_iter(context)
Expand All @@ -54,6 +60,7 @@ impl FnCompiler {
block_to_break_to: None,
block_to_continue_to: None,
lexical_map,
returns_by_ref,
recreated_fns: HashMap::new(),
current_fn_param: None,
}
Expand Down Expand Up @@ -186,8 +193,7 @@ impl FnCompiler {
}
}
ty::TyAstNodeContent::ImplicitReturnExpression(te) => {
let value = self.compile_expression(context, md_mgr, te)?;
Ok(Some(value))
self.compile_expression(context, md_mgr, te).map(Some)
}
// a side effect can be () because it just impacts the type system/namespacing.
// There should be no new IR generated.
Expand Down Expand Up @@ -674,25 +680,50 @@ impl FnCompiler {
}

let ret_value = self.compile_expression(context, md_mgr, ast_expr.clone())?;

if ret_value.is_diverging(context) {
return Ok(ret_value);
}

let span_md_idx = md_mgr.span_to_md(context, &ast_expr.span);

if self.returns_by_ref {
// We need to copy the actual return value to the out parameter.
self.compile_copy_to_last_arg(context, ret_value, span_md_idx);
}

match ret_value.get_stripped_ptr_type(context) {
None => Err(CompileError::Internal(
"Unable to determine type for return statement expression.",
ast_expr.span,
)),
Some(ret_ty) => {
let span_md_idx = md_mgr.span_to_md(context, &ast_expr.span);
Ok(self
.current_block
.ins(context)
.ret(ret_value, ret_ty)
.add_metadatum(context, span_md_idx))
}
Some(ret_ty) => Ok(self
.current_block
.ins(context)
.ret(ret_value, ret_ty)
.add_metadatum(context, span_md_idx)),
}
}

pub(super) fn compile_copy_to_last_arg(
&mut self,
context: &mut Context,
ret_val: Value,
span_md_idx: Option<MetadataIndex>,
) -> Value {
let dst_val = self.function.args_iter(context).last().unwrap().1;
let src_val = ret_val;
let byte_len =
ir_type_size_in_bytes(context, &src_val.get_stripped_ptr_type(context).unwrap());

self.current_block
.ins(context)
.mem_copy(dst_val, src_val, byte_len)
.add_metadatum(context, span_md_idx);

dst_val
}

fn compile_lazy_op(
&mut self,
context: &mut Context,
Expand Down Expand Up @@ -1027,7 +1058,7 @@ impl FnCompiler {
};

// Now actually call the new function.
let args = {
let mut args = {
let mut args = Vec::with_capacity(ast_args.len());
for ((_, expr), param) in ast_args.into_iter().zip(callee.parameters.into_iter()) {
self.current_fn_param = Some(param);
Expand All @@ -1040,12 +1071,37 @@ impl FnCompiler {
}
args
};
let state_idx_md_idx = match self_state_idx {
Some(self_state_idx) => {
md_mgr.storage_key_to_md(context, self_state_idx.to_usize() as u64)

// If there is an 'unexpected' extra arg in the callee and it's a pointer then we need to
// set up returning by reference.
if args.len() + 1 == new_callee.num_args(context) {
if let Some(Type::Pointer(ptr)) = new_callee
.args_iter(context)
.last()
.unwrap()
.1
.get_argument_type(context)
{
// Create a local to pass in as the 'out' parameter.
let ptr_type = *ptr.get_type(context);
let local_name = format!("__ret_val_{}", new_callee.get_name(context));
let local_ptr = self
.function
.new_unique_local_ptr(context, local_name, ptr_type, true, None);

// Pass it as the final arg.
args.push(
self.current_block
.ins(context)
.get_ptr(local_ptr, ptr_type, 0),
);
}
None => None,
};
}

let state_idx_md_idx = self_state_idx.and_then(|self_state_idx| {
md_mgr.storage_key_to_md(context, self_state_idx.to_usize() as u64)
});

Ok(self
.current_block
.ins(context)
Expand Down
10 changes: 4 additions & 6 deletions sway-core/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,17 +475,15 @@ fn inline_function_calls(

let inline_heuristic = |ctx: &Context, func: &Function, _call_site: &Value| {
// For now, pending improvements to ASMgen for calls, we must inline any function which has
// a non-copy return type or has too many args.
if !func.get_return_type(ctx).is_copy_type()
|| func.args_iter(ctx).count() as u8
> crate::asm_generation::compiler_constants::NUM_ARG_REGISTERS
// too many args.
if func.args_iter(ctx).count() as u8
> crate::asm_generation::compiler_constants::NUM_ARG_REGISTERS
{
return true;
}

// If the function is called only once then definitely inline it.
let call_count = call_counts.get(func).copied().unwrap_or(0);
if call_count == 1 {
if call_counts.get(func).copied().unwrap_or(0) == 1 {
return true;
}

Expand Down
22 changes: 0 additions & 22 deletions sway-ir/src/constant.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,6 @@ pub enum ConstantValue {
}

impl Constant {
pub fn new_undef(context: &Context, ty: Type) -> Self {
match ty {
Type::Array(aggregate) => {
let (elem_type, count) = context.aggregates[aggregate.0].array_type();
let elem_init = Self::new_undef(context, *elem_type);
Constant::new_array(&aggregate, vec![elem_init; *count as usize])
}
Type::Struct(aggregate) => {
let field_types = context.aggregates[aggregate.0].field_types().clone();
let field_inits = field_types
.into_iter()
.map(|field_type| Self::new_undef(context, field_type)) // Hrmm, recursive structures would break here.
.collect();
Constant::new_struct(&aggregate, field_inits)
}
_otherwise => Constant {
ty,
value: ConstantValue::Undef,
},
}
}

pub fn new_unit() -> Self {
Constant {
ty: Type::Unit,
Expand Down
Loading

0 comments on commit 9f31df0

Please sign in to comment.