Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ASM printing CLI in forc #5997

Merged
merged 2 commits into from
May 13, 2024
Merged

Conversation

ironcev
Copy link
Member

@ironcev ironcev commented May 13, 2024

Description

This PR simplifies and gives more control ove printing the ASM. It's a first step in harmonizing printing options for ASM and IR, where the goal is to be able to print arbitrary optimization passes in IR.

This PR:

  • replaces two existing, verbose and unintuitive to discover, ASM printing CLI options with a single --asm, same as the single --ir option.
  • improves printed ASM by removing the confusing empty programs printed for every external library dependency.
  • harmonizes printed virtual and allocated abstract ASM to have the same-looking comments and structure.

Additionally, the PR fixes the bug in the PassManager::insert_after_each helper function, where passes were not inserted within groups. This is a preparation step for the changes needed for improving the --ir CLI option.

Demo

--asm all               'Prints virtual, allocated, and final ASM.
--asm final             'Prints only the final ASM.
--asm allocated final   'Prints the allocated and the final ASM.

Breaking Changes

CLI

Instead of existing --intermediate-asm and --finalized-asm options we now have a single --asm CLI option with parameters. The existing options should be replaced:

Current New
--intermediate-asm --asm abstract
--finalized-asm --asm final
--intermediate-asm --finalized-asm --asm all

Build Profile

Instead of existing print-intermediate-asm and print-finalized-asm options we now have a single print-asm CLI option with parameters. E.g.:

print-asm = { virtual = true, allocated = true, final = true }

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.

@ironcev ironcev added forc testing General testing breaking May cause existing user code to break. Requires a minor or major release. forc-pkg Everything related to the `forc-pkg` crate. labels May 13, 2024
@ironcev ironcev self-assigned this May 13, 2024
Copy link

Benchmark for 6a5bb45

Click to view benchmark
Test Base PR %
code_action 5.4±0.02ms 5.4±0.03ms 0.00%
code_lens 337.8±10.85ns 296.5±8.58ns -12.23%
compile 2.9±0.03s 2.9±0.06s 0.00%
completion 4.7±0.06ms 4.7±0.01ms 0.00%
did_change_with_caching 2.7±0.04s 2.7±0.03s 0.00%
document_symbol 992.9±39.44µs 987.7±31.78µs -0.52%
format 76.7±0.54ms 77.1±0.88ms +0.52%
goto_definition 358.5±7.29µs 362.9±12.52µs +1.23%
highlight 9.1±0.14ms 9.0±0.10ms -1.10%
hover 479.0±8.99µs 483.5±10.44µs +0.94%
idents_at_position 122.3±0.39µs 121.9±0.87µs -0.33%
inlay_hints 661.6±21.00µs 667.6±34.41µs +0.91%
on_enter 493.6±3.68ns 494.7±17.32ns +0.22%
parent_decl_at_position 3.8±0.10ms 3.7±0.06ms -2.63%
prepare_rename 358.0±2.64µs 362.9±3.89µs +1.37%
rename 9.7±0.16ms 9.6±0.15ms -1.03%
semantic_tokens 1026.2±17.73µs 1057.7±16.51µs +3.07%
token_at_position 424.3±2.52µs 357.1±1.83µs -15.84%
tokens_at_position 3.8±0.01ms 3.7±0.06ms -2.63%
tokens_for_file 422.3±3.66µs 420.2±2.34µs -0.50%
traverse 40.0±0.90ms 40.1±1.08ms +0.25%

@ironcev ironcev marked this pull request as ready for review May 13, 2024 12:18
@ironcev ironcev requested a review from kayagokalp as a code owner May 13, 2024 12:18
@ironcev ironcev requested review from a team May 13, 2024 12:19
@ironcev ironcev enabled auto-merge (squash) May 13, 2024 12:36
@ironcev ironcev requested a review from a team May 13, 2024 12:43
@ironcev ironcev merged commit fa62d82 into master May 13, 2024
42 checks passed
@ironcev ironcev deleted the ironcev/improve-asm-compiler-flag branch May 13, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking May cause existing user code to break. Requires a minor or major release. forc forc-pkg Everything related to the `forc-pkg` crate. testing General testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants