Skip to content

Commit

Permalink
Do not force inling functions that have pointer args. (#5762)
Browse files Browse the repository at this point in the history
The new encoding can handle functions with pointer arguments.

So this change reverts #4899. That PR also added a test which now passes
without the forced inlining.

Also see #2819, which first introduced this hack.
  • Loading branch information
vaivaswatha authored Apr 16, 2024
1 parent fc572d5 commit b94d3d7
Show file tree
Hide file tree
Showing 28 changed files with 134 additions and 61 deletions.
6 changes: 5 additions & 1 deletion sway-core/src/asm_generation/fuel/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,11 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {
self.cur_bytecode.push(Op::register_move(
locals_base_reg.clone(),
VirtualRegister::Constant(ConstantRegister::StackPointer),
"save locals base register",
format!(
"save locals base register for {}",
function.get_name(self.context)
)
.to_string(),
None,
));

Expand Down
41 changes: 30 additions & 11 deletions sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,25 +295,44 @@ impl<'eng> FnCompiler<'eng> {
md_mgr: &mut MetadataManager,
ast_expr: &ty::TyExpression,
) -> Result<TerminatorValue, CompileError> {
// Compile expression which *may* be a pointer. We can't return a value so create a
// temporary here, store the value and return its pointer.
// Compile expression which *may* be a pointer.
let val =
return_on_termination_or_extract!(self.compile_expression(context, md_mgr, ast_expr)?);
let ty = match val.get_type(context) {
Some(ty) if !ty.is_ptr(context) => ty,
_ => return Ok(TerminatorValue::new(val, context)),
};

// Create a temporary.
let temp_name = self.lexical_map.insert_anon();
let tmp_var = self
.function
.new_local_var(context, temp_name, ty, None, false)
.map_err(|ir_error| CompileError::InternalOwned(ir_error.to_string(), Span::dummy()))?;
let tmp_val = self.current_block.append(context).get_local(tmp_var);
self.current_block.append(context).store(tmp_val, val);
let is_argument = val.get_argument(context).is_some_and(|arg| {
arg.block.get_function(context).get_entry_block(context) == arg.block
});

let ptr_val = if is_argument {
// The `ptr_to_int` instructions gets the address of a variable into an integer.
// We then cast it back to a pointer.
let ptr_ty = Type::new_ptr(context, ty);
let int_ty = Type::get_uint64(context);
let ptr_to_int = self.current_block.append(context).ptr_to_int(val, int_ty);
let int_to_ptr = self
.current_block
.append(context)
.int_to_ptr(ptr_to_int, ptr_ty);
int_to_ptr
} else {
// We can't return a value so create a temporary here, store the value and return its pointer.
let temp_name = self.lexical_map.insert_anon();
let tmp_var = self
.function
.new_local_var(context, temp_name, ty, None, false)
.map_err(|ir_error| {
CompileError::InternalOwned(ir_error.to_string(), Span::dummy())
})?;
let tmp_val = self.current_block.append(context).get_local(tmp_var);
self.current_block.append(context).store(tmp_val, val);
tmp_val
};

Ok(TerminatorValue::new(tmp_val, context))
Ok(TerminatorValue::new(ptr_val, context))
}

fn compile_string_slice(
Expand Down
9 changes: 0 additions & 9 deletions sway-ir/src/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,6 @@ pub fn inline_in_module(
return true;
}

// See https://github.com/FuelLabs/sway/pull/4899
if func.args_iter(ctx).any(|(_name, arg_val)| {
arg_val.get_type(ctx).map_or(false, |ty| {
ty.is_ptr(ctx) || !(ty.is_unit(ctx) | ty.is_bool(ctx) | ty.is_uint(ctx))
})
}) {
return true;
}

false
};

Expand Down
8 changes: 8 additions & 0 deletions sway-ir/src/optimize/memcpyopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ fn local_copy_prop_prememcpy(context: &mut Context, function: Function) -> Resul
instr_info_map.insert(inst, info());
}
}
Instruction {
op: InstOp::PtrToInt(value, _),
..
} => {
if let Some(local) = get_symbol(context, *value) {
escaping_uses.insert(local);
}
}
Instruction {
op: InstOp::AsmBlock(_, args),
..
Expand Down
16 changes: 14 additions & 2 deletions sway-ir/src/optimize/misc_demotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,21 @@ fn ptr_to_int_demotion(context: &mut Context, function: Function) -> Result<bool
return Ok(false);
}

