Skip to content

Commit

Permalink
[compiler v2] Fix bugs around compilation of vector code
Browse files Browse the repository at this point in the history
This fixes a few bugs and closes #9629

- Usage of precompiled stdlib in transactional tests: v2 compiler currently (and probably never) supports precompiled modules, working around this
- Stack-based bytecode generator created wrong target on generic function calls
- Needed to make implicit conversion from `&mut` to `&` explicit in generated bytecode via Freeze operation

Added some additional tests while debugging this.
  • Loading branch information
wrwg committed Aug 13, 2023
1 parent c9a8721 commit e0b3636
Show file tree
Hide file tree
Showing 12 changed files with 290 additions and 4 deletions.
42 changes: 40 additions & 2 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@ impl<'env> Generator<'env> {
// Dispatcher

impl<'env> Generator<'env> {
/// Generate code, for the given expression, and store the result in the given temporary.
fn gen(&mut self, targets: Vec<TempIndex>, exp: &Exp) {
match exp.as_ref() {
ExpData::Invalid(id) => self.internal_error(*id, "invalid expression"),
Expand Down Expand Up @@ -720,7 +719,25 @@ impl<'env> Generator<'env> {
.env()
.get_node_instantiation_opt(id)
.unwrap_or_default();
let args = self.gen_arg_list(args);
// Function calls can have implicit conversion of &mut to &, need to compute implicit
// conversions.
let param_types: Vec<Type> = self
.env()
.get_function(fun)
.get_parameters()
.into_iter()
.map(|Parameter(_, ty)| ty.instantiate(&type_args))
.collect();
if args.len() != param_types.len() {
self.internal_error(id, "inconsistent type arity");
return;
}
let args = args
.iter()
.zip(param_types.into_iter())
.map(|(e, t)| self.maybe_convert(e, &t))
.collect::<Vec<_>>();
let args = self.gen_arg_list(&args);
self.emit_with(id, |attr| {
Bytecode::Call(
attr,
Expand All @@ -732,6 +749,27 @@ impl<'env> Generator<'env> {
})
}

/// Convert the expression so it matches the expected type. This is currently only needed
/// for `&mut` to `&` conversion, in which case we need to to introduce a Freeze operation.
fn maybe_convert(&self, exp: &Exp, expected_ty: &Type) -> Exp {
let id = exp.node_id();
let exp_ty = self.env().get_node_type(id);
if let (
Type::Reference(ReferenceKind::Mutable, _),
Type::Reference(ReferenceKind::Immutable, et),
) = (exp_ty, expected_ty)
{
let freeze_id = self
.env()
.new_node(self.env().get_node_loc(id), expected_ty.clone());
self.env()
.set_node_instantiation(freeze_id, vec![et.as_ref().clone()]);
ExpData::Call(freeze_id, Operation::Freeze, vec![exp.clone()]).into_exp()
} else {
exp.clone()
}
}

fn gen_arg_list(&mut self, exps: &[Exp]) -> Vec<TempIndex> {
exps.iter().map(|exp| self.gen_arg(exp)).collect()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ impl<'a> FunctionGenerator<'a> {
let idx = self.gen.function_instantiation_index(
&fun_ctx.module,
&fun_ctx.loc,
fun_ctx.fun.func_env,
&fun_ctx.module.env.get_function(id),
inst.to_vec(),
);
self.emit(FF::Bytecode::CallGeneric(idx))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ impl FunctionTargetProcessor for LiveVarAnalysisProcessor {
mut data: FunctionData,
_scc_opt: Option<&[FunctionEnv]>,
) -> FunctionData {
if fun_env.is_native() {
return data;
}
// Call the existing live-var analysis from the move-prover.
let target = FunctionTarget::new(fun_env, &data);
let offset_to_live_refs = livevar_analysis::LiveVarAnnotation::from_map(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// ---- Model Dump
module 0x42::reference_conversion {
private fun deref(r: &u64) {
Deref(r)
}
private fun use_it() {
{
let x: u64 = 42;
{
let r: &mut u64 = Borrow(Mutable)(x);
r = 43;
reference_conversion::deref(r)
}
}
}
} // end 0x42::reference_conversion

============ initial bytecode ================

[variant baseline]
fun reference_conversion::deref($t0: &u64): u64 {
var $t1: u64
0: $t1 := read_ref($t0)
1: return $t1
}


[variant baseline]
fun reference_conversion::use_it(): u64 {
var $t0: u64
var $t1: u64
var $t2: u64
var $t3: &mut u64
var $t4: &mut u64
var $t5: u64
var $t6: &u64
0: $t2 := 42
1: $t1 := move($t2)
2: $t4 := borrow_local($t1)
3: $t3 := move($t4)
4: $t5 := 43
5: write_ref($t3, $t5)
6: $t6 := freeze_ref($t3)
7: $t0 := reference_conversion::deref($t6)
8: return $t0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module 0x42::reference_conversion {

fun deref(r: &u64): u64 {
*r
}

fun use_it(): u64 {
let x = 42;
let r = &mut x;
*r = 43;
deref(r)
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
============ initial bytecode ================

[variant baseline]
fun Test::foo($t0: u64): u64 {
var $t1: u64
0: $t1 := Test::identity<u64>($t0)
1: return $t1
}


[variant baseline]
fun Test::identity<#0>($t0: #0): #0 {
var $t1: #0
0: $t1 := move($t0)
1: return $t1
}

============ after LiveVarAnalysisProcessor: ================

[variant baseline]
fun Test::foo($t0: u64): u64 {
var $t1: u64
# live vars: $t0
0: $t1 := Test::identity<u64>($t0)
# live vars: $t1
1: return $t1
}


[variant baseline]
fun Test::identity<#0>($t0: #0): #0 {
var $t1: #0
# live vars: $t0
0: $t1 := move($t0)
# live vars: $t1
1: return $t1
}


============ disassembled file-format ==================
// Move bytecode v6
module 42.Test {


foo(Arg0: u64): u64 {
B0:
0: MoveLoc[0](Arg0: u64)
1: Call identity<u64>(u64): u64
2: Ret
}
identity<Ty0>(Arg0: Ty0): Ty0 {
B0:
0: MoveLoc[0](Arg0: Ty0)
1: StLoc[1](loc0: Ty0)
2: MoveLoc[1](loc0: Ty0)
3: Ret
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module 0x42::Test {
fun identity<T>(x: T): T {
x
}

fun foo(x: u64): u64 {
identity(x)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
processed 2 tasks

==> Compiler v2 delivered same results!
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//# publish
module 0x42::heap {
use std::vector;

fun create1(): vector<u64> {
vector<u64>[3, 2, 1, 5, 8, 4]
}

fun create2(): vector<u64> {
vector<u64>[1, 2, 3, 4, 5, 8]
}

fun vcopy(x: &vector<u64>): vector<u64> {
let y : vector<u64> = vector::empty<u64>();
let i : u64 = 0;
let l : u64 = vector::length<u64>(x);
while (i < l) {
vector::push_back<u64>(&mut y, *vector::borrow<u64>(x, i));
i = i + 1;
};
y
}

fun sort(x: &mut vector<u64>) {
let i: u64 = 0;
while (i < vector::length<u64>(x)) {
let j: u64 = i + 1;
while (j < vector::length<u64>(x)) {
if (*vector::borrow<u64>(x, i) > *vector::borrow<u64>(x, j)) {
vector::swap<u64>(x, i, j)
};
j = j + 1;
};
i = i + 1;
}
}

fun array_equals(x: &vector<u64>, y: &vector<u64>): bool {
let l1: u64 = vector::length<u64>(x);
let l2: u64 = vector::length<u64>(y);
if (l1 != l2) {
return false
};
let i: u64 = 0;
while (i < l1) {
if (*vector::borrow<u64>(x, i) != *vector::borrow<u64>(y, i)) {
return false
};
i = i + 1;
};
true
}

public fun main() {
let x: vector<u64> = create1();
let y: vector<u64> = create2();
let z: vector<u64> = vcopy(&x);
assert!(array_equals(&x, &z), 23);
assert!(array_equals(&y, &y), 29);
sort(&mut x);
assert!(array_equals(&y, &x), 31);
assert!(array_equals(&x, &y), 29);
assert!(!array_equals(&x, &z), 31);
}
}

//# run
script {
use 0x42::heap::main;
fun mymain() {
main();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
processed 2 tasks

==> Compiler v2 delivered same results!
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//# publish
module 0x42::test {
public fun two_args(x: u64, b: bool): u64 {
if (b) {
x
} else {
0
}
}
}

//# run
script {
use 0x42::test::two_args;
fun mymain() {
assert!(two_args(42, true) == 42, 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ pub trait MoveTestAdapter<'a>: Sized {
// Run the V2 compiler if requested
SyntaxChoice::Source if run_config == TestRunConfig::CompilerV2 => {
let ((module, _), warning_opt) = compile_source_unit_v2(
state.pre_compiled_deps,
state.named_address_mapping.clone(),
&state.source_files().cloned().collect::<Vec<_>>(),
data_path.to_owned(),
Expand Down Expand Up @@ -326,6 +327,7 @@ pub trait MoveTestAdapter<'a>: Sized {
// Run the V2 compiler if requested.
SyntaxChoice::Source if run_config == TestRunConfig::CompilerV2 => {
let ((_, script), warning_opt) = compile_source_unit_v2(
state.pre_compiled_deps,
state.named_address_mapping.clone(),
&state.source_files().cloned().collect::<Vec<_>>(),
data_path.to_owned(),
Expand Down Expand Up @@ -603,16 +605,34 @@ impl<'a> CompiledState<'a> {
}

fn compile_source_unit_v2(
pre_compiled_deps: Option<&FullyCompiledProgram>,
named_address_mapping: BTreeMap<String, NumericalAddress>,
deps: &[String],
path: String,
) -> Result<(
(Option<CompiledModule>, Option<CompiledScript>),
Option<String>,
)> {
let deps = if let Some(p) = pre_compiled_deps {
// The v2 compiler does not (and perhaps never) supports precompiled programs, so
// compile from the sources again, computing the directories where they are found.
let mut dirs: BTreeSet<_> = p
.files
.iter()
.filter_map(|(_, (file_name, _))| {
Path::new(file_name.as_str())
.parent()
.map(|p| p.to_string_lossy().to_string())
})
.collect();
dirs.extend(deps.iter().cloned());
dirs.into_iter().collect()
} else {
deps.to_vec()
};
let options = move_compiler_v2::Options {
sources: vec![path],
dependencies: deps.to_vec(),
dependencies: deps,
named_address_mapping: named_address_mapping
.into_iter()
.map(|(alias, addr)| format!("{}={}", alias, addr))
Expand Down

0 comments on commit e0b3636

Please sign in to comment.