Skip to content

Commit

Permalink
const-demotion locals must be loaded in same basic block as its use. (F…
Browse files Browse the repository at this point in the history
…uelLabs#4534)

## Description

Previously, const-demotion would load all demoted consts from a new
block at entry. The problem with this is that optimizing sequences of
load+store (memcpy) via copy-propagation would then require a function
wide data-flow analysis. In the same block, it's easier to do, and we
already have the optimization.

Related to FuelLabs#4345 

## 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 4, 2023
1 parent 1a881ee commit 990d02f
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 58 deletions.
7 changes: 7 additions & 0 deletions sway-ir/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,13 @@ impl Block {
ins.retain(|value| !pred(*value));
}

/// Insert instruction(s) at the beginning of the block.
pub fn prepend_instructions(&self, context: &mut Context, mut insts: Vec<Value>) {
let block_ins = &mut context.blocks[self.0].instructions;
insts.append(block_ins);
context.blocks[self.0].instructions = insts;
}

/// Replace an instruction in this block with another. Will return a ValueNotFound on error.
/// Any use of the old instruction value will also be replaced by the new value throughout the
/// owning function.
Expand Down
94 changes: 46 additions & 48 deletions sway-ir/src/optimize/const_demotion.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::hash_map::Entry;

///! Constant value demotion.
///!
///! This pass demotes 'by-value' constant types to 'by-reference` pointer types, based on target
Expand All @@ -6,8 +8,8 @@
///! Storage for constant values is created on the stack in variables which are initialized with the
///! original values.
use crate::{
AnalysisResults, Block, Constant, Context, Function, IrError, Pass, PassMutability, ScopedPass,
Value,
AnalysisResults, Block, Constant, Context, Function, Instruction, IrError, Pass,
PassMutability, ScopedPass, Value,
};

use rustc_hash::FxHashMap;
Expand All @@ -29,57 +31,53 @@ pub fn const_demotion(
function: Function,
) -> Result<bool, IrError> {
// Find all candidate constant values and their wrapped constants.
let candidate_values = function
.instruction_iter(context)
.flat_map(|(_block, inst)| inst.get_instruction(context).unwrap().get_operands())
.filter_map(|val| {
val.get_constant(context).and_then(|c| {
super::target_fuel::is_demotable_type(context, &c.ty).then(|| (val, c.clone()))
})
})
.collect::<Vec<_>>();
let mut candidate_values: FxHashMap<Block, Vec<(Value, Constant)>> = FxHashMap::default();

for (block, inst) in function.instruction_iter(context) {
let operands = inst.get_instruction(context).unwrap().get_operands();
for val in operands.iter() {
if let Some(c) = val.get_constant(context) {
if super::target_fuel::is_demotable_type(context, &c.ty) {
let dem = (*val, c.clone());
match candidate_values.entry(block) {
Entry::Occupied(mut occ) => {
occ.get_mut().push(dem);
}
Entry::Vacant(vac) => {
vac.insert(vec![dem]);
}
}
}
}
}
}

if candidate_values.is_empty() {
return Ok(false);
}

// Create a new entry block to initialise and load the constants.
let (const_init_block, orig_entry_block) =
function.get_entry_block(context).split_at(context, 0);

// Insert const initialisation into new init block, gather into a replacement map.
let replace_map =
FxHashMap::from_iter(candidate_values.into_iter().map(|(old_value, constant)| {
(
old_value,
demote(context, &function, &const_init_block, &constant),
)
}));

// Terminate the init block.
const_init_block
.ins(context)
.branch(orig_entry_block, Vec::new());

// Replace the value.
function.replace_values(context, &replace_map, Some(orig_entry_block));

assert_eq!(const_init_block, function.get_entry_block(context));
for (block, cands) in candidate_values {
let mut replace_map: FxHashMap<Value, Value> = FxHashMap::default();
// The new instructions we're going to insert at the start of this block.
let mut this_block_new = Vec::new();
for (c_val, c) in cands {
// Create a variable for const.
let var = function.new_unique_local_var(
context,
"__const".to_owned(),
c.ty,
Some(c.clone()),
false,
);
let var_val = Value::new_instruction(context, Instruction::GetLocal(var));
let load_val = Value::new_instruction(context, Instruction::Load(var_val));
replace_map.insert(c_val, load_val);
this_block_new.push(var_val);
this_block_new.push(load_val);
}
block.replace_values(context, &replace_map);
block.prepend_instructions(context, this_block_new);
}

Ok(true)
}

fn demote(context: &mut Context, function: &Function, block: &Block, constant: &Constant) -> Value {
// Create a variable for const.
let var = function.new_unique_local_var(
context,
"__const".to_owned(),
constant.ty,
Some(constant.clone()),
false,
);

// Create local_var and load instructions.
let var_val = block.ins(context).get_local(var);
block.ins(context).load(var_val)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"typeArguments": null
},
"name": "C0",
"offset": 3764
"offset": 3740
},
{
"configurableType": {
Expand All @@ -16,7 +16,7 @@
"typeArguments": null
},
"name": "C1",
"offset": 3772
"offset": 3748
},
{
"configurableType": {
Expand All @@ -25,7 +25,7 @@
"typeArguments": null
},
"name": "C2",
"offset": 3788
"offset": 3764
},
{
"configurableType": {
Expand All @@ -34,7 +34,7 @@
"typeArguments": []
},
"name": "C3",
"offset": 3820
"offset": 3796
},
{
"configurableType": {
Expand All @@ -43,7 +43,7 @@
"typeArguments": []
},
"name": "C4",
"offset": 3836
"offset": 3812
},
{
"configurableType": {
Expand All @@ -52,7 +52,7 @@
"typeArguments": []
},
"name": "C5",
"offset": 3852
"offset": 3828
},
{
"configurableType": {
Expand All @@ -61,7 +61,7 @@
"typeArguments": null
},
"name": "C6",
"offset": 3868
"offset": 3844
},
{
"configurableType": {
Expand All @@ -70,7 +70,7 @@
"typeArguments": null
},
"name": "C7",
"offset": 3884
"offset": 3860
}
],
"functions": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ script;
use basic_storage_abi::{BasicStorage, Quad};

fn main() -> u64 {
let addr = abi(BasicStorage, 0x47ddf698943d2327a24e9ef44c8d21f752d5d9eedd8d5f29c1835c3ba0092f28);
let addr = abi(BasicStorage, 0xb58bd85320e71f5d4f079ee969c7eeda8bf6284d0604b8ff70806355ab90aef4);
let key = 0x0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
let value = 4242;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ script;
use increment_abi::Incrementor;

fn main() -> bool {
let the_abi = abi(Incrementor, 0xa99b5dd42245f9d214f8066a2bb92439cc3e2282625d1980b50552b2f64de2b1);
let the_abi = abi(Incrementor, 0xabc8916cc1d37a762308deab3fc46e2fbe2ce3b9c918002c28368e0e026aa418);
the_abi.increment(5);
the_abi.increment(5);
let result = the_abi.get();
Expand Down

0 comments on commit 990d02f

Please sign in to comment.