Skip to content

Optimise get_config for trivial types - #7682

Draft
xunilrj wants to merge 8 commits into
masterfrom
xunilrj/opt-get-config
Draft

Optimise get_config for trivial types#7682
xunilrj wants to merge 8 commits into
masterfrom
xunilrj/opt-get-config

Conversation

@xunilrj

@xunilrj xunilrj commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Description

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@xunilrj
xunilrj temporarily deployed to fuel-sway-bot July 14, 2026 13:29 — with GitHub Actions Inactive
@codspeed-hq

codspeed-hq Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.88%

⚡ 1 improved benchmark
✅ 24 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
tokens_for_program 655 µs 590.8 µs +10.88%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing xunilrj/opt-get-config (e3c79a8) with master (5184c93)

Open in CodSpeed

@xunilrj

xunilrj commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

I think I found the reason for the regressions. This is an example from o2.

get_config before this optimization generates just one ADDI, as apps normally do not have enough configurables to overflow ADDI immediate argument. With the new optimization, on the other hand, apps normally DO have enough bytes in their data section to overflow it, so we end-up with two instructions: MOVI and ADD.

BEFORE

addi $r0 $ssp i1              ; get address of configurable BASE_ASSET

0x00001808 ADDI R52 $ssp 0x1                               ;; [80, 208, 64, 1]

AFTER

addr $r0 data_Configurable_1  ; get address of configurable BASE_ASSET

0x00001630 MOVI R52 0xd30                                  ;; [114, 208, 13, 48]
0x00001634 ADD R52 R52 R63                                 ;; [16, 211, 79, 192]

The regression then occurs when the number of access compensates the gain of removing the decode.

@ironcev

ironcev commented Jul 15, 2026

Copy link
Copy Markdown
Member

@xunilrj Regarding the large data section, it is likely we can improve there in general. E.g., I've noticed cases where we have a large number of u64s in the data section: #7612

@xunilrj
xunilrj force-pushed the xunilrj/opt-get-config branch from a60b3ba to e8f3a2a Compare July 20, 2026 13:27
@xunilrj
xunilrj temporarily deployed to fuel-sway-bot July 20, 2026 13:27 — with GitHub Actions Inactive
@xunilrj
xunilrj temporarily deployed to fuel-sway-bot July 20, 2026 13:59 — with GitHub Actions Inactive
@xunilrj

xunilrj commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author
order-book binary size:
Before: 124.56 KB
After: 124.12 KB (0.3% better)

fuel-o2-exports

Improvements Regressions
Count 32 1
Average 0.61% -0.04%
Median 0.64% -0.04%
Max 1.35% -0.04%
Min 0.04% -0.04%

ironcev added a commit that referenced this pull request Jul 23, 2026
## Description

Whilst trying to improve #7682, I
came across the following bug:

SROA was generating invalid IR when an aggregated had `load`s across
multiple blocks. When generating the scalar accesses, the older
algorithm was gathering only the "last" block that had access, and
incorrectly generating `load`s pointing to this last block, even when
the `load` was from a previous block. A "use-before-def" problem. (see
sway-ir/tests/sroa/cross_block_gep_reuse.ir).

To verify this issue this PR also creates an "SSA dominance check". We
check if all "uses" are dominated by all its "defs". But this check is
expensive, so, for the moment, this check is opt-in. Below we have some
timings to justify that:

```
dominance check off:
> hyperfine "cargo r -p forc -r -- build --path fuel-o2-exports/contracts/order-book --release"
Benchmark 1: cargo r -p forc -r -- build --path fuel-o2-exports/contracts/order-book --release
  Time (mean ± σ):     11.213 s ±  0.100 s    [User: 8.347 s, System: 1.095 s]
  Range (min … max):   11.105 s … 11.383 s    10 runs

dominance check on:
> SWAY_FORCE_VERIFY_IR=true hyperfine "cargo r -p forc -r -- build --path fuel-o2-exports/contracts/order-book --release"
Benchmark 1: cargo r -p forc -r -- build --path fuel-o2-exports/contracts/order-book --release
  Time (mean ± σ):     16.663 s ±  0.577 s    [User: 13.563 s, System: 1.121 s]
  Range (min … max):   16.358 s … 18.271 s    10 runs
```

This PR also removes `DCE` and `MEM2REG` passes. from the SROA test.
They were there to facilitate `filecheck` directives. As we do not use
them anymore, seeing the diff as it is, is actually better.

## Checklist

- [ ] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Igor Rončević <ironcev@hotmail.com>
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.

2 participants