refactor(codegen): de-globalize the LLVM stack-too-deep and option channels - #587
Draft
abinavpp wants to merge 2 commits into
Draft
refactor(codegen): de-globalize the LLVM stack-too-deep and option channels#587abinavpp wants to merge 2 commits into
abinavpp wants to merge 2 commits into
Conversation
…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).
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. |
Comment on lines
+33
to
39
| 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"); | ||
| } |
Comment on lines
+162
to
+166
| if let LLVMDiagnosticSeverity::LLVMDSError = severity { | ||
| let mut error = captured.error.borrow_mut(); | ||
| if error.is_none() { | ||
| *error = Some(message); | ||
| } |
Comment on lines
50
to
+56
| 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); | ||
| }); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NomicFoundation/solx-llvm#123