Skip to content

Commit

Permalink
[compiler v2] Pulls out fixes from #10544 to unblock testing (#10887)
Browse files Browse the repository at this point in the history 
* [compiler v2] Pulls out fixes from #10544 to umblock testing

There are fixes how to deal with global resources in #10544, pulling them out to unblock testing.

* Addressing reviewer comments, fixing tests, adding test from original PR
  • Loading branch information
@wrwg @rtso
wrwg authored and rtso committed Nov 22, 2023
1 parent aff2a97 commit b60af4c
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 61 deletions.
82 changes: 56 additions & 26 deletions third_party/move/move-compiler-v2/src/bytecode_generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ impl<'env> Generator<'env> {
self.func_env.module_env.env
}

/// Shortcut to get type of a node.
fn get_node_type(&'env self, id: NodeId) -> Type {
self.env().get_node_type(id)
}

/// Emit a bytecode.
fn emit(&mut self, b: Bytecode) {
self.code.push(b)
Expand Down Expand Up @@ -197,7 +202,7 @@ impl<'env> Generator<'env> {
self.error(
id,
format!("cannot assign tuple type `{}` to single variable (use `(a, b, ..) = ..` instead)",
ty.display(&self.env().get_type_display_ctx()))
ty.display(&self.env().get_type_display_ctx()))
)
}
let next_idx = self.temps.len();
Expand Down Expand Up @@ -299,7 +304,6 @@ impl<'env> Generator<'env> {
// Result is thrown away, but for typing reasons, we need to introduce
// temps to construct the step target.
let step_targets = self
.env()
.get_node_type(step.node_id())
.flatten()
.into_iter()
Expand All @@ -318,7 +322,7 @@ impl<'env> Generator<'env> {
// Declare all variables bound by the pattern
let mut scope = BTreeMap::new();
for (id, sym) in pat.vars() {
let ty = self.env().get_node_type(id);
let ty = self.get_node_type(id);
let temp = self.new_temp_with_valid_type(id, ty);
scope.insert(sym, temp);
}
Expand Down Expand Up @@ -430,7 +434,7 @@ impl<'env> Generator<'env> {

/// Convert a value from AST world into a constant as expected in bytecode.
fn to_constant(&self, id: NodeId, val: &Value) -> Constant {
let ty = self.env().get_node_type(id);
let ty = self.get_node_type(id);
match val {
Value::Address(x) => Constant::Address(x.clone()),
Value::Number(x) => match ty {
Expand Down Expand Up @@ -521,10 +525,25 @@ impl<'env> Generator<'env> {
self.gen_op_call(targets, id, BytecodeOperation::Pack(*mid, *sid, inst), args)
},
Operation::Select(mid, sid, fid) => {
let inst = self.env().get_node_instantiation(id);
let target = self.require_unary_target(id, targets);
let arg = self.require_unary_arg(id, args);
self.gen_select(target, id, mid.qualified_inst(*sid, inst), *fid, &arg)
// Get the instantiation of the struct. It is not contained in the select
// expression but in the type of it's operand.
if let Some((_, inst)) = self
.get_node_type(arg.node_id())
.skip_reference()
.get_struct(self.env())
{
self.gen_select(
target,
id,
mid.qualified_inst(*sid, inst.to_vec()),
*fid,
&arg,
)
} else {
self.internal_error(id, "inconsistent type in select expression")
}
},
Operation::Exists(None) => {
let inst = self.env().get_node_instantiation(id);
Expand Down Expand Up @@ -654,7 +673,7 @@ impl<'env> Generator<'env> {
}

fn gen_cast_call(&mut self, targets: Vec<TempIndex>, id: NodeId, args: &[Exp]) {
let ty = self.env().get_node_type(id);
let ty = self.get_node_type(id);
let bytecode_op = match ty {
Type::Primitive(PrimitiveType::U8) => BytecodeOperation::CastU8,
Type::Primitive(PrimitiveType::U16) => BytecodeOperation::CastU16,
Expand Down Expand Up @@ -763,7 +782,7 @@ impl<'env> Generator<'env> {
/// 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);
let exp_ty = self.get_node_type(id);
if let (
Type::Reference(ReferenceKind::Mutable, _),
Type::Reference(ReferenceKind::Immutable, et),
Expand All @@ -788,22 +807,19 @@ impl<'env> Generator<'env> {
match exp.as_ref() {
ExpData::Temporary(_, temp) => *temp,
ExpData::LocalVar(id, sym) => self.find_local(*id, *sym),
ExpData::Call(_, Operation::Select(..), _) if self.reference_mode() => {
ExpData::Call(id, Operation::Select(..), _) if self.reference_mode() => {
// In reference mode, a selection is interpreted as selecting a reference to the
// field.
let id = exp.node_id();
let ty = Type::Reference(
self.reference_mode_kind,
Box::new(self.env().get_node_type(id)),
);
let ty =
Type::Reference(self.reference_mode_kind, Box::new(self.get_node_type(*id)));
let temp = self.new_temp(ty);
self.gen(vec![temp], exp);
temp
},
_ => {
// Otherwise, introduce a temporary
let id = exp.node_id();
let ty = self.env().get_node_type(id);
let ty = self.get_node_type(id);
let temp = self.new_temp(ty);
self.gen(vec![temp], exp);
temp
Expand Down Expand Up @@ -844,10 +860,10 @@ impl<'env> Generator<'env> {
impl<'env> Generator<'env> {
fn gen_borrow(&mut self, target: TempIndex, id: NodeId, kind: ReferenceKind, arg: &Exp) {
match arg.as_ref() {
ExpData::Call(_arg_id, Operation::Select(mid, sid, fid), args) => {
ExpData::Call(arg_id, Operation::Select(mid, sid, fid), args) => {
return self.gen_borrow_field(
target,
id,
*arg_id,
kind,
mid.qualified(*sid),
*fid,
Expand Down Expand Up @@ -924,13 +940,27 @@ impl<'env> Generator<'env> {
// Currently no conditions for immutable, so we do allow `&fun().field`.
},
}
let inst = self.env().get_node_instantiation(id);
self.emit_call(
id,
vec![target],
BytecodeOperation::BorrowField(struct_id.module_id, struct_id.id, inst, field_offset),
vec![temp],
);
// Get instantiation of field. It is not contained in the select expression but in the
// type of its operand.
if let Some((_, inst)) = self
.get_node_type(oper.node_id())
.skip_reference()
.get_struct(self.env())
{
self.emit_call(
id,
vec![target],
BytecodeOperation::BorrowField(
struct_id.module_id,
struct_id.id,
inst.to_vec(),
field_offset,
),
vec![temp],
);
} else {
self.internal_error(id, "inconsistent type in select expression")
}
}
}

Expand Down Expand Up @@ -1100,7 +1130,7 @@ impl<'env> Generator<'env> {
Pattern::Wildcard(id) => {
// Wildcard pattern: we need to create a temporary to receive the value, even
// if its dropped afterwards.
let temp = self.new_temp(self.env().get_node_type(*id));
let temp = self.new_temp(self.get_node_type(*id));
(temp, None)
},
Pattern::Var(id, sym) => {
Expand All @@ -1112,7 +1142,7 @@ impl<'env> Generator<'env> {
// Pattern is not flat: create a new temporary and an Assignment of this
// temporary to the pattern.
let id = pat.node_id();
let ty = self.env().get_node_type(id);
let ty = self.get_node_type(id);
let temp = self.new_temp_with_valid_type(id, ty);
(temp, Some((id, pat.clone(), temp)))
},
Expand Down
16 changes: 14 additions & 2 deletions third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ struct LabelInfo {

impl<'a> FunctionGenerator<'a> {
/// Runs the function generator for the given function.
pub fn run<'b>(gen: &'a mut ModuleGenerator, ctx: &'b ModuleContext, fun_env: FunctionEnv<'b>) {
pub fn run<'b>(
gen: &'a mut ModuleGenerator,
ctx: &'b ModuleContext,
fun_env: FunctionEnv<'b>,
acquires_list: &BTreeSet<StructId>,
) {
let loc = fun_env.get_loc();
let function = gen.function_index(ctx, &loc, &fun_env);
let visibility = fun_env.visibility();
Expand All @@ -111,11 +116,18 @@ impl<'a> FunctionGenerator<'a> {
} else {
(gen, None)
};
let acquires_global_resources = acquires_list
.iter()
.map(|id| {
let struct_env = fun_env.module_env.get_struct(*id);
gen.struct_def_index(ctx, &struct_env.get_loc(), &struct_env)
})
.collect();
let def = FF::FunctionDefinition {
function,
visibility,
is_entry: fun_env.is_entry(),
acquires_global_resources: vec![],
acquires_global_resources,
code,
};
ctx.checked_bound(
Expand Down
68 changes: 65 additions & 3 deletions third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ use move_model::{
ty::{PrimitiveType, ReferenceKind, Type},
};
use move_stackless_bytecode::{
function_target_pipeline::FunctionTargetsHolder, stackless_bytecode::Constant,
function_target_pipeline::{FunctionTargetsHolder, FunctionVariant},
stackless_bytecode::{Bytecode, Constant, Operation},
};
use move_symbol_pool::symbol as IR_SYMBOL;
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};

/// Internal state of the module code generator
#[derive(Debug)]
Expand Down Expand Up @@ -144,8 +145,11 @@ impl ModuleGenerator {
for struct_env in module_env.get_structs() {
self.gen_struct(ctx, &struct_env)
}

let acquires_map = ctx.generate_acquires_map(module_env);
for fun_env in module_env.get_functions() {
FunctionGenerator::run(self, ctx, fun_env);
let acquires_list = &acquires_map[&fun_env.get_id()];
FunctionGenerator::run(self, ctx, fun_env, acquires_list);
}
}

Expand Down Expand Up @@ -766,3 +770,61 @@ impl<'env> ModuleContext<'env> {
s.display(self.env.symbol_pool()).to_string()
}
}

impl<'env> ModuleContext<'env> {
/// Acquires analysis. This is temporary until we have the full reference analysis.
fn generate_acquires_map(&self, module: &ModuleEnv) -> BTreeMap<FunId, BTreeSet<StructId>> {
// Compute map with direct usage of resources
let mut usage_map = module
.get_functions()
.map(|f| (f.get_id(), self.get_direct_function_acquires(&f)))
.collect::<BTreeMap<_, _>>();
// Now run a fixed-point loop: add resources used by called functions until there are no
// changes.
loop {
let mut changes = false;
for fun in module.get_functions() {
if let Some(callees) = fun.get_called_functions() {
let mut usage = usage_map[&fun.get_id()].clone();
let count = usage.len();
// Extend usage by that of callees from the same module. Acquires is only
// local to a module.
for callee in callees {
if callee.module_id == module.get_id() {
usage.extend(usage_map[&callee.id].iter().cloned());
}
}
if usage.len() > count {
*usage_map.get_mut(&fun.get_id()).unwrap() = usage;
changes = true;
}
}
}
if !changes {
break;
}
}
usage_map
}

fn get_direct_function_acquires(&self, fun: &FunctionEnv) -> BTreeSet<StructId> {
let mut result = BTreeSet::new();
let target = self.targets.get_target(fun, &FunctionVariant::Baseline);
for bc in target.get_bytecode() {
use Bytecode::*;
use Operation::*;
match bc {
Call(_, _, MoveFrom(mid, sid, ..), ..)
| Call(_, _, BorrowGlobal(mid, sid, ..), ..)
// GetGlobal from spec language, but cover it anyway.
| Call(_, _, GetGlobal(mid, sid, ..), ..)
if *mid == fun.module_env.get_id() =>
{
result.insert(*sid);
},
_ => {},
}
}
result
}
}
36 changes: 36 additions & 0 deletions third_party/move/move-compiler-v2/tests/bytecode-generator/fields.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,25 @@ module 0x42::fields {
struct T {
h: u64,
}
struct G {
f: #0,
}
struct S {
f: u64,
g: fields::T,
}
private fun read_generic_val(x: fields::G<u64>): u64 {
select fields::G.f(x)
}
private fun read_ref(x: &fields::S): u64 {
select fields::T.h(select fields::S.g(x))
}
private fun read_val(x: fields::S): u64 {
select fields::T.h(select fields::S.g(x))
}
private fun write_generic_val(x: &mut fields::G<u64>,v: u64) {
select fields::G.f(x) = v
}
private fun write_local_direct(): fields::S {
{
let x: fields::S = pack fields::S(0, pack fields::T(0));
Expand All @@ -38,12 +47,18 @@ module 0x42::fields {
select fields::T.h(select fields::S.g(x)) = 42;
x
}
spec fun $read_generic_val(x: fields::G<u64>): u64 {
select fields::G.f(x)
}
spec fun $read_ref(x: fields::S): u64 {
select fields::T.h(select fields::S.g(x))
}
spec fun $read_val(x: fields::S): u64 {
select fields::T.h(select fields::S.g(x))
}
spec fun $write_generic_val(x: fields::G<u64>,v: u64) {
select fields::G.f(x) = v
}
spec fun $write_local_direct(): fields::S;
spec fun $write_local_via_ref(): fields::S;
spec fun $write_param(x: &mut fields::S);
Expand All @@ -52,6 +67,18 @@ module 0x42::fields {

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

[variant baseline]
fun fields::read_generic_val($t0: fields::G<u64>): u64 {
var $t1: u64
var $t2: &fields::G<u64>
var $t3: &u64
0: $t2 := borrow_local($t0)
1: $t3 := borrow_field<fields::G<u64>>.f($t2)
2: $t1 := read_ref($t3)
3: return $t1
}


[variant baseline]
fun fields::read_ref($t0: &fields::S): u64 {
var $t1: u64
Expand All @@ -78,6 +105,15 @@ fun fields::read_val($t0: fields::S): u64 {
}


[variant baseline]
fun fields::write_generic_val($t0: &mut fields::G<u64>, $t1: u64) {
var $t2: &mut u64
0: $t2 := borrow_field<fields::G<u64>>.f($t0)
1: write_ref($t2, $t1)
2: return ()
}


[variant baseline]
fun fields::write_local_direct(): fields::S {
var $t0: fields::S
Expand Down
Loading

0 comments on commit b60af4c

Please sign in to comment.