refactor(codegen): de-globalize the LLVM stack-too-deep and option channels - #587
refactor(codegen): de-globalize the LLVM stack-too-deep and option channels#587abinavpp wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the EVM codegen pipeline to remove process-global “stack-too-deep”/option plumbing by capturing EVM backend diagnostics per LLVMContext and moving per-unit codegen parameters to module flags; additionally introduces an opt-in in-process compilation mode for the process pool.
Changes:
- Add per-LLVM-context diagnostic capture (
StackRegionOverflow) and map it toError::StackTooDeepviaanyhowdowncasting. - Replace per-unit LLVM CLI options for stack region/metadata sizing with LLVM module flags set during
Context::build. - Add
SOLX_IN_PROCESSmode to compile jobs on the dispatch thread (no worker subprocess), with a fatal error handler installed once.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| solx-core/src/process/pool.rs | Adds SOLX_IN_PROCESS mode and installs an LLVM fatal error handler for in-process compilation. |
| solx-core/src/process/child.rs | Removes legacy stack-too-deep handler/global fallback state; relies on typed diagnostics instead. |
| solx-core/src/error/mod.rs | Downcasts StackRegionOverflow to surface StackTooDeep instead of generic string errors. |
| solx-codegen-evm/src/target_machine.rs | Stops injecting per-unit stack/metadata sizing via LLVM options; still parses user LLVM options. |
| solx-codegen-evm/src/lib.rs | Exposes the new diagnostics type and module. |
| solx-codegen-evm/src/diagnostics.rs | Implements per-context LLVM diagnostic handler capturing overflow payloads and errors/warnings. |
| solx-codegen-evm/src/codegen/mod.rs | Removes the global IS_SIZE_FALLBACK flag. |
| solx-codegen-evm/src/codegen/context/mod.rs | Installs diagnostics capture, writes module flags, and checks diagnostics after emissions. |
| let mut arguments = Vec::with_capacity(1 + llvm_options.len()); | ||
| arguments.push(Self::TARGET.to_string()); | ||
| arguments.extend_from_slice(llvm_options); | ||
| if let Some(size) = optimizer_settings.spill_area_size { | ||
| arguments.push(format!( | ||
| "-evm-stack-region-offset={}", | ||
| crate::r#const::SOLC_USER_MEMORY_OFFSET | ||
| )); | ||
| arguments.push(format!("-evm-stack-region-size={size}")); | ||
| } | ||
| if let Some(size) = optimizer_settings.metadata_size { | ||
| arguments.push(format!("-evm-metadata-size={size}")); | ||
| if arguments.len() > 1 { | ||
| let arguments: Vec<&str> = arguments.iter().map(|argument| argument.as_str()).collect(); | ||
| inkwell::support::parse_command_line_options(arguments.as_slice(), "LLVM options"); | ||
| } |
| if let LLVMDiagnosticSeverity::LLVMDSError = severity { | ||
| let mut error = captured.error.borrow_mut(); | ||
| if error.is_none() { | ||
| *error = Some(message); | ||
| } |
| pub fn new(session: Session) -> anyhow::Result<Self> { | ||
| let in_process = std::env::var_os("SOLX_IN_PROCESS").is_some_and(|value| value != "0"); | ||
| if in_process { | ||
| FATAL_ERROR_HANDLER.call_once(|| unsafe { | ||
| inkwell::support::error_handling::install_fatal_error_handler(fatal_error_handler); | ||
| }); | ||
| } |
8d81599 to
ce7f8c9
Compare
Coverage Summary
|
…annels The exit()-based stack error handler and the per-unit LLVM option parsing are process-global, making process-per-unit isolation a correctness requirement rather than a performance choice. Stack-too-deep now arrives as a per-context diagnostic captured around emission, and per-unit codegen parameters ride the module as flags. Needs the paired solx-llvm branch (per-module flag reading, the diagnostic kind and its C API accessor).
59e16a8 to
dffd7e3
Compare
|
This PR carries a benchmark comparing multithreading against the multiprocess worker pool ( The input is generated: many tiny contracts, a few big ones (~3s of codegen each), each in a clean variant and a variant that triggers stack-too-deep, plus a mix of all four. ratio = multiprocess/multithreading (>1 = threads faster):
Parity everywhere except stack-too-deep on cheap contracts: multiprocess kills and respawns the worker on every overflow, and that cost scales with how expensive process spawning is on the platform (2.1x on macOS x86). Windows: multithreading is broken there (LLVM's PrettyStackTrace gets corrupted under concurrent compilation - needs its own investigation), so Windows keeps the multiprocess default; all tests are green with that arrangement. Not measured yet: real contracts (limited by current slang coverage), and the optimizations only multithreading can unlock later, like compiling each contract fully in memory instead of shipping it to the job as text (~6-12% of a big contract's compile time today) and in-memory stack-too-deep retries. There is no urgency to force the switch; the question is worth revisiting with real contracts and a working Windows build once slang matures. |
dffd7e3 to
a16eee1
Compare
NomicFoundation/solx-llvm#123