Skip to content

Conversation

@Dentosal
Copy link
Member

Closes #857. See the spec PR for full description: FuelLabs/fuel-specs#630.

Adds JAL $rA $rB imm instruction that sets $ra = $pc + 4 and then jump setting $pc = $rB + imm * 4. Allows $rA to be $zero in which case the value is discarded instead.

Open questions:

  • Do we want to do overflow checking when setting $rA? Since we're always within valid memory before the jump, worst that can happen that the panic is delayed until trying to use that address.
  • Do we want to support discard-writing in other opcodes as well? Seems like it's rarely useful in other cases.

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog: none!
  • New behavior is reflected in tests: yep!
  • If performance characteristic of an instruction change, update gas costs as well or make a follow-up PR for that: using costs of the jmp opcode for this, should be about the same
  • The specification matches the implemented behavior: Jump-and-link instruction fuel-specs#630

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

@Dentosal Dentosal self-assigned this Mar 14, 2025
@Dentosal Dentosal marked this pull request as ready for review April 7, 2025 09:58
netrome
netrome previously approved these changes Apr 10, 2025
Copy link
Contributor

@netrome netrome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reviewed by someone who understands the VM better than I, but I don't spot any obvious errors at least.

xgreenx
xgreenx previously approved these changes Apr 22, 2025
assert_success(&receipts);

if let Some(Receipt::Log { ra, .. }) = receipts.first() {
assert_eq!(*ra, skip, "Expected correct number of skipped instructions");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't ra be 2 here since we skipped 3 instructions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. We compute 5-1-1 = 3. If that feels confusing, the test could be rewritten to use addi instead. Let me know if you'd prefer that.

}

#[test]
fn jump_and_link__recursive_fibonacci() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will skip this function from the review=D

Looks like it works, so very cool that you managed to write it with just instructions=)

@Dentosal Dentosal dismissed stale reviews from xgreenx and netrome via 63fc3c0 April 28, 2025 13:41
@Dentosal Dentosal requested review from netrome and xgreenx April 28, 2025 14:44
Voxelot
Voxelot previously approved these changes May 12, 2025
@Voxelot
Copy link
Member

Voxelot commented May 12, 2025

Do we want to support discard-writing in other opcodes as well? Seems like it's rarely useful in other cases.

Maybe we should analyze some sway compiler output examples to see if there are many cases of registers being allocated for capturing results and never getting used afterwards? This should likely be done as a separate cleanup PR anyways.

@Dentosal
Copy link
Member Author

Maybe we should analyze some sway compiler output examples to see if there are many cases of registers being allocated for capturing results and never getting used afterwards? This should likely be done as a separate cleanup PR anyways.

I did quickly check some compiler output, and it seems either super rare or nonexisting. And yep definitely a separate PR in any case.

Voxelot
Voxelot previously approved these changes May 13, 2025
xgreenx
xgreenx previously approved these changes May 13, 2025
@Dentosal Dentosal dismissed stale reviews from xgreenx and Voxelot via 052874e May 13, 2025 14:39
@Dentosal Dentosal requested a review from xgreenx May 14, 2025 11:26
@xgreenx xgreenx added no changelog Skips the CI changelog check and removed no changelog Skips the CI changelog check labels May 14, 2025
@Dentosal Dentosal enabled auto-merge May 14, 2025 13:49
@Dentosal Dentosal added this pull request to the merge queue May 14, 2025
Merged via the queue into master with commit 2945b2e May 14, 2025
39 of 51 checks passed
@Dentosal Dentosal deleted the dento/fncall-jumps branch May 14, 2025 13:59
Dentosal added a commit to FuelLabs/fuel-specs that referenced this pull request Jul 28, 2025
Closes #627. VM issue: FuelLabs/fuel-vm#857.
VM PR: FuelLabs/fuel-vm#925.

The design is quite similar of the RISC-V of the same name. `JAL $ra $rb
imm` stores the address of the next instruction to `$ra`, so that
register can be used as a return address from the subroutine. If `ra` is
`$zero`, the value is discarded instead, so this can be used as a jump
without having to trash a register. After storing the return address, it
jumps to instruction at memory address `$rb + imm * 4`.

The main purpose of this instruction is efficient subroutine-calling and
returning. `JAL $ret_addr $subroutine_addr 0` is used to perform the
call, and `JAL $zero $ret_addr 0` returns from it. For nexted function
calls, the callee is responsible for storing the `$ret_addr`.

The following snippet shows a minimal program using the functionality:

```rust
// main function
jal $ret_addr $pc 2 // call subroutine
ret $zero // end program

// subroutine
/* subroutine body comes here */
jal $zero $ret_addr 0 // Return from the subroutine
```

### Fibonacci example

To show off how compact code this makes, I wrote a small fibonacci
function using it. The function here uses the following register-based
ABI:
* Function argument and return value `$fnarg` in `0x10` 
* Function return address `$return_addr` in `0x11` 

Also the code uses the following locals: `$local1: 0x12, $local2: 0x13,
$local3: 0x14` (named for `pshl/popl`)

```rust
// Set argument
movi $fnarg 10 // <- this computes fibo(10), i.e. 10th fibonacci number, 55

// Main function
jal $return_addr $pc 3 // <- offset to the subroutine
log $fnarg $zero $zero $zero
ret $one

// Fibonacci subroutine
// fibo(0) = 0, fibo(1) = 1, fibo(n) = fibo(n-1) + fibo(n-2)
pshl 0b11110 // Save return_address and local{1,2,3}
// Compute fn pointer to the current function and place it in local3
subi $local3 $pc 4 // <- subtract 4 to get prev instruction start
// If n < 2 no computation needed
movi $local1 2
lt $local1 $fnarg $local1
jnzf $local1 $zero 8 // Skip over computation
// Else call self with n - 1 and n - 2 and sum those
subi $local2 $fnarg 2         // Save n - 2 to local2
subi $fnarg $fnarg 1          // n -= 1
jal $return_addr $local3 0    // Call self
move $local1 $fnarg           // Copy result to local1
move $fnarg $local2           // Restore n - 2 from local2
jal $return_addr $local3 0    // Call self
move $local2 $fnarg           // Copy result to local2
add $fnarg $local1 $local2 // result = local1 + local2
// Computation ends here this is where jnzf jumps to
popl 0b11110 // Restore return_address and local{1,2,3}
jal $zero $return_addr 0 // Return from subroutine
```

### Before requesting review
- [x] I have reviewed the changes myself
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New function call/return helper opcodes

5 participants