// Take the ptr_to_int value, store it in a temporary local, and replace it with its pointer in
// the ptr_to_int instruction.
for (block, ptr_to_int_instr_val, ptr_val, ptr_ty) in candidates {
// If the ptr_val is a load from a memory location, we can just refer to that.
if let Some(instr) = ptr_val.get_instruction(context) {
if let Some(loaded_val) = match instr.op {
InstOp::Load(loaded_val) => Some(loaded_val),
_ => None,
} {
ptr_to_int_instr_val.replace_instruction_value(context, ptr_val, loaded_val);
continue;
}
}

// Take the ptr_to_int value, store it in a temporary local, and replace it with its pointer in
// the ptr_to_int instruction.

// Create a variable for the arg, a get_local for it and a store.
let loc_var = function.new_unique_local_var(
context,
Expand Down
21 changes: 14 additions & 7 deletions sway-ir/tests/demote_misc/demote_ptr_to_int.ir
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
script {
fn test(v1: b256) -> u64 {
entry(v1: b256):
v2 = ptr_to_int v1 to u64
ret u64 v2
}
entry fn main() -> u64 {
local b256 foo = const b256 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b

entry():
v0 = get_local ptr b256, foo
v1 = load v0
v2 = ptr_to_int v1 to u64
ret u64 v2
foo_ptr = get_local ptr b256, foo
foo = load foo_ptr
v1 = call test(foo)
ret u64 v1
}
}

// regex: VAL=v\d+
// regex: ID=[[:alpha:]0-9_]+

// check: fn test($(foo_val=$ID): b256) -> u64
// check: local b256 $(tmp_loc=$ID)
// check: local b256 foo = const b256 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b

// check: $(foo_ptr=$VAL) = get_local ptr b256, foo
// check: $(foo_val=$VAL) = load $foo_ptr
// check: $(tmp_ptr=$VAL) = get_local ptr b256, $tmp_loc
// check: store $foo_val to $tmp_ptr
// check: $(tmp_ptr_int=$VAL) = ptr_to_int $tmp_ptr to u64
// check: ret u64 $tmp_ptr_int

// check: entry fn main() -> u64
// check: local b256 foo = const b256 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b
// check: $(foo_ptr=$VAL) = get_local ptr b256, foo
// check: $(foo_val=$VAL) = load $foo_ptr
11 changes: 11 additions & 0 deletions sway-lib-std/src/bytes.sw
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use ::assert::{assert, assert_eq};
use ::intrinsics::size_of_val;
use ::option::Option::{self, *};
use ::convert::{From, Into, *};
use ::clone::Clone;

struct RawBytes {
ptr: raw_ptr,
Expand Down Expand Up @@ -907,6 +908,16 @@ impl From<Bytes> for Vec<u8> {
}
}

impl Clone for Bytes {
fn clone(self) -> Self {
let len = self.len();
let mut c = Self::with_capacity(len);
c.len = len;
self.ptr().copy_bytes_to(c.ptr(), len);
c
}
}

impl AbiEncode for Bytes {
fn abi_encode(self, ref mut buffer: Buffer) {
buffer.push_u64(self.len);
Expand Down
8 changes: 8 additions & 0 deletions sway-lib-std/src/clone.sw
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//! The clone trait, for explicit duplication.
library;

/// A common trait for the ability to explicitly duplicate an object.
pub trait Clone {
/// Clone self into a new value of the same type.
fn clone(self) -> Self;
}
1 change: 1 addition & 0 deletions sway-lib-std/src/lib.sw
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,6 @@ pub mod prelude;
pub mod low_level_call;
pub mod array_conversions;
pub mod bytes_conversions;
pub mod clone;

use core::*;
4 changes: 2 additions & 2 deletions sway-lib-std/src/low_level_call.sw
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,10 @@ fn create_payload(
// let mut payload = Bytes::new().append(contract_id_to_bytes(target)).append(function_selector);
let mut payload = Bytes::new();
payload.append(contract_id_to_bytes(target));
payload.append(function_selector);
payload.append(function_selector.clone());

if (single_value_type_arg) {
payload.append(calldata); // When calldata is copy type, just pass calldata
payload.append(calldata.clone()); // When calldata is copy type, just pass calldata
} else {
payload.append(ptr_as_bytes(calldata.ptr())); // When calldata is reference type, need to get pointer as bytes
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ convert.sw
intrinsics.sw
constants.sw
bytes.sw
clone.sw
bytes_conversions.sw
bytes_conversions/b256.sw
bytes_conversions/u16.sw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub mod bytes;
pub mod primitive_conversions;
pub mod array_conversions;
pub mod bytes_conversions;
pub mod clone;

pub mod prelude;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"typeArguments": null
},
"name": "C0",
"offset": 4476
"offset": 4724
},
{
"configurableType": {
Expand All @@ -16,7 +16,7 @@
"typeArguments": null
},
"name": "C1",
"offset": 4492
"offset": 4740
},
{
"configurableType": {
Expand All @@ -25,7 +25,7 @@
"typeArguments": null
},
"name": "C2",
"offset": 4508
"offset": 4756
},
{
"configurableType": {
Expand All @@ -34,7 +34,7 @@
"typeArguments": []
},
"name": "C3",
"offset": 4540
"offset": 4788
},
{
"configurableType": {
Expand All @@ -43,7 +43,7 @@
"typeArguments": []
},
"name": "C4",
"offset": 4556
"offset": 4804
},
{
"configurableType": {
Expand All @@ -52,7 +52,7 @@
"typeArguments": []
},
"name": "C5",
"offset": 4572
"offset": 4820
},
{
"configurableType": {
Expand All @@ -61,7 +61,7 @@
"typeArguments": null
},
"name": "C6",
"offset": 4588
"offset": 4836
},
{
"configurableType": {
Expand All @@ -70,7 +70,7 @@
"typeArguments": null
},
"name": "C7",
"offset": 4604
"offset": 4852
},
{
"configurableType": {
Expand All @@ -79,7 +79,7 @@
"typeArguments": null
},
"name": "C7_2",
"offset": 4708
"offset": 4956
},
{
"configurableType": {
Expand All @@ -88,7 +88,7 @@
"typeArguments": null
},
"name": "C9",
"offset": 4652
"offset": 4900
}
],
"functions": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ fn empty_struct_parameter_not_inlined(p: EmptyStruct) {

#[inline(always)]
fn struct_parameter(p: S) {

let r_p_1_addr_of = __addr_of(p);
assert(r_p_1_addr_of == __addr_of(p));

let r_p_1 = &p;
let r_p_2 = &p;

Expand All @@ -123,6 +127,9 @@ fn struct_parameter(p: S) {
assert(*r_p_1 == *r_p_2);

assert(r_p_1.x == r_p_2.x);

let q = S::new();
assert(r_p_1_addr_of != __addr_of(q));
}

#[inline(never)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::hash::*;
#[cfg(experimental_new_encoding = false)]
const CONTRACT_ID = 0x7fae96947a8cad59cc2a25239f9f80897955d4c1b10d31510681f15842b93265;
#[cfg(experimental_new_encoding = true)]
const CONTRACT_ID = 0x54996ea6be619740d315438a413cf060d858fd548ffb75a54408fd8af1519ff8;
const CONTRACT_ID = 0x0e38f647c805a9ea913ffc8ae37cf3b5a76aa86bb4f42d2057c06c5f45a7edd6;

fn main() -> u64 {
let addr = abi(TestContract, CONTRACT_ID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ use test_fuel_coin_abi::*;
#[cfg(experimental_new_encoding = false)]
const FUEL_COIN_CONTRACT_ID = 0x27447d931b1c2c0eaf94aa9ffd1c1ea09298ee23a632937accdac91947a502a0;
#[cfg(experimental_new_encoding = true)]
const FUEL_COIN_CONTRACT_ID = 0x32dcf3d874415397505a12b60c85c63f4f70eb36a33f984ee81fd8214d4faeeb;
const FUEL_COIN_CONTRACT_ID = 0xce28916d1aafcab28b0495481b514632c7f908e4915e6152c77290b78bc99353;

#[cfg(experimental_new_encoding = false)]
const BALANCE_CONTRACT_ID = 0x3b8cb681056f61a41e138b8884d7e3bb9332fbd7a8e38e3e0b0ada766cabfa4e;
#[cfg(experimental_new_encoding = true)]
const BALANCE_CONTRACT_ID = 0x2d15bdaa30e59eb25bab934e9533d10ace0a971ae942e47119e49ef411978d34;
const BALANCE_CONTRACT_ID = 0xdf3aecbed3bde3772553ed1ea84411a114aff5c31b90b0d7f13c6e4e74cb804a;

fn main() -> bool {
let default_gas = 1_000_000_000_000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use balance_test_abi::BalanceTest;
#[cfg(experimental_new_encoding = false)]
const CONTRACT_ID = 0x3b8cb681056f61a41e138b8884d7e3bb9332fbd7a8e38e3e0b0ada766cabfa4e;
#[cfg(experimental_new_encoding = true)]
const CONTRACT_ID = 0x2d15bdaa30e59eb25bab934e9533d10ace0a971ae942e47119e49ef411978d34;
const CONTRACT_ID = 0xdf3aecbed3bde3772553ed1ea84411a114aff5c31b90b0d7f13c6e4e74cb804a;

fn main() -> bool {
let balance_test_contract = abi(BalanceTest, CONTRACT_ID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use abi_with_tuples::*;
#[cfg(experimental_new_encoding = false)]
const CONTRACT_ID = 0xb351aff8258ce46d16a71be666dd2b0b09d72243105c51f4423765824e59cac9;
#[cfg(experimental_new_encoding = true)]
const CONTRACT_ID = 0xde6ab165b5b0f9058daf26002d22f472e6d1af1c35e0210821e743d297d55b17;
const CONTRACT_ID = 0xf96a023e849fb8e84db3e4fc22fea0041080223e7b8c37a97f3fa1682a151f4b;

fn main() -> bool {
let the_abi = abi(MyContract, CONTRACT_ID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ script;
use basic_storage_abi::{BasicStorage, Quad};

#[cfg(experimental_new_encoding = false)]
const CONTRACT_ID = 0xa75e1629f14cf3fa28c6fc442d2a2470cbeb966ee53574935ee4fe28bd24dcb1;
const CONTRACT_ID = 0xbb236cbc9f99b66bb8b8daea5702fd58b740163b0629a042b73f3a8063329ffa;
#[cfg(experimental_new_encoding = true)]
const CONTRACT_ID = 0xbc349573913779db616406eaf74d0ac24da58ce2f26cdedd8a7c3134f34d2739;
const CONTRACT_ID = 0x2a999e8bc26b68091f593906e2136508658065264ccd9e6d49a72d1898ea7080;

fn main() -> u64 {
let addr = abi(BasicStorage, CONTRACT_ID);
Expand Down
Loading

0 comments on commit b94d3d7

Please sign in to comment.