Skip to content

Commit

Permalink
Add a simple simplify-cfg optimization for assembly (#6197)
Browse files Browse the repository at this point in the history
## Description
When compiling code such as

```
fn main() {
     while true {}
}
```
we would end up with unreachable code in the assembly:
```
.program:
.2                                      ; --- start of function: main_0 ---
pusha .2                                ; save all regs
move $$locbase $sp                      ; save locals base register for main_0
cfei i0                                 ; allocate 0 bytes for locals and 0 slots for call arguments.
move $r28 $$reta                        ; save reta
move $r29 $$retv                        ; save retv
.6
ji  .7
.7
ji  .7
.3
cfsi i0                                 ; free 0 bytes for locals and 0 slots for extra call arguments.
move $$reta $r28                        ; restore reta
popa .2                                 ; restore all regs
jmp $$reta                              ; return from call

;; --- Data ---
.data:
```

Here the label `.3` and what follows is unreachable because the program
gets stuck in an infinite loops with label `.7`. This means that the
instruction `move $r28 $$reta` is dead and hence removed by ASM DCE.

Our
[verifier](https://github.com/FuelLabs/sway/blob/4a6cf38143b398a568d9587f79bfa46c15f48459/sway-core/src/asm_generation/fuel/abstract_instruction_set.rs#L131)
on the other hand will see a use of `$r28` in the unreachable code. But
with the definition having been eliminated, it will now flag an error.

The verifier is unaware of reachability. Making the verifier more
accurate is probably an overkill. So we workaround this by adding an
unreachable code eliminator optimization in the assembly stage. While it
has its own benefits (of removing unreachable code), it'll also make the
verifier happy as-is. The algorithm is linear in the size of the code.

Closes #6174.

---------

Co-authored-by: Igor Rončević <ironcev@hotmail.com>
  • Loading branch information
vaivaswatha and ironcev committed Jun 28, 2024
1 parent 03c09bf commit fc2a90b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ impl AbstractInstructionSet {
pub(crate) fn optimize(self, data_section: &DataSection) -> AbstractInstructionSet {
self.const_indexing_aggregates_function(data_section)
.dce()
.simplify_cfg()
.remove_sequential_jumps()
.remove_redundant_moves()
.remove_unused_ops()
Expand Down
44 changes: 42 additions & 2 deletions sway-core/src/asm_generation/fuel/optimizations.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::collections::BTreeSet;
use std::collections::{BTreeSet, HashMap};

use either::Either;
use rustc_hash::{FxHashMap, FxHashSet};

use crate::{
asm_generation::fuel::compiler_constants,
asm_lang::{ControlFlowOp, VirtualImmediate12, VirtualOp, VirtualRegister},
asm_lang::{ControlFlowOp, Label, VirtualImmediate12, VirtualOp, VirtualRegister},
};

use super::{
Expand Down Expand Up @@ -269,4 +269,44 @@ impl AbstractInstructionSet {

self
}

// Remove unreachable instructions.
pub(crate) fn simplify_cfg(mut self) -> AbstractInstructionSet {
let ops = &self.ops;

if ops.is_empty() {
return self;
}

// Keep track of a map between jump labels and op indices. Useful to compute op successors.
let mut label_to_index: HashMap<Label, usize> = HashMap::default();
for (idx, op) in ops.iter().enumerate() {
if let Either::Right(ControlFlowOp::Label(op_label)) = op.opcode {
label_to_index.insert(op_label, idx);
}
}

let mut reachables = vec![false; ops.len()];
let mut worklist = vec![0];
while let Some(op_idx) = worklist.pop() {
assert!(!reachables[op_idx]);
reachables[op_idx] = true;
let op = &ops[op_idx];
for s in &op.successors(op_idx, ops, &label_to_index) {
if !reachables[*s] {
worklist.push(*s);
}
}
}

let reachable_ops = self
.ops
.into_iter()
.enumerate()
.filter_map(|(idx, op)| if reachables[idx] { Some(op) } else { None })
.collect();
self.ops = reachable_ops;

self
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
category = "disabled" # TODO: Enable once https://github.com/FuelLabs/sway/issues/6174 is fixed.
category = "run"
expected_result = { action = "revert", value = 42 }
expected_result_new_encoding = { action = "revert", value = 42 }
validate_abi = true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
category = "disabled" # TODO: Enable once https://github.com/FuelLabs/sway/issues/6174 is fixed.
category = "compile"

# not: $()This returns a value of type unknown, which is not assigned to anything and is ignored.

0 comments on commit fc2a90b

Please sign in to comment.