Skip to content

fix(evm): describe the spill region with module flags - #690

Merged
hedgar2017 merged 4 commits into
main-solcfrom
app-spill-region-flags-solc
Sep 29, 2026
Merged

hedgar2017 merged 4 commits into
main-solcfrom
app-spill-region-flags-solc

Conversation

@abinavpp

@abinavpp abinavpp commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Copy of #653 for main-solc.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Coverage Summary

Crate Line Coverage Function Coverage
solx 🟡 72.7% 🔴 40.0%
solx-benchmark-converter 🔴 0.0% 🔴 0.0%
solx-codegen-evm 🟡 61.5% 🟡 63.0%
solx-compiler-downloader 🔴 0.0% 🔴 0.0%
solx-core 🟡 50.3% 🟡 62.8%
solx-dev 🔴 2.4% 🔴 3.0%
solx-evm-assembly 🟡 50.9% 🟡 54.5%
solx-solc-test-adapter 🔴 1.7% 🔴 2.1%
solx-standard-json 🟡 53.9% 🟡 57.5%
solx-tester 🔴 37.9% 🔴 35.7%
solx-utils 🟡 61.6% 🟡 60.0%
solx-yul 🟡 54.0% 🟡 60.4%
Total 🔴 36.9% 🔴 36.9%

Codecov Report | HTML Report | Workflow Run

@hedgar2017 hedgar2017 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please see #653.

@abinavpp
abinavpp force-pushed the app-spill-region-flags-solc branch from 7ac7379 to ddee470 Compare September 25, 2026 14:34
@hedgar2017
hedgar2017 requested a balanced review from Copilot September 28, 2026 13:31

@hedgar2017 hedgar2017 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Will merge after bumping solx-llvm and inkwell.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The developer guide still documents the obsolete spill-region LLVM options as automatically configured.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Migrates EVM spill-region configuration from process-global LLVM options to per-module flags.

Changes:

  • Adds memory-guard and spill-size module flags.
  • Simplifies target-machine construction.
  • Updates the pinned inkwell revision.
File Description
solx-codegen-evm/​src/​target_machine.rs Removes spill-region CLI argument injection.
solx-codegen-evm/​src/​codegen/​context/​mod.rs Sets spill-region module flags before optimization.
Cargo.toml Updates the inkwell revision.
Cargo.lock Locks the updated inkwell dependencies.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread solx-codegen-evm/src/target_machine.rs
@hedgar2017
hedgar2017 added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main-solc with commit e49ec62 Sep 29, 2026
43 checks passed
@hedgar2017
hedgar2017 deleted the app-spill-region-flags-solc branch September 29, 2026 12:22
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.

3 participants