You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, managed ilasm builds a parse tree for the whole input .il file. This causes massive memory allocations (and OOMs on x86) for some of our larger test assemblies (HardwareIntrinsics AVX512 and HugeArray) that are on the scale of CoreLib. It also causes a massive slowdown due to the allocations.
This PR converts our parse tree visitor to instead be called as separate "actions" during grammar parsing, similar to how native ilasm does in the YACC file. Unlike native ilasm, all of the real logic is in C# files, and the logic in the CIL.g4 file is minimal (function calls into C#).
Because ANTLR4 only emits public types, we limit the exposed public API surface for (expected) internal consumers of the ILAssembler library (such as Roslyn for their test tree) to only the expected public API surface.
Performance
Measured Debug NativeAOT ilasm.exe builds from the visitor-based ilasm-more-tests base (89f324378bf3) and this PR's action branch (edca868dbae5) on the same Windows machine. Each input was produced by running checked ildasm -raweh -unicode over an existing CoreCLR test assembly, and each assembler invocation used -DET -DLL -DEBUG. Times and peak working sets are medians after warmup except for the single-run HugeField2 cases.
Architecture
Input
IL size
Runs
Visitor elapsed
Actions elapsed
Visitor peak working set
Actions peak working set
x64
async/simple-eh
54,886 B
15
359.7 ms
316.4 ms (-12.0%)
49.8 MiB
38.6 MiB (-22.5%)
x64
HugeArray
9,558,004 B
5
4,614.8 ms
3,243.3 ms (-29.7%)
508.2 MiB
95.3 MiB (-81.2%)
x64
HugeField2
99,120,704 B
1
122.272 s
114.357 s (-6.5%)
3,603.8 MiB
395.8 MiB (-89.0%)
x86
async/simple-eh
54,886 B
10
344.1 ms
301.7 ms (-12.3%)
45.1 MiB
37.1 MiB (-17.7%)
x86
HugeArray
9,558,004 B
3
4,668.5 ms
3,158.8 ms (-32.3%)
304.8 MiB
119.9 MiB (-60.7%)
x86
HugeField2
99,120,704 B
1
OOM after 51.463 s
106.208 s (success)
1,058.0 MiB before failure
372.5 MiB
Peak working set was sampled every 100 ms. The x86 HugeField2 elapsed times are not directly comparable because the visitor process terminated with Insufficient memory to continue the execution of the program, while the actions process completed successfully.
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors the managed ILAssembler pipeline to avoid building a full ANTLR parse tree by switching to action-driven parsing (with an unbuffered token stream), and introduces an explicit reference-assembly “contract” to constrain the intended public API surface while keeping ANTLR-generated types out of the supported contract.
Changes:
Switch DocumentCompiler to parse with UnbufferedTokenStream, disable parse-tree construction, and drive compilation through GrammarActions.
Add a custom reference-assembly project under src/tools/ilasm/src/ILAssembler/ref/ and adjust tests to compile against implementation where needed.
Add extensive regression tests for error-tolerant parsing and state-leak prevention across documents/scopes.
Reviewed changes
Copilot reviewed 83 out of 88 changed files in this pull request and generated 2 comments.
ParserErrorListener uses TokenSource.SourceName to look up SourceText in loadedDocuments. For synthetic/missing tokens created during error recovery, TokenSource may be PreprocessedTokenSource, whose SourceName can include include-chain decorations (and may not match the raw document keys). Using the underlying InputStream.SourceName aligns with Location.From(...) and should make document lookup robust for recovery tokens.
Remove the temporary managed-ilasm incompatibility markers now that the GrammarActions layer addresses the practical CI limits for these merged runners.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
Remove the temporary OOM-based managed-ilasm exclusions from the Directed runners, AVX512F tests, and HugeArray1 now that the GrammarActions layer avoids those memory failures.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
Remove the temporary memory exclusion in the GrammarActions layer, which avoids the bottom-layer managed ilasm memory limit.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
Port the bottom-layer encoder simplification to GrammarActions now that method bodies conservatively preserve InitLocals.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
The GrammarActions parser round-trips HugeField2 and hugeSimpleExpr1 within practical CI memory limits, so keep both covered on the action layer.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
_currentLanguageGuid is mutable parser-wide state, but PDB metadata is built only after all documents have been parsed. If two source documents use different valid .language directives, every newly created Document row receives the language from the last directive rather than the language active when that document's sequence points were recorded. Capture the language with each method/document's debug information and use that value when creating the PDB document handle.
Capture the active language with each method's debug document so multi-document compilations emit the correct portable PDB language metadata.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
The generated truefalse rule uses RecoverInline and then invokes this @after conversion even when recovery deleted an unexpected token. In a malformed initializer such as bool(bad true), token.Text can still be the offending text, so bool.Parse throws FormatException instead of allowing error-tolerant compilation to report a parser diagnostic and continue. Use a non-throwing conversion with a safe fallback here (and apply the same guard to the other single-token @after converters).\n\n> [!NOTE] This review comment was generated by GitHub Copilot.
Prevent secAction conversion from throwing after recovery
secAction is another single-token rule whose generated @after action runs after RecoverInline. If recovery skips an unexpected token before a valid action (for example foo demand ...), ParseSecurityAction receives foo and reaches this UnreachableException, so malformed input throws rather than producing the parser diagnostic. Return a safe fallback or otherwise guard this conversion on invalid token text, consistently with the boolean conversion.
Use neutral fallback values when single-token grammar actions receive an invalid token during ANTLR recovery, with direct and end-to-end regression coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 806a996a-9fcc-42b7-a902-cac954582b24
hexbyte accepts INT32, and the lexer allows 0x-prefixed INT32 values, but the fallback parses the original text with AllowHexSpecifier, which does not accept the 0x prefix. Therefore an accepted value such as 0x123 falls through to zero instead of the documented masked byte value 0x23; normalize the prefix (and the signed form) before applying the fallback.
Report diagnostics for unresolved custom-attribute aliases
An unresolved custom-attribute alias returns null here without reporting DiagnosticIds.TypedefNotFound; every caller simply ignores the null result. Thus a syntactically valid use such as MissingAttributeAlias silently drops the attribute and produces no diagnostic, unlike the method, field, and type typedef paths. Pass the source location through this path and report the missing alias before returning.
Resolve typedef aliases in structured custom-attribute arguments
This branch reports TypedefNotFound for every TypedefSerializationTypeValue without consulting the typedef table. The grammar explicitly accepts aliases in serializTypeElement, so a declared alias such as .typedef int32 as MyInt cannot be used in a structured custom-attribute named argument: the emitted serialization-type blob is empty and compilation reports a false error. Resolve the alias (or reject this syntax in the grammar) and add a regression test.
Resolve typedef aliases used as native marshal types
The grammar accepts a dotted-name typedef as a native marshal type, but this materialization path unconditionally reports TypedefNotFound and emits an empty blob. Consequently, even a declared alias used as marshal(Alias) is unusable and can leave the field or parameter with an invalid marshalling descriptor. Either resolve the alias here or remove the accepted typedef form and report a deliberate unsupported-syntax diagnostic.
This switches the compiler to the new streaming/action path that is intended to eliminate the large allocation and slowdown described for the AVX512/HugeArray inputs, but the PR provides no before/after measurements. Please include representative peak memory and elapsed-time results (especially x86, where OOMs are reported) and confirm that the action path does not regress ordinary IL files before relying on this high-risk parser rewrite.
[!NOTE] This review comment was generated by GitHub Copilot.
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
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.
Currently, managed ilasm builds a parse tree for the whole input .il file. This causes massive memory allocations (and OOMs on x86) for some of our larger test assemblies (HardwareIntrinsics AVX512 and HugeArray) that are on the scale of CoreLib. It also causes a massive slowdown due to the allocations.
This PR converts our parse tree visitor to instead be called as separate "actions" during grammar parsing, similar to how native ilasm does in the YACC file. Unlike native ilasm, all of the real logic is in C# files, and the logic in the CIL.g4 file is minimal (function calls into C#).
Because ANTLR4 only emits public types, we limit the exposed public API surface for (expected) internal consumers of the ILAssembler library (such as Roslyn for their test tree) to only the expected public API surface.
Performance
Measured Debug NativeAOT
ilasm.exebuilds from the visitor-basedilasm-more-testsbase (89f324378bf3) and this PR's action branch (edca868dbae5) on the same Windows machine. Each input was produced by running checkedildasm -raweh -unicodeover an existing CoreCLR test assembly, and each assembler invocation used-DET -DLL -DEBUG. Times and peak working sets are medians after warmup except for the single-runHugeField2cases.async/simple-ehHugeArrayHugeField2async/simple-ehHugeArrayHugeField2Peak working set was sampled every 100 ms. The x86
HugeField2elapsed times are not directly comparable because the visitor process terminated withInsufficient memory to continue the execution of the program, while the actions process completed successfully.Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
The benchmark section was prepared with GitHub Copilot assistance.