Skip to content

Chore: Eliminate debug/release IL baseline duality#19611

Open
T-Gro wants to merge 3 commits intomainfrom
infra/ci-debug-mode
Open

Chore: Eliminate debug/release IL baseline duality#19611
T-Gro wants to merge 3 commits intomainfrom
infra/ci-debug-mode

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented Apr 17, 2026 •
edited
Loading

Summary

Eliminates the meaningless debug/release distinction in IL test baselines by fixing the root cause: FSharp.Core was unoptimized in Debug builds, causing different IL when tests referenced it.

Changes

Root cause fix

  • FSharp.Core.fsproj: Added <Optimize>true</Optimize> (alongside existing <Tailcalls>true</Tailcalls>) so Debug FSharp.Core produces equivalent IL to Release.

Test infrastructure simplification

  • Compiler.fs: Removed baselineIsDebug, configTag, and TEST_BASELINE_CONFIG env var from IL baseline resolution. Fallback chain simplified from 4 paths to 2 (*.il.netcore.bsl then *.il.bsl or *.il.net472.bsl then *.il.bsl).
  • Inlining.fs: Removed #if Release/#else fork - NaN is now inlined in both configs.
  • StaticsInInterfaces.fs: Removed #if Release/#else fork - Failure is now inlined in both configs.
  • azure-pipelines-PR.yml: Reverted WindowsDebugBaselines CI job (no longer needed).

Baseline consolidation

Category Action Count
.release.bsl to generic .bsl Renamed 151
.debug.bsl Deleted 176
Total files removed 327

Intentionally kept separate

  • SurfaceArea baselines (FSharp.Core.SurfaceArea.*.{debug,release}.bsl): MethodHandleOf is public in Debug but internal in Release via #if DEBUG in FSharp.Core source. This is a genuine API surface difference.

Validation

  • EmittedIL tests: Release net10.0 (1104 passed), Release net472 (1083 passed), Debug net10.0 (1102 passed), Debug net472 (1081 passed)
  • SurfaceArea tests: All 4 combos (Release/Debug x net10.0/net472)

@T-Gro T-Gro requested a review from a team as a code owner April 17, 2026 22:02
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026 •
edited
Loading

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Apr 20, 2026
@T-Gro T-Gro force-pushed the infra/ci-debug-mode branch 3 times, most recently from dc36f56 to c0c214f Compare April 20, 2026 12:35
auduchinok
auduchinok reviewed Apr 20, 2026
View reviewed changes
Comment thread src/FSharp.Core/FSharp.Core.fsproj
<!-- .tail annotations always emitted for this binary, even in debug mode -->
<Tailcalls>true</Tailcalls>
<!-- Optimization always enabled for this binary, even in debug mode -->
<Optimize>true</Optimize>
Copy link
Copy Markdown
Member

@auduchinok auduchinok Apr 20, 2026

Choose a reason for hiding this comment

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

I tend to think this could make debugging FSharp.Core much more difficult. Maybe we could force the tests to use the release build of it instead?

Copy link
Copy Markdown
Member Author

@T-Gro T-Gro Apr 20, 2026

Choose a reason for hiding this comment

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

It would then force people to build it twice (and maybe even having to remember to run build script twice, which I wanted to avoid). The config of Fsharp.Core flows to all projects involved in the run.

I would say that changes affecting any form of IL layout outnumber the need for FSharp.Core debugging, so I would leave the default as is in this PR now.

Once the need comes, to avoid fiddling a property, we can do a new target for FSharpCoreDebugging perharps?

Copy link
Copy Markdown
Member

@baronfel baronfel Apr 20, 2026

Choose a reason for hiding this comment

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

If you wanted to force all ProjectReferences to FSharp.Core to use Release mode (to prevent double-builds, etc) you can do that via the SetConfiguration metadata on the ProjectReference:

<ProjectReference Include="../FSharp.Core/FSharp.Core.fsproj" SetConfiguration="Configuration=Release" />

You can do similar with TargetFramework, and any other special properties you may need. Doing this skips portions of the ProjectReference protocol entirely (because we no longer need to ask which config to use for this reference).

Copy link
Copy Markdown
Member Author

@T-Gro T-Gro Apr 20, 2026

Choose a reason for hiding this comment

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

Thanks @baronfel , this fits nicely - I think by default, I would want most stuff reference Optimized FSharp.Core, but I can introduce a new configuration (Since based on docs I cannot do SetCustomProperty=false) just for Fsharp.Core.UnitTests when those are ran in Debug.

@T-Gro T-Gro force-pushed the infra/ci-debug-mode branch from c0c214f to e506866 Compare April 20, 2026 14:19
T-Gro

This comment was marked as resolved.

Sign in to view

@T-Gro
Chore: Eliminate debug/release IL baseline duality
05f1130 
- Add <Optimize>true</Optimize> to FSharp.Core.fsproj so Debug and Release
  FSharp.Core produce equivalent IL, eliminating the root cause of
  debug/release baseline differences in EmittedIL tests.
- Remove debug/release config branching from Compiler.fs IL baseline
  resolution (baselineIsDebug, configTag, TEST_BASELINE_CONFIG).
- Consolidate 151 .release.bsl files into generic .bsl files.
- Delete 176 .debug.bsl files that are now redundant.
- Unify #if Release/#else IL forks in Inlining.fs and StaticsInInterfaces.fs
  test files (FSharp.Core inlining now identical in both configs).
- Revert WindowsDebugBaselines CI job from azure-pipelines-PR.yml.
- SurfaceArea baselines intentionally kept separate (MethodHandleOf is
  public in Debug, internal in Release via #if DEBUG in FSharp.Core).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the infra/ci-debug-mode branch from e506866 to 05f1130 Compare April 20, 2026 15:58
@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Apr 20, 2026
abonie and others added 2 commits April 20, 2026 22:03
@abonie
Merge branch 'main' into infra/ci-debug-mode
1de20f2
@T-Gro
Improve Optimize comment with debugging instructions
a1e618c 
Address review feedback by adding a note on how to disable optimization
for local FSharp.Core debugging (dotnet build -p:Optimize=false).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the infra/ci-debug-mode branch from 2a1b0f0 to a1e618c Compare April 21, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@baronfel baronfel baronfel left review comments

@abonie abonie abonie approved these changes

+1 more reviewer

@auduchinok auduchinok auduchinok left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

F# Compiler and Tooling
Status: In Progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@T-Gro @baronfel @auduchinok @abonie