Repository navigation
Optimise get_config for trivial types - #7682
Conversation
|
I think I found the reason for the regressions. This is an example from
BEFORE AFTER The regression then occurs when the number of access compensates the gain of removing the decode. |
a60b3ba to
e8f3a2a
Compare
fuel-o2-exports
|
## 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>
e3c79a8 to
98d47fd
Compare
98d47fd to
3f1dd93
Compare
|
order book
"single_level/total_gas",1588956,1590246,-0.08 trade account
|
3f1dd93 to
2245cc4
Compare
|
I had to remove the idea of having "$cs" for now. It was causing some annoying regressions exactly in hpt paths. The issue is that we were decreasing the allocation pool, and one of the affected methods was exactly in a important method inside the order-book contract. Nor order-book, nor trade-account contracts have trivial configurables after the limit of 12 bit offsets. We can try to improve this case later in multiple ways without reserving a new register. fuel-o2-exports / order-book
Distributionfuel-o2-exports / trade-account
Distribution |
2245cc4 to
97a6715
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
97a6715 to
ebca105
Compare
|
👍 |
Description
This PR optimize access to trivially decodable configurables.
We know that decode for these configurables is essentially a
noop. So now we do not run their decode and simply point to the configurable section directly.Checklist
Breaking*orNew Featurelabels where relevant.