Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not force inling functions that have pointer args. #5762

Merged
merged 23 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
af58459
Do not force inling functions that have pointer args.
vaivaswatha Mar 23, 2024
a437240
Update ABI and contract IDs for tests
vaivaswatha Mar 24, 2024
8cf6272
memcpyopt: bugfix
vaivaswatha Mar 28, 2024
a74c1b3
Merge branch 'master' into vaivaswatha/inline_pointer_args
vaivaswatha Mar 28, 2024
a100f64
update ir_generation test
vaivaswatha Mar 28, 2024
e4ae3c2
ptr_to_int demotion: do not create unnecessary copies
vaivaswatha Apr 3, 2024
1a9dacf
Handle generating reference (pointers) using ptr_to_int
vaivaswatha Apr 5, 2024
d99f96b
Merge branch 'master' of github.com:FuelLabs/sway into vaivaswatha/in…
vaivaswatha Apr 5, 2024
00f7785
fix ir_generation tests
vaivaswatha Apr 5, 2024
9432c37
Use ptr_to_int for references (pointers) only for args
vaivaswatha Apr 6, 2024
2bce7ed
cargo fmt
vaivaswatha Apr 6, 2024
bde6bab
Fix demote_ptr_to_int test
vaivaswatha Apr 6, 2024
ef62bab
stdlib: Fix bug in low_level_call API
vaivaswatha Apr 11, 2024
6f01e00
Merge branch 'master' of github.com:FuelLabs/sway into vaivaswatha/in…
vaivaswatha Apr 11, 2024
c9998bf
Update reduced_std_libs with clone trait
vaivaswatha Apr 11, 2024
d5d3e69
Fix how clone.sw is used in reduced_lib.config in testing
vaivaswatha Apr 11, 2024
5d7017f
update some tests
vaivaswatha Apr 12, 2024
93d32ba
Merge branch 'master' into vaivaswatha/inline_pointer_args
vaivaswatha Apr 12, 2024
12a2aac
address review comments
vaivaswatha Apr 15, 2024
0b0bad7
Merge branch 'master' of github.com:FuelLabs/sway into vaivaswatha/in…
vaivaswatha Apr 15, 2024
24e0dd8
Check for only function arg
vaivaswatha Apr 15, 2024
6d8703f
update contract id in test
vaivaswatha Apr 15, 2024
ed30b41
Merge branch 'master' into vaivaswatha/inline_pointer_args
vaivaswatha Apr 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
45 changes: 34 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,48 @@ 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 = if let ty::TyExpressionVariant::VariableExpression { name, .. } =
vaivaswatha marked this conversation as resolved.
Show resolved Hide resolved
&ast_expr.expression
{
self.function.get_arg(context, name.as_str()).is_some()
} else {
false
};

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 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 = 0x089ad9c09f74d319c4dd17ce8ac1ee2d41d7bb348831eb0b7438f09f82da4204;
const CONTRACT_ID = 0xb02f1aba538e8bd39bae7235caa20b897ba1c42ef1a18ff310e01e927aa87940;

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