Use relatives jumps (FuelLabs#4804)

## Description Switch the compiler to use relative jumps instead of absolute jumps. 1. The function `relocate_control_flow` is now removed. It used to move basic blocks around in case absolute addresses crossed the easily addressable boundary. The code movement can then avoid having to load the address to a register and using indirect jumps. This is unlikely to happen with relative jumps. But if it does, the existing algorithm wouldn't work anyway. So we can add a new one when required later. 2. In the case of function call jumps, we use `$pc` to save the return address for the callee. This is a bit inefficient as it involves 3 extra computations: `$$reta = ($pc - $is) / 4 + offset_to_return_point`. We could change this to the following: Since we already have a common return point for every function, compute the relative offset from that point to the callee return point statically and use that. We don't have the infrastructure to do that right now, but it shouldn't be too difficult an extension. Fixes FuelLabs#4443
taaraneh · Jul 18, 2023 · 01ee3cb · 01ee3cb
1 parent 62f9857
commit 01ee3cb
Show file tree

Hide file tree

Showing 32 changed files with 426 additions and 501 deletions.
diff --git a/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs b/sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs
diff --git a/sway-core/src/asm_generation/fuel/functions.rs b/sway-core/src/asm_generation/fuel/functions.rs
@@ -80,7 +80,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {
 
         // Set a new return address.
         let ret_label = self.reg_seqr.get_label();
-        self.cur_bytecode.push(Op::move_address(
+        self.cur_bytecode.push(Op::save_ret_addr(
             VirtualRegister::Constant(ConstantRegister::CallReturnAddress),
             ret_label,
             "set new return addr",

diff --git a/sway-core/src/asm_generation/programs/allocated.rs b/sway-core/src/asm_generation/programs/allocated.rs
@@ -12,9 +12,8 @@ impl AllocatedProgram {
                 .collect(),
         };
 
-        let (realized_ops, mut label_offsets) = abstract_ops
-            .relocate_control_flow(&self.data_section)
-            .realize_labels(&mut self.data_section)?;
+        let (realized_ops, mut label_offsets) =
+            abstract_ops.realize_labels(&mut self.data_section)?;
         let ops = realized_ops.pad_to_even();
 
         // Collect the entry point offsets.

diff --git a/sway-core/src/asm_lang/allocated_ops.rs b/sway-core/src/asm_lang/allocated_ops.rs
@@ -98,6 +98,10 @@ pub(crate) enum AllocatedOpcode {
     JNE(AllocatedRegister, AllocatedRegister, AllocatedRegister),
     JNEI(AllocatedRegister, AllocatedRegister, VirtualImmediate12),
     JNZI(AllocatedRegister, VirtualImmediate18),
+    JMPB(AllocatedRegister, VirtualImmediate18),
+    JMPF(AllocatedRegister, VirtualImmediate18),
+    JNZB(AllocatedRegister, AllocatedRegister, VirtualImmediate12),
+    JNZF(AllocatedRegister, AllocatedRegister, VirtualImmediate12),
     RET(AllocatedRegister),
 
     /* Memory Instructions */
@@ -246,6 +250,10 @@ impl AllocatedOpcode {
             JNE(_r1, _r2, _r3) => vec![],
             JNEI(_r1, _r2, _i) => vec![],
             JNZI(_r1, _i) => vec![],
+            JMPB(_r1, _i) => vec![],
+            JMPF(_r1, _i) => vec![],
+            JNZB(_r1, _r2, _i) => vec![],
+            JNZF(_r1, _r2, _i) => vec![],
             RET(_r1) => vec![],
 
             /* Memory Instructions */
@@ -355,6 +363,10 @@ impl fmt::Display for AllocatedOpcode {
             JNE(a, b, c) => write!(fmtr, "jne  {a} {b} {c}"),
             JNEI(a, b, c) => write!(fmtr, "jnei {a} {b} {c}"),
             JNZI(a, b) => write!(fmtr, "jnzi {a} {b}"),
+            JMPB(a, b) => write!(fmtr, "jmpb {a} {b}"),
+            JMPF(a, b) => write!(fmtr, "jmpf {a} {b}"),
+            JNZB(a, b, c) => write!(fmtr, "jnzb {a} {b} {c}"),
+            JNZF(a, b, c) => write!(fmtr, "jnzf {a} {b} {c}"),
             RET(a) => write!(fmtr, "ret  {a}"),
 
             /* Memory Instructions */
@@ -495,6 +507,10 @@ impl AllocatedOp {
             JNE(a, b, c) => op::JNE::new(a.to_reg_id(), b.to_reg_id(), c.to_reg_id()).into(),
             JNEI(a, b, c) => op::JNEI::new(a.to_reg_id(), b.to_reg_id(), c.value.into()).into(),
             JNZI(a, b) => op::JNZI::new(a.to_reg_id(), b.value.into()).into(),
+            JMPB(a, b) => op::JMPB::new(a.to_reg_id(), b.value.into()).into(),
+            JMPF(a, b) => op::JMPF::new(a.to_reg_id(), b.value.into()).into(),
+            JNZB(a, b, c) => op::JNZB::new(a.to_reg_id(), b.to_reg_id(), c.value.into()).into(),
+            JNZF(a, b, c) => op::JNZF::new(a.to_reg_id(), b.to_reg_id(), c.value.into()).into(),
             RET(a) => op::RET::new(a.to_reg_id()).into(),
 
             /* Memory Instructions */

diff --git a/sway-core/src/asm_lang/mod.rs b/sway-core/src/asm_lang/mod.rs
@@ -186,14 +186,14 @@ impl Op {
     }
 
     /// Move an address at a label into a register.
-    pub(crate) fn move_address(
+    pub(crate) fn save_ret_addr(
         reg: VirtualRegister,
         label: Label,
         comment: impl Into<String>,
         owning_span: Option<Span>,
     ) -> Self {
         Op {
-            opcode: Either::Right(OrganizationalOp::MoveAddress(reg, label)),
+            opcode: Either::Right(OrganizationalOp::SaveRetAddr(reg, label)),
             comment: comment.into(),
             owning_span,
         }
@@ -237,19 +237,6 @@ impl Op {
         }
     }
 
-    /// Jumps to [Label] `label`  if the given [VirtualRegister] `reg1` is not equal to `reg0`.
-    pub(crate) fn jump_if_not_equal(
-        reg0: VirtualRegister,
-        reg1: VirtualRegister,
-        label: Label,
-    ) -> Self {
-        Op {
-            opcode: Either::Right(OrganizationalOp::JumpIfNotEq(reg0, reg1, label)),
-            comment: String::new(),
-            owning_span: None,
-        }
-    }
-
     /// Jumps to [Label] `label`  if the given [VirtualRegister] `reg0` is not equal to zero.
     pub(crate) fn jump_if_not_zero(reg0: VirtualRegister, label: Label) -> Self {
         Op {
@@ -1550,14 +1537,12 @@ pub(crate) enum ControlFlowOp<Reg> {
     Comment,
     // Jumps to a label
     Jump(Label),
-    // Jumps to a label if the two registers are different
-    JumpIfNotEq(Reg, Reg, Label),
     // Jumps to a label if the register is not equal to zero
     JumpIfNotZero(Reg, Label),
     // Jumps to a label, similarly to Jump, though semantically expecting to return.
     Call(Label),
-    // Save a label address in a register.
-    MoveAddress(Reg, Label),
+    // Save a return label address in a register.
+    SaveRetAddr(Reg, Label),
     // placeholder for the DataSection offset
     DataSectionOffsetPlaceholder,
     // Placeholder for loading an address from the data section.
@@ -1580,10 +1565,9 @@ impl<Reg: fmt::Display> fmt::Display for ControlFlowOp<Reg> {
                 Label(lab) => format!("{lab}"),
                 Jump(lab) => format!("ji  {lab}"),
                 Comment => "".into(),
-                JumpIfNotEq(r1, r2, lab) => format!("jnei {r1} {r2} {lab}"),
                 JumpIfNotZero(r1, lab) => format!("jnzi {r1} {lab}"),
                 Call(lab) => format!("fncall {lab}"),
-                MoveAddress(r1, lab) => format!("mova {r1} {lab}"),
+                SaveRetAddr(r1, lab) => format!("mova {r1} {lab}"),
                 DataSectionOffsetPlaceholder =>
                     "DATA SECTION OFFSET[0..32]\nDATA SECTION OFFSET[32..64]".into(),
                 LoadLabel(r1, lab) => format!("lwlab {r1} {lab}"),
@@ -1606,8 +1590,7 @@ impl<Reg: Clone + Eq + Ord + Hash> ControlFlowOp<Reg> {
             | PushAll(_)
             | PopAll(_) => vec![],
 
-            JumpIfNotEq(r1, r2, _) => vec![r1, r2],
-            JumpIfNotZero(r1, _) | MoveAddress(r1, _) | LoadLabel(r1, _) => vec![r1],
+            JumpIfNotZero(r1, _) | SaveRetAddr(r1, _) | LoadLabel(r1, _) => vec![r1],
         })
         .into_iter()
         .collect()
@@ -1620,14 +1603,13 @@ impl<Reg: Clone + Eq + Ord + Hash> ControlFlowOp<Reg> {
             | Comment
             | Jump(_)
             | Call(_)
-            | MoveAddress(..)
+            | SaveRetAddr(..)
             | DataSectionOffsetPlaceholder
             | LoadLabel(..)
             | PushAll(_)
             | PopAll(_) => vec![],
 
             JumpIfNotZero(r1, _) => vec![r1],
-            JumpIfNotEq(r1, r2, _) => vec![r1, r2],
         })
         .into_iter()
         .collect()
@@ -1636,12 +1618,11 @@ impl<Reg: Clone + Eq + Ord + Hash> ControlFlowOp<Reg> {
     pub(crate) fn def_registers(&self) -> BTreeSet<&Reg> {
         use ControlFlowOp::*;
         (match self {
-            MoveAddress(reg, _) | LoadLabel(reg, _) => vec![reg],
+            SaveRetAddr(reg, _) | LoadLabel(reg, _) => vec![reg],
 
             Label(_)
             | Comment
             | Jump(_)
-            | JumpIfNotEq(..)
             | JumpIfNotZero(..)
             | Call(_)
             | DataSectionOffsetPlaceholder
@@ -1665,9 +1646,8 @@ impl<Reg: Clone + Eq + Ord + Hash> ControlFlowOp<Reg> {
             | PushAll(_)
             | PopAll(_) => self.clone(),
 
-            JumpIfNotEq(r1, r2, label) => Self::JumpIfNotEq(update_reg(r1), update_reg(r2), *label),
             JumpIfNotZero(r1, label) => Self::JumpIfNotZero(update_reg(r1), *label),
-            MoveAddress(r1, label) => Self::MoveAddress(update_reg(r1), *label),
+            SaveRetAddr(r1, label) => Self::SaveRetAddr(update_reg(r1), *label),
             LoadLabel(r1, label) => Self::LoadLabel(update_reg(r1), *label),
         }
     }
@@ -1690,13 +1670,13 @@ impl<Reg: Clone + Eq + Ord + Hash> ControlFlowOp<Reg> {
             Label(_)
             | Comment
             | Call(_)
-            | MoveAddress(..)
+            | SaveRetAddr(..)
             | DataSectionOffsetPlaceholder
             | LoadLabel(..)
             | PushAll(_)
             | PopAll(_) => (),
 
-            Jump(jump_label) | JumpIfNotEq(_, _, jump_label) | JumpIfNotZero(_, jump_label) => {
+            Jump(jump_label) | JumpIfNotZero(_, jump_label) => {
                 next_ops.push(label_to_index[jump_label]);
             }
         };
@@ -1751,9 +1731,8 @@ impl ControlFlowOp<VirtualRegister> {
             PushAll(label) => PushAll(*label),
             PopAll(label) => PopAll(*label),
 
-            JumpIfNotEq(r1, r2, label) => JumpIfNotEq(map_reg(r1), map_reg(r2), *label),
             JumpIfNotZero(r1, label) => JumpIfNotZero(map_reg(r1), *label),
-            MoveAddress(r1, label) => MoveAddress(map_reg(r1), *label),
+            SaveRetAddr(r1, label) => SaveRetAddr(map_reg(r1), *label),
             LoadLabel(r1, label) => LoadLabel(map_reg(r1), *label),
         }
     }

diff --git a/sway-core/src/asm_lang/virtual_immediate.rs b/sway-core/src/asm_lang/virtual_immediate.rs
@@ -25,6 +25,17 @@ impl VirtualImmediate06 {
             })
         }
     }
+
+    /// This method should only be used if the size of the raw value has already been manually
+    /// checked.
+    /// This is valuable when you don't necessarily have exact [Span] info and want to handle the
+    /// error at a higher level, probably via an internal compiler error or similar.
+    /// A panic message is still required, just in case the programmer has made an error.
+    pub(crate) fn new_unchecked(raw: u64, msg: impl Into<String>) -> Self {
+        Self {
+            value: raw.try_into().unwrap_or_else(|_| panic!("{}", msg.into())),
+        }
+    }
 }
 impl fmt::Display for VirtualImmediate06 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/chained_if_let/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/chained_if_let/src/main.sw
@@ -5,10 +5,7 @@ enum Result<T, E> {
   Err: E,
 }
 
-// should return 5
-fn main() -> u64 {
-  let result_a = Result::Ok::<u64, bool>(5u64);
-  let result_b = Result::Err::<u64, bool>(false);
+fn foo(result_a: Result<u64, bool>, result_b: Result<u64, bool>) -> u64 {
 
   if let Result::Err(_a) = result_a {
     6
@@ -20,3 +17,10 @@ fn main() -> u64 {
     42 
   }
 }
+
+// should return 5
+fn main() -> u64 {
+  let result_a = Result::Ok::<u64, bool>(5u64);
+  let result_b = Result::Err::<u64, bool>(false);
+  foo(result_a, result_b)
+}
diff --git a/.../e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle.json b/.../e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle.json
@@ -7,7 +7,7 @@
         "typeArguments": null
       },
       "name": "C0",
-      "offset": 3876
+      "offset": 4212
     },
     {
       "configurableType": {
@@ -16,7 +16,7 @@
         "typeArguments": null
       },
       "name": "C1",
-      "offset": 3884
+      "offset": 4220
     },
     {
       "configurableType": {
@@ -25,7 +25,7 @@
         "typeArguments": null
       },
       "name": "C2",
-      "offset": 3900
+      "offset": 4236
     },
     {
       "configurableType": {
@@ -34,7 +34,7 @@
         "typeArguments": []
       },
       "name": "C3",
-      "offset": 3932
+      "offset": 4268
     },
     {
       "configurableType": {
@@ -43,7 +43,7 @@
         "typeArguments": []
       },
       "name": "C4",
-      "offset": 3948
+      "offset": 4284
     },
     {
       "configurableType": {
@@ -52,7 +52,7 @@
         "typeArguments": []
       },
       "name": "C5",
-      "offset": 3964
+      "offset": 4300
     },
     {
       "configurableType": {
@@ -61,7 +61,7 @@
         "typeArguments": null
       },
       "name": "C6",
-      "offset": 3980
+      "offset": 4316
     },
     {
       "configurableType": {
@@ -70,7 +70,7 @@
         "typeArguments": null
       },
       "name": "C7",
-      "offset": 3996
+      "offset": 4332
     },
     {
       "configurableType": {
@@ -79,7 +79,7 @@
         "typeArguments": null
       },
       "name": "C9",
-      "offset": 4044
+      "offset": 4380
     }
   ],
   "functions": [

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/far_jumps/many_blobs/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/language/far_jumps/many_blobs/Forc.lock
@@ -1,3 +1,8 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-CA7273114FB9B4E5'
+
 [[package]]
 name = 'many_blobs'
 source = 'member'
+dependencies = ['core']
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/far_jumps/many_blobs/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/far_jumps/many_blobs/Forc.toml
@@ -6,3 +6,4 @@ name = "many_blobs"
 implicit-std = false
 
 [dependencies]
+core = { path = "../../../../../../../../sway-lib-core" }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -6,3 +6,4 @@ name = "many_blobs"
		implicit-std = false

		[dependencies]
		core = { path = "../../../../../../../../sway-lib-core" }