Skip to content

Commit

Permalink
[move-compiler v2] Comparison testing for file-format generator (#9345)
Browse files Browse the repository at this point in the history
* [move-compiler v2] File-format generator first version

This adds an initial version of the generation of the external representation of Move code, the so-called file format.

- Compiles from the register machine of the compiler IR into the stack machine the VM uses. Some attempts are made to optimize usage of the stack, though more can/must be done to optimize the stack machine code.
- Adds a set of basic tests, but more e2e execution tests should be added. (Subsequent PRs.)
- The translation can benefit from analysis results like live-var which allows it make smarter use of `CopyLoc` vs `MoveLoc` (to be added in the future)

* Addressing reviewer comments

* More review fixes

* [move-compiler v2] Comparison testing for file-format generator

This extends the transactional-test-runner to support compiler V2 _and_ to do comparison testing between V1 and V2.

It ports over most of the transactional tests of the V1 compiler into the V2 tree and runs comparison testing on them, fixing bugs throughout the compilation chain as needed.

Closes #9334
  • Loading branch information
wrwg authored Jul 31, 2023
1 parent 16eba53 commit 53913b4
Show file tree
Hide file tree
Showing 183 changed files with 7,595 additions and 388 deletions.
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ members = [
"third_party/move/move-command-line-common",
"third_party/move/move-compiler",
"third_party/move/move-compiler-v2",
"third_party/move/move-compiler-v2/transactional-tests",
"third_party/move/move-compiler/transactional-tests",
"third_party/move/move-core/types",
"third_party/move/move-ir-compiler",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use move_resource_viewer::{AnnotatedMoveValue, MoveValueAnnotator};
use move_transactional_test_runner::{
framework::{run_test_impl, CompiledState, MoveTestAdapter},
tasks::{InitCommand, SyntaxChoice, TaskInput},
vm_test_harness::view_resource_in_move_storage,
vm_test_harness::{view_resource_in_move_storage, TestRunConfig},
};
use move_vm_runtime::session::SerializedReturnValues;
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -553,6 +553,8 @@ impl<'a> MoveTestAdapter<'a> for AptosTestAdapter<'a> {

fn init(
default_syntax: SyntaxChoice,
_comparison_mode: bool,
_run_config: TestRunConfig,
pre_compiled_deps: Option<&'a FullyCompiledProgram>,
task_opt: Option<TaskInput<(InitCommand, Self::ExtraInitArgs)>>,
) -> (Self, Option<String>) {
Expand Down Expand Up @@ -959,5 +961,9 @@ fn render_events(events: &[ContractEvent]) -> Option<String> {
pub fn run_aptos_test(path: &Path) -> Result<(), Box<dyn std::error::Error>> {
// TODO: remove once bundles removed
aptos_vm::aptos_vm::allow_module_bundle_for_test();
run_test_impl::<AptosTestAdapter>(path, Some(&*PRECOMPILED_APTOS_FRAMEWORK))
run_test_impl::<AptosTestAdapter>(
TestRunConfig::CompilerV1,
path,
Some(&*PRECOMPILED_APTOS_FRAMEWORK),
)
}
26 changes: 26 additions & 0 deletions third_party/move/move-command-line-common/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,29 @@ pub fn format_diff(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String
}
ret
}

pub fn format_diff_no_color(expected: impl AsRef<str>, actual: impl AsRef<str>) -> String {
use difference::*;

let changeset = Changeset::new(expected.as_ref(), actual.as_ref(), "\n");

let mut ret = String::new();

for seq in changeset.diffs {
match &seq {
Difference::Same(x) => {
ret.push_str(&format!("= {}", x.replace('\n', "\n= ")));
ret.push('\n');
},
Difference::Add(x) => {
ret.push_str(&format!("+ {}", x.replace('\n', "\n+ ")));
ret.push('\n');
},
Difference::Rem(x) => {
ret.push_str(&format!("- {}", x.replace('\n', "\n- ")));
ret.push('\n');
},
}
}
ret
}
6 changes: 5 additions & 1 deletion third_party/move/move-compiler-v2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ publish = false
edition = "2021"

[dependencies]
anyhow = "1.0.62"
move-binary-format = { path = "../move-binary-format" }
move-core-types = { path = "../move-core/types" }
move-model = { path = "../move-model" }
move-stackless-bytecode = { path = "../move-prover/bytecode" }

anyhow = "1.0.62"
bcs = { workspace = true }
clap = { version = "3.2.23", features = ["derive", "env"] }
codespan = "0.11.1"
codespan-reporting = { version = "0.11.1", features = ["serde", "serialization"] }
Expand All @@ -32,6 +33,9 @@ serde = { version = "1.0.124", features = ["derive"] }
[dev-dependencies]
anyhow = "1.0.52"
datatest-stable = "0.1.1"
move-command-line-common = { path = "../move-command-line-common" }
move-disassembler = { path = "../tools/move-disassembler" }
move-ir-types = { path = "../move-ir/types" }
move-prover-test-utils = { path = "../move-prover/test-utils" }
move-stdlib = { path = "../move-stdlib" }

Expand Down
55 changes: 49 additions & 6 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,21 @@ pub fn generate_bytecode(env: &GlobalEnv, fid: QualifiedId<FunId>) -> FunctionDa
loops: vec![],
reference_mode_counter: 0,
reference_mode_kind: ReferenceKind::Immutable,
results: vec![],
code: vec![],
};
let mut scope = BTreeMap::new();
for Parameter(name, ty) in gen.func_env.get_parameters() {
let temp = gen.new_temp(ty);
scope.insert(name, temp);
}
let mut results = vec![];
for ty in gen.func_env.get_result_type().flatten() {
let temp = gen.new_temp(ty);
results.push(temp)
gen.results.push(temp)
}
gen.scopes.push(scope);
if let Some(def) = gen.func_env.get_def().cloned() {
let results = gen.results.clone();
// Need to clone expression if present because of sharing issues with `gen`. However, because
// of interning, clone is cheap.
gen.gen(results.clone(), &def);
Expand All @@ -66,6 +67,7 @@ pub fn generate_bytecode(env: &GlobalEnv, fid: QualifiedId<FunId>) -> FunctionDa
loops: _,
reference_mode_counter: _,
reference_mode_kind: _,
results: _,
code,
} = gen;
FunctionData::new(
Expand Down Expand Up @@ -107,6 +109,8 @@ struct Generator<'env> {
reference_mode_counter: usize,
/// The kind of the reference mode.
reference_mode_kind: ReferenceKind,
/// The list of temporaries where to store function return result.
results: Vec<TempIndex>,
/// The bytecode, as generated so far.
code: Vec<Bytecode>,
// TODO: location_table,
Expand Down Expand Up @@ -327,6 +331,7 @@ impl<'env> Generator<'env> {
self.scopes.pop();
},
ExpData::Mutate(id, lhs, rhs) => {
let rhs_temp = self.gen_arg(rhs);
let lhs_temp = self.gen_auto_ref_arg(lhs, ReferenceKind::Mutable);
if !self.temp_type(lhs_temp).is_mutable_reference() {
self.error(
Expand All @@ -338,13 +343,16 @@ impl<'env> Generator<'env> {
),
);
}
let rhs_temp = self.gen_arg(rhs);
self.emit_call(*id, targets, BytecodeOperation::WriteRef, vec![
lhs_temp, rhs_temp,
])
},
ExpData::Assign(id, lhs, rhs) => self.gen_assign(*id, lhs, rhs, None),
ExpData::Return(_, exp) => self.gen(targets, exp),
ExpData::Return(id, exp) => {
let results = self.results.clone();
self.gen(results.clone(), exp);
self.emit_with(*id, |attr| Bytecode::Ret(attr, results))
},
ExpData::IfElse(id, cond, then_exp, else_exp) => {
let cond_temp = self.gen_arg(cond);
let then_label = self.new_label(*id);
Expand Down Expand Up @@ -574,8 +582,8 @@ impl<'env> Generator<'env> {
Operation::Xor => self.gen_op_call(targets, id, BytecodeOperation::Xor, args),
Operation::Shl => self.gen_op_call(targets, id, BytecodeOperation::Shl, args),
Operation::Shr => self.gen_op_call(targets, id, BytecodeOperation::Shr, args),
Operation::And => self.gen_op_call(targets, id, BytecodeOperation::And, args),
Operation::Or => self.gen_op_call(targets, id, BytecodeOperation::Or, args),
Operation::And => self.gen_logical_shortcut(true, targets, id, args),
Operation::Or => self.gen_logical_shortcut(false, targets, id, args),
Operation::Eq => self.gen_op_call(targets, id, BytecodeOperation::Eq, args),
Operation::Neq => self.gen_op_call(targets, id, BytecodeOperation::Neq, args),
Operation::Lt => self.gen_op_call(targets, id, BytecodeOperation::Lt, args),
Expand Down Expand Up @@ -666,6 +674,41 @@ impl<'env> Generator<'env> {
})
}

fn gen_logical_shortcut(
&mut self,
is_and: bool,
targets: Vec<TempIndex>,
id: NodeId,
args: &[Exp],
) {
let target = self.require_unary_target(id, targets);
let arg1 = self.gen_arg(&args[0]);
let true_label = self.new_label(id);
let false_label = self.new_label(id);
let done_label = self.new_label(id);
self.emit_with(id, |attr| {
Bytecode::Branch(attr, true_label, false_label, arg1)
});
self.emit_with(id, |attr| Bytecode::Label(attr, true_label));
if is_and {
self.gen(vec![target], &args[1]);
} else {
self.emit_with(id, |attr| {
Bytecode::Load(attr, target, Constant::Bool(true))
})
}
self.emit_with(id, |attr| Bytecode::Jump(attr, done_label));
self.emit_with(id, |attr| Bytecode::Label(attr, false_label));
if is_and {
self.emit_with(id, |attr| {
Bytecode::Load(attr, target, Constant::Bool(false))
})
} else {
self.gen(vec![target], &args[1]);
}
self.emit_with(id, |attr| Bytecode::Label(attr, done_label));
}

fn gen_function_call(
&mut self,
targets: Vec<TempIndex>,
Expand Down
3 changes: 0 additions & 3 deletions third_party/move/move-compiler-v2/src/bytecode_pipeline.rs

This file was deleted.

Loading

0 comments on commit 53913b4

Please sign in to comment.