Skip to content

Commit

Permalink
[move-2024] Enable Move 2024 optimizations (MystenLabs#16120)
Browse files Browse the repository at this point in the history
## Description 

The Move 2024 optimizations got disabled from alpha at some point. This
re-enables them.

## Test Plan 

The test suite still works, plus more tests to ensure the new freeze
mechanisms work.

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
cgswords authored Feb 8, 2024
1 parent c58e6ae commit 7e2f857
Show file tree
Hide file tree
Showing 12 changed files with 239 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ fn find_forwarding_jump_destinations(blocks: &BasicBlocks) -> LabelMap {
let mut final_jumps: LabelMap = BTreeMap::new();

for start in forwarding_jumps.keys() {
if final_jumps.contains_key(start) { break };
if final_jumps.contains_key(start) {
break;
};
let mut target = *start;
let mut seen = BTreeSet::new();
while let Some(next_target) = forwarding_jumps.get(&target) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@

mod constant_fold;
mod eliminate_locals;
// mod forwarding_jumps;
mod forwarding_jumps;
mod inline_blocks;
mod simplify_jumps;

use move_symbol_pool::Symbol;

use crate::{
cfgir::cfg::MutForwardCFG, hlir::ast::*, parser::ast::ConstantName,
shared::unique_map::UniqueMap,
cfgir::cfg::MutForwardCFG,
editions::FeatureGate,
hlir::ast::*,
parser::ast::ConstantName,
shared::{unique_map::UniqueMap, CompilationEnv},
};

pub type Optimization = fn(
Expand All @@ -22,23 +27,38 @@ pub type Optimization = fn(
const OPTIMIZATIONS: &[Optimization] = &[
eliminate_locals::optimize,
constant_fold::optimize,
// forwarding_jumps::optimize,
simplify_jumps::optimize,
inline_blocks::optimize,
];

const MOVE_2024_OPTIMIZATIONS: &[Optimization] = &[
eliminate_locals::optimize,
constant_fold::optimize,
forwarding_jumps::optimize,
simplify_jumps::optimize,
inline_blocks::optimize,
];

pub fn optimize(
env: &mut CompilationEnv,
package: Option<Symbol>,
signature: &FunctionSignature,
locals: &UniqueMap<Var, SingleType>,
constants: &UniqueMap<ConstantName, Value>,
cfg: &mut MutForwardCFG,
) {
let mut count = 0;
for optimization in OPTIMIZATIONS.iter().cycle() {
let optimizations = if env.supports_feature(package, FeatureGate::Move2024Optimizations) {
MOVE_2024_OPTIMIZATIONS
} else {
OPTIMIZATIONS
};
let opt_count = optimizations.len();
for optimization in optimizations.iter().cycle() {
// if we have fully cycled through the list of optimizations without a change,
// it is safe to stop
if count >= OPTIMIZATIONS.len() {
debug_assert_eq!(count, OPTIMIZATIONS.len());
if count >= opt_count {
debug_assert_eq!(count, opt_count);
break;
}

Expand Down
24 changes: 21 additions & 3 deletions external-crates/move/crates/move-compiler/src/cfgir/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::{
use cfgir::ast::LoopInfo;
use move_core_types::{account_address::AccountAddress as MoveAddress, runtime_value::MoveValue};
use move_ir_types::location::*;
use move_symbol_pool::Symbol;
use petgraph::{
algo::{kosaraju_scc as petgraph_scc, toposort as petgraph_toposort},
graphmap::DiGraphMap,
Expand All @@ -37,6 +38,7 @@ enum NamedBlockType {

struct Context<'env> {
env: &'env mut CompilationEnv,
current_package: Option<Symbol>,
struct_declared_abilities: UniqueMap<ModuleIdent, UniqueMap<StructName, AbilitySet>>,
label_count: usize,
named_blocks: UniqueMap<BlockLabel, (Label, Label)>,
Expand Down Expand Up @@ -66,6 +68,7 @@ impl<'env> Context<'env> {
.unwrap();
Context {
env,
current_package: None,
struct_declared_abilities,
label_count: 0,
named_blocks: UniqueMap::new(),
Expand Down Expand Up @@ -172,11 +175,12 @@ fn module(
functions: hfunctions,
constants: hconstants,
} = mdef;

context.current_package = package_name;
context.env.add_warning_filter_scope(warning_filter.clone());
let constants = constants(context, module_ident, hconstants);
let functions = hfunctions.map(|name, f| function(context, module_ident, name, f));
context.env.pop_warning_filter_scope();
context.current_package = None;
(
module_ident,
G::ModuleDefinition {
Expand Down Expand Up @@ -474,7 +478,14 @@ fn constant_(
"{}",
ICE_MSG
);
cfgir::optimize(&fake_signature, &locals, constant_values, &mut cfg);
cfgir::optimize(
context.env,
context.current_package,
&fake_signature,
&locals,
constant_values,
&mut cfg,
);

if blocks.len() != 1 {
context.env.add_diag(diag!(
Expand Down Expand Up @@ -620,7 +631,14 @@ fn function_body(
cfgir::refine_inference_and_verify(context.env, &function_context, &mut cfg);
// do not optimize if there are errors, warnings are okay
if !context.env.has_errors() {
cfgir::optimize(signature, &locals, &UniqueMap::new(), &mut cfg);
cfgir::optimize(
context.env,
context.current_package,
signature,
&locals,
&UniqueMap::new(),
&mut cfg,
);
}

let block_info = block_info
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const E2024_ALPHA_FEATURES: &[FeatureGate] = &[
FeatureGate::BlockLabels,
FeatureGate::Move2024Paths,
FeatureGate::MacroFuns,
FeatureGate::Move2024Optimizations,
];

const E2024_MIGRATION_FEATURES: &[FeatureGate] = &[FeatureGate::Move2024Migration];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module 0x8675309::M {
public struct S has copy, drop { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0(cond: bool, s: &mut S, other: &S) {
let mut f;
if (cond) f = &s.f else f = &other.f;
freeze(s);
*f;
}

fun t1(cond: bool, s: &mut S) {
let mut f;
if (cond) f = &s.f else f = &s.g;
freeze(s);
*f;
}

fun t2(cond: bool, s: &mut S, other: &S) {
let mut x;
if (cond) x = freeze(s) else x = other;
freeze(s);
*x;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E07002]: mutable ownership violated
┌─ tests/move_2024/borrows/freeze_combo_invalid.move:13:9
12 │ if (cond) f = &mut s.f else f = &mut other.f;
│ -------- Field 'f' is still being mutably borrowed by this reference
13 │ freeze(s);
│ ^^^^^^^^^ Invalid freeze.

error[E07002]: mutable ownership violated
┌─ tests/move_2024/borrows/freeze_combo_invalid.move:20:9
19 │ if (cond) f = &mut s.f else f = &mut s.g;
│ -------- -------- Field 'g' is still being mutably borrowed by this reference
│ │
│ Field 'f' is still being mutably borrowed by this reference
20 │ freeze(s);
│ ^^^^^^^^^ Invalid freeze.

error[E07002]: mutable ownership violated
┌─ tests/move_2024/borrows/freeze_combo_invalid.move:27:9
26 │ if (cond) x = s else x = other;
│ - It is still being mutably borrowed by this reference
27 │ freeze(s);
│ ^^^^^^^^^ Invalid freeze.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module 0x8675309::M {
public struct S has copy, drop { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0(cond: bool, s: &mut S, other: &mut S) {
let mut f;
if (cond) f = &mut s.f else f = &mut other.f;
freeze(s);
*f;
}

fun t1(cond: bool, s: &mut S) {
let mut f;
if (cond) f = &mut s.f else f = &mut s.g;
freeze(s);
*f;
}

fun t2(cond: bool, s: &mut S, other: &mut S) {
let mut x;
if (cond) x = s else x = other;
freeze(s);
*x;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module 0x8675309::M {
public struct S { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0(s: &mut S) {
let f = &mut s.f;
*f;
freeze(s);
}

fun t1(s: &mut S) {
let f = &s.f;
freeze(s);
*f;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E07002]: mutable ownership violated
┌─ tests/move_2024/borrows/freeze_field_invalid.move:12:9
11 │ let f = &mut s.f;
│ -------- Field 'f' is still being mutably borrowed by this reference
12 │ freeze(s);
│ ^^^^^^^^^ Invalid freeze.

error[E07002]: mutable ownership violated
┌─ tests/move_2024/borrows/freeze_field_invalid.move:19:9
18 │ let g = &mut s.f;
│ -------- Field 'f' is still being mutably borrowed by this reference
19 │ freeze(s);
│ ^^^^^^^^^ Invalid freeze.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module 0x8675309::M {
public struct S { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0(s: &mut S) {
let f = &mut s.f;
freeze(s);
*f;
}

fun t1(s: &mut S) {
let f = &s.f;
let g = &mut s.f;
freeze(s);
*f;
*g;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module 0x8675309::M {
public struct S { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0() {
let x = &mut 0;
freeze(x);

let x = id_mut(&mut 0);
freeze(x);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module 0x8675309::M {
public struct S { f: u64, g: u64 }
fun id<T>(r: &T): &T {
r
}
fun id_mut<T>(r: &mut T): &mut T {
r
}

fun t0() {
let x = &mut 0;
freeze(x);
*x = 0;

let x = id_mut(&mut 0);
freeze(x);
*x = 0;
}

}

0 comments on commit 7e2f857

Please sign in to comment.