Skip to content

Conversation

@am11
Copy link
Member

@am11 am11 commented Jan 30, 2026

No description provided.

@am11 am11 requested a review from jkoritzinsky January 30, 2026 22:52
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 30, 2026
@am11 am11 added area-ILTools-coreclr community-contribution Indicates that the PR has been added by a community member and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 30, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

@am11
Copy link
Member Author

am11 commented Jan 30, 2026

Code coverage is pretty much the same:

Summary
  Generated on: 31.1.2026 - 0.52.46
  Coverage date: 30.1.2026 - 20.16.29 - 31.1.2026 - 0.51.53
  Parser: MultiReport (2x Cobertura)
  Assemblies: 1
  Classes: 25
  Files: 24
  Line coverage: 44.4%
  Covered lines: 10510
  Uncovered lines: 13150
  Coverable lines: 23660
  Total lines: 33971
  Branch coverage: 35.9% (2228 of 6205)
  Covered branches: 2228
  Total branches: 6205
  Method coverage: 26.6% (870 of 3268)
  Full method coverage: 19.2% (629 of 3268)
  Covered methods: 870
  Fully covered methods: 629
  Total methods: 3268

ILAssembler                                  44.4%
  ILAssembler.BlobBuilderExtensions          12.1%
  ILAssembler.CILBaseListener                   0%
  ILAssembler.CILBaseVisitor<T>                 0%
  ILAssembler.CILLexer                       99.5%
  ILAssembler.CILParser                      35.5%
  ILAssembler.Diagnostic                      100%
  ILAssembler.DocumentCompiler               71.4%
  ILAssembler.EntityRegistry                 64.6%
  ILAssembler.GrammarResult                  72.7%
  ILAssembler.GrammarVisitor                 47.9%
  ILAssembler.InstructionEncoderExtensions    100%
  ILAssembler.Location                        100%
  ILAssembler.MetadataExtensions               30%
  ILAssembler.NamedElementList<T>            20.9%
  ILAssembler.NameHelpers                    78.5%
  ILAssembler.Options                        81.8%
  ILAssembler.PreprocessedTokenSource        52.5%
  ILAssembler.SignatureArg                     10%
  ILAssembler.SourceSpan                      100%
  ILAssembler.SourceText                      100%
  ILAssembler.StackExtensions                 100%
  ILAssembler.StringHelpers                  61.9%
  ILAssembler.TypeName                        100%
  ILAssembler.VTableExportPEBuilder          65.7%
  ILAssembler.VTableFixupSupport              7.3%

With cli, we can perform ad-hoc testing and align the implementation with relative ease. Perhaps quicker way would be to get the fresh list of missing things / TODOs is by replacing native ilasm with managed one in src/tests (IlasmRoundTripBatchScript)?

@jkoritzinsky
Copy link
Member

I think we should definitely introduce a new test lane in the runtime-coreclr ilasm pipeline that replaces the native ilasm with the managed one. That will help us figure out what we need for parity (and that being green is our acceptance criteria for the new ilasm implementation).

Copy link
Member

Choose a reason for hiding this comment

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

We'd like to ship this as a NativeAOT'd tool eventually. Can we set <AotOrSingleFile> to true for this project?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can think of it separately; how we want to support community platforms (with --bootstrap): #123822 (comment).

@am11
Copy link
Member Author

am11 commented Jan 31, 2026

I think we should definitely introduce a new test lane in the runtime-coreclr ilasm pipeline that replaces the native ilasm with the managed one. That will help us figure out what we need for parity (and that being green is our acceptance criteria for the new ilasm implementation).

How about the following plan:

  • Land this PR.
  • Assuming we want parity with crossgen2/ilc (PublishAot, or PublishSingleFile for non-AOT platforms), we either move this next to cDAC or relocate the src/native/managed infrastructure into this directory.
  • Depending on the final form factor, copy the result into Core_Root with the appropriate layout.

After that, we can iterate with ongoing development and fixes. I ran some ad-hoc tests (/tmp/foo.il) using both the native and managed ilasm implementations. Error reporting differs slightly with the managed version (and arguably is better); we can align details like this over time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ILTools-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants