Skip to content

Commit

Permalink
Inlining heuristic: return value demotion adds arg (FuelLabs#4518)
Browse files Browse the repository at this point in the history
## Description

We currently don't support compiling functions with more than 6 args.
Such functions are inlined. However, the inliner heuristic that checks
this fails to say "inline" if there are 6 args, but the return type will
be demoted later on, adding an additional arg. Fix that.

Fixes FuelLabs#4477.

## 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.
  • Loading branch information
vaivaswatha authored May 2, 2023
1 parent b3cfea4 commit f4422d4
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 5 deletions.
14 changes: 13 additions & 1 deletion sway-ir/src/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,21 @@ pub fn inline_in_non_predicate_module(
None => {}
}

let ret_type = func.get_return_type(ctx);
let num_args = {
func.args_iter(ctx).count()
+ if super::target_fuel::is_demotable_type(ctx, &ret_type) {
// The return type will be demoted to memory,
// which means that there'll be an additional return arg.
1
} else {
0
}
};

// For now, pending improvements to ASMgen for calls, we must inline any function which has
// too many args.
if func.args_iter(ctx).count() as u8 > NUM_ARG_REGISTERS {
if num_args as u8 > NUM_ARG_REGISTERS {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[[package]]
name = 'arg_demotion_inline'
source = 'member'
dependencies = [
'core',
'std',
]

[[package]]
name = 'core'
source = 'path+from-root-603115901A793008'

[[package]]
name = 'std'
source = 'path+from-root-603115901A793008'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "arg_demotion_inline"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
contract;

abi MyContract {
fn test_function() -> bool;
}

struct Foo {
a: u64,
b: u64,
c: u64,
d: u64,
e: u64,
f: u64,
}

impl Foo {
fn new(
a: u64,
b: u64,
c: u64,
d: u64,
e: u64,
f: u64,
) -> Self {
Self {
a,
b,
c,
d,
e,
f,
}
}
}

impl MyContract for Contract {
fn test_function() -> bool {
let bar1 = Foo::new(0,0,0,0,0,0);
let bar2 = Foo::new(0,0,0,0,0,0);
true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
category = "compile"
expected_warnings = 8
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
[[package]]
name = 'core'
source = 'path+from-root-6BF9CEE717879469'
source = 'path+from-root-E733504800A298D4'

[[package]]
name = 'script_multi_test'
name = 'stack_indexing_overflow'
source = 'member'
dependencies = ['std']

[[package]]
name = 'std'
source = 'path+from-root-6BF9CEE717879469'
source = 'path+from-root-E733504800A298D4'
dependencies = ['core']
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "script_multi_test"
name = "stack_indexing_overflow"

[dependencies]
std = { path = "../../../../../../../sway-lib-std" }

0 comments on commit f4422d4

Please sign in to comment.