Skip to content

Commit 4af1be6

Browse files
Dentosalxgreenx
Dentosal
and
xgreenx
authored
Make Interpreter::execute and friends generic over predicate-ness (#920)
* Make Interpreter::execute generic over predicate-ness * Update fuel-vm/src/interpreter/executors/main.rs Co-authored-by: Green Baneling <[email protected]> * Update fuel-vm/src/interpreter/executors/predicate.rs Co-authored-by: Green Baneling <[email protected]> * Update fuel-vm/src/interpreter/executors/instruction.rs Co-authored-by: Green Baneling <[email protected]> * Add changelog entry * Bump CI Rust version to 1.81.0 --------- Co-authored-by: Green Baneling <[email protected]>
1 parent 992651b commit 4af1be6

File tree

10 files changed

+63
-48
lines changed

10 files changed

+63
-48
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ concurrency:
1616

1717
env:
1818
CARGO_TERM_COLOR: always
19-
RUST_VERSION: 1.79.0
19+
RUST_VERSION: 1.81.0
2020
RUST_VERSION_FMT: nightly-2024-02-07
2121
RUST_VERSION_COV: nightly-2024-06-05
2222
GIT_BRANCH: ${{ github.head_ref || github.ref_name }}

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1919
- [907](https://github.com/FuelLabs/fuel-vm/pull/907): `StorageRead` and `StorageWrite` traits no longer return the number of bytes written. They already required that the whole buffer is used, but now this is reflected in signature and documentation as well.
2020
- [914](https://github.com/FuelLabs/fuel-vm/pull/914): The built-in profiler is removed. Use the debugger with single-stepping instead.
2121
- [918](https://github.com/FuelLabs/fuel-vm/pull/918): Interpreter is now generic over the validation error handling. No behavioural changes, but the `Interpreter` has one extra type parameter.
22+
- [920](https://github.com/FuelLabs/fuel-vm/pull/920): `Interpreter::execute` and friends are now generic over predicateness of the context. This forces monomorphization so the predicate branch is optimized away on non-predicate execution.
2223

2324
### Changed
2425
- [904](https://github.com/FuelLabs/fuel-vm/pull/904): Moved the logic of each opcode into its own function. It helps so reduce the size of the `instruction_inner` function, allowing compiler to do better optimizations. The Mira swaps receive performance improvement in 16.5%.

‎fuel-vm/benches/execution.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ fn execution(c: &mut Criterion) {
5858
group_execution.bench_function("Infinite `meq` loop", |b| {
5959
b.iter(|| {
6060
for _ in 0..1000 {
61-
let _ = interpreter.execute().unwrap();
61+
let _ = interpreter.execute::<false>().unwrap();
6262
}
6363
})
6464
});
6565

6666
group_execution.bench_function("Infinite `meq` loop black box", |b| {
6767
b.iter(|| {
6868
for _ in 0..1000 {
69-
black_box(interpreter.execute()).unwrap();
69+
black_box(interpreter.execute::<false>()).unwrap();
7070
}
7171
})
7272
});
@@ -75,7 +75,7 @@ fn execution(c: &mut Criterion) {
7575
b.iter(|| {
7676
for _ in 0..1000 {
7777
unsafe {
78-
let dummy = interpreter.execute().unwrap();
78+
let dummy = interpreter.execute::<false>().unwrap();
7979
std::ptr::read_volatile(&dummy)
8080
};
8181
}
@@ -103,15 +103,15 @@ fn execution(c: &mut Criterion) {
103103
group_execution.bench_function("Infinite `add` loop", |b| {
104104
b.iter(|| {
105105
for _ in 0..1000 {
106-
let _ = interpreter.execute().unwrap();
106+
let _ = interpreter.execute::<false>().unwrap();
107107
}
108108
})
109109
});
110110

111111
group_execution.bench_function("Infinite `add` loop black box", |b| {
112112
b.iter(|| {
113113
for _ in 0..1000 {
114-
black_box(interpreter.execute()).unwrap();
114+
black_box(interpreter.execute::<false>()).unwrap();
115115
}
116116
})
117117
});
@@ -120,7 +120,7 @@ fn execution(c: &mut Criterion) {
120120
b.iter(|| {
121121
for _ in 0..1000 {
122122
unsafe {
123-
let dummy = interpreter.execute().unwrap();
123+
let dummy = interpreter.execute::<false>().unwrap();
124124
std::ptr::read_volatile(&dummy)
125125
};
126126
}
@@ -148,15 +148,15 @@ fn execution(c: &mut Criterion) {
148148
group_execution.bench_function("Infinite `not` loop", |b| {
149149
b.iter(|| {
150150
for _ in 0..1000 {
151-
let _ = interpreter.execute().unwrap();
151+
let _ = interpreter.execute::<false>().unwrap();
152152
}
153153
})
154154
});
155155

156156
group_execution.bench_function("Infinite `not` loop black box", |b| {
157157
b.iter(|| {
158158
for _ in 0..1000 {
159-
black_box(interpreter.execute()).unwrap();
159+
black_box(interpreter.execute::<false>()).unwrap();
160160
}
161161
})
162162
});
@@ -165,7 +165,7 @@ fn execution(c: &mut Criterion) {
165165
b.iter(|| {
166166
for _ in 0..1000 {
167167
unsafe {
168-
let dummy = interpreter.execute().unwrap();
168+
let dummy = interpreter.execute::<false>().unwrap();
169169
std::ptr::read_volatile(&dummy)
170170
};
171171
}

‎fuel-vm/src/interpreter/diff/tests.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ fn reset_vm_state() {
4242
let desired = Interpreter::<_, _, Script>::with_memory_storage();
4343
let mut latest = Interpreter::<_, _, Script>::with_memory_storage();
4444
latest.set_gas(1_000_000);
45-
latest.instruction(op::addi(0x10, 0x11, 1)).unwrap();
45+
latest
46+
.instruction::<_, false>(op::addi(0x10, 0x11, 1))
47+
.unwrap();
4648
let diff: Diff<InitialVmState> = latest.rollback_to(&desired).into();
4749
assert_ne!(desired, latest);
4850
latest.reset_vm_state(&diff);
@@ -75,7 +77,9 @@ fn record_and_invert_storage() {
7577
)
7678
.unwrap();
7779
latest.set_gas(1_000_000);
78-
latest.instruction(op::addi(0x10, 0x11, 1)).unwrap();
80+
latest
81+
.instruction::<_, false>(op::addi(0x10, 0x11, 1))
82+
.unwrap();
7983

8084
let storage_diff: Diff<InitialVmState> = latest.storage_diff().into();
8185
let mut diff: Diff<InitialVmState> = latest.rollback_to(&desired).into();
@@ -95,7 +99,7 @@ fn record_and_invert_storage() {
9599
)
96100
.unwrap();
97101
d.set_gas(1_000_000);
98-
d.instruction(op::addi(0x10, 0x11, 1)).unwrap();
102+
d.instruction::<_, false>(op::addi(0x10, 0x11, 1)).unwrap();
99103

100104
assert_ne!(c, d);
101105
d.reset_vm_state(&diff);

‎fuel-vm/src/interpreter/executors/instruction.rs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@ where
3232
V: Verifier,
3333
{
3434
/// Execute the current instruction located in `$m[$pc]`.
35-
pub fn execute(&mut self) -> Result<ExecuteState, InterpreterError<S::DataError>> {
35+
pub fn execute<const PREDICATE: bool>(
36+
&mut self,
37+
) -> Result<ExecuteState, InterpreterError<S::DataError>> {
3638
let raw_instruction = self.fetch_instruction()?;
37-
self.instruction_per_inner(raw_instruction)
39+
self.instruction_per_inner::<PREDICATE>(raw_instruction)
3840
}
3941

4042
/// Reads the current instruction located in `$m[$pc]`,
@@ -59,17 +61,20 @@ where
5961
}
6062

6163
/// Execute a provided instruction
62-
pub fn instruction<R: Into<RawInstruction> + Copy>(
64+
pub fn instruction<R, const PREDICATE: bool>(
6365
&mut self,
6466
raw: R,
65-
) -> Result<ExecuteState, InterpreterError<S::DataError>> {
67+
) -> Result<ExecuteState, InterpreterError<S::DataError>>
68+
where
69+
R: Into<RawInstruction> + Copy,
70+
{
6671
let raw = raw.into();
6772
let raw = raw.to_be_bytes();
6873

69-
self.instruction_per_inner(raw)
74+
self.instruction_per_inner::<PREDICATE>(raw)
7075
}
7176

72-
fn instruction_per_inner(
77+
fn instruction_per_inner<const PREDICATE: bool>(
7378
&mut self,
7479
raw: [u8; 4],
7580
) -> Result<ExecuteState, InterpreterError<S::DataError>> {
@@ -80,22 +85,24 @@ where
8085
}
8186
}
8287

83-
self.instruction_inner(raw).map_err(|e| {
88+
self.instruction_inner::<PREDICATE>(raw).map_err(|e| {
8489
InterpreterError::from_runtime(e, RawInstruction::from_be_bytes(raw))
8590
})
8691
}
8792

88-
fn instruction_inner(
93+
fn instruction_inner<const PREDICATE: bool>(
8994
&mut self,
9095
raw: [u8; 4],
9196
) -> IoResult<ExecuteState, S::DataError> {
9297
let instruction = Instruction::try_from(raw)
9398
.map_err(|_| RuntimeError::from(PanicReason::InvalidInstruction))?;
9499

95-
// TODO additional branch that might be optimized after
96-
// https://github.com/FuelLabs/fuel-asm/issues/68
97-
if self.is_predicate() && !instruction.opcode().is_predicate_allowed() {
98-
return Err(PanicReason::ContractInstructionNotAllowed.into())
100+
if PREDICATE {
101+
// TODO additional branch that might be optimized after
102+
// https://github.com/FuelLabs/fuel-asm/issues/68
103+
if !instruction.opcode().is_predicate_allowed() {
104+
return Err(PanicReason::ContractInstructionNotAllowed.into())
105+
}
99106
}
100107

101108
instruction.execute(self)

‎fuel-vm/src/interpreter/executors/instruction/tests/reserved_registers.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ fn cant_write_to_reserved_registers(raw_random_instruction: u32) -> TestResult {
7979
.expect("failed dynamic checks");
8080

8181
vm.init_script(tx).expect("Failed to init VM");
82-
let res = vm.instruction(raw_random_instruction);
82+
let res = vm.instruction::<_, false>(raw_random_instruction);
8383

8484
if writes_to_ra(opcode) || writes_to_rb(opcode) {
8585
// if this opcode writes to $rA or $rB, expect an error since we're attempting to

‎fuel-vm/src/interpreter/executors/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ where
981981
// Check whether the instruction will be executed in a call context
982982
let in_call = !self.frames.is_empty();
983983

984-
match self.execute() {
984+
match self.execute::<false>() {
985985
// Proceeding with the execution normally
986986
Ok(ExecuteState::Proceed) => continue,
987987
// Debugger events are returned directly to the caller

‎fuel-vm/src/interpreter/executors/predicate.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ where
3030
&mut self,
3131
) -> Result<ProgramState, PredicateVerificationFailed> {
3232
loop {
33-
match self.execute()? {
33+
match self.execute::<true>()? {
3434
ExecuteState::Return(r) => {
3535
if r == 1 {
3636
return Ok(ProgramState::Return(r))

‎fuel-vm/src/interpreter/internal.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,6 @@ where
111111
&self.context
112112
}
113113

114-
pub(crate) const fn is_predicate(&self) -> bool {
115-
self.context.is_predicate()
116-
}
117-
118114
pub(crate) fn internal_contract(&self) -> Result<ContractId, PanicReason> {
119115
internal_contract(&self.context, self.registers.fp(), self.memory.as_ref())
120116
}

‎fuel-vm/src/interpreter/memory/tests.rs‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,45 +48,51 @@ fn memcopy() {
4848
let alloc = 1024;
4949

5050
// r[0x10] := 1024
51-
vm.instruction(op::addi(0x10, RegId::ZERO, alloc)).unwrap();
52-
vm.instruction(op::aloc(0x10)).unwrap();
51+
vm.instruction::<_, false>(op::addi(0x10, RegId::ZERO, alloc))
52+
.unwrap();
53+
vm.instruction::<_, false>(op::aloc(0x10)).unwrap();
5354

5455
// r[0x20] := 128
55-
vm.instruction(op::addi(0x20, RegId::ZERO, 128)).unwrap();
56+
vm.instruction::<_, false>(op::addi(0x20, RegId::ZERO, 128))
57+
.unwrap();
5658

5759
for i in 0..alloc {
58-
vm.instruction(op::addi(0x21, RegId::ZERO, i)).unwrap();
59-
vm.instruction(op::sb(RegId::HP, 0x21, i as Immediate12))
60+
vm.instruction::<_, false>(op::addi(0x21, RegId::ZERO, i))
61+
.unwrap();
62+
vm.instruction::<_, false>(op::sb(RegId::HP, 0x21, i as Immediate12))
6063
.unwrap();
6164
}
6265

6366
// r[0x23] := m[$hp, 0x20] == m[$zero, 0x20]
64-
vm.instruction(op::meq(0x23, RegId::HP, RegId::ZERO, 0x20))
67+
vm.instruction::<_, false>(op::meq(0x23, RegId::HP, RegId::ZERO, 0x20))
6568
.unwrap();
6669

6770
assert_eq!(0, vm.registers()[0x23]);
6871

6972
// r[0x12] := $hp + r[0x20]
70-
vm.instruction(op::add(0x12, RegId::HP, 0x20)).unwrap();
73+
vm.instruction::<_, false>(op::add(0x12, RegId::HP, 0x20))
74+
.unwrap();
7175

7276
// Test ownership
73-
vm.instruction(op::mcp(RegId::HP, 0x12, 0x20)).unwrap();
77+
vm.instruction::<_, false>(op::mcp(RegId::HP, 0x12, 0x20))
78+
.unwrap();
7479

7580
// r[0x23] := m[0x30, 0x20] == m[0x12, 0x20]
76-
vm.instruction(op::meq(0x23, RegId::HP, 0x12, 0x20))
81+
vm.instruction::<_, false>(op::meq(0x23, RegId::HP, 0x12, 0x20))
7782
.unwrap();
7883

7984
assert_eq!(1, vm.registers()[0x23]);
8085

8186
// Assert ownership
82-
vm.instruction(op::subi(0x24, RegId::HP, 1)).unwrap(); // TODO: look into this
83-
let ownership_violated = vm.instruction(op::mcp(0x24, 0x12, 0x20));
87+
vm.instruction::<_, false>(op::subi(0x24, RegId::HP, 1))
88+
.unwrap(); // TODO: look into this
89+
let ownership_violated = vm.instruction::<_, false>(op::mcp(0x24, 0x12, 0x20));
8490

8591
assert!(ownership_violated.is_err());
8692

8793
// Assert no panic on overlapping
88-
vm.instruction(op::subi(0x25, 0x12, 1)).unwrap();
89-
let overlapping = vm.instruction(op::mcp(RegId::HP, 0x25, 0x20));
94+
vm.instruction::<_, false>(op::subi(0x25, 0x12, 1)).unwrap();
95+
let overlapping = vm.instruction::<_, false>(op::mcp(RegId::HP, 0x25, 0x20));
9096

9197
assert!(overlapping.is_err());
9298
}
@@ -112,11 +118,12 @@ fn stack_alloc_ownership() {
112118
.unwrap();
113119
vm.init_script(tx).expect("Failed to init VM");
114120

115-
vm.instruction(op::move_(0x10, RegId::SP)).unwrap();
116-
vm.instruction(op::cfei(2)).unwrap();
121+
vm.instruction::<_, false>(op::move_(0x10, RegId::SP))
122+
.unwrap();
123+
vm.instruction::<_, false>(op::cfei(2)).unwrap();
117124

118125
// Assert allocated stack is writable
119-
vm.instruction(op::mcli(0x10, 2)).unwrap();
126+
vm.instruction::<_, false>(op::mcli(0x10, 2)).unwrap();
120127
}
121128

122129
#[test_case(

0 commit comments

Comments
 (0)