Skip to content

Optimizer: prefer no inlining in debug builds#19548

Open
auduchinok wants to merge 35 commits intodotnet:mainfrom
auduchinok:inlineNamedFunctions
Open

Optimizer: prefer no inlining in debug builds#19548
auduchinok wants to merge 35 commits intodotnet:mainfrom
auduchinok:inlineNamedFunctions

Conversation

@auduchinok
Copy link
Copy Markdown
Member

@auduchinok auduchinok commented Apr 3, 2026

inline functions are fully inlined on the call sites by F# compiler. This means the source information and actual calls are lost, so it's very hard to reliably debug code using such functions.

This PR prevents inlining inline functions in debug builds. This improves debugging experience by allowing setting breakpoints inside such functions and allowing stepping into them and checking the function arguments. It also allows IDEs to analyze the produced IL more reliably during debug.

The implementation handles two distinct cases. When possible, a normal call to the existing method is emitted. When a function contains SRTPs and no callable method is produced, a specialized method is created for each type instantiation.

An example inline function that can be called directly:

let inline add a =
    a + 1

[<EntryPoint>]
let main _ =
    let i = add 1
    0
Screenshot 2026-04-03 at 21 18 51

The existing method is called:
Screenshot 2026-04-03 at 21 18 43

An inline function with SRTPs:

let inline add a b =
    a + b

[<EntryPoint>]
let main _ =
    let i = add 1 2
    0
Screenshot 2026-04-03 at 20 57 08

A specialized method is produced and called:
Screenshot 2026-04-03 at 20 57 22

Open questions:

  • Should we annotate specialized functions from other assemblies somehow? DebuggerNonUserCode?
  • Should we whitelist something from FSharp.Core, like built-in operators?

Implements fsharp/fslang-suggestions#824 for debug builds.
Fixes #9555.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 3, 2026

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@T-Gro
Copy link
Copy Markdown
Member

T-Gro commented Apr 9, 2026

re: Debugging

I would rely on people choosing to do "Just My Code" or not, in general.
For existing FSharp.Core functions, this might be indeed way too intrusive - are you thinking about handrolling the attributes, or a more general treatment when compiling fslib?

I would assume most of the "ugly to debug" fslib code uses statically optimized switches and is forced to be inlined even after this feature?

@auduchinok
Copy link
Copy Markdown
Member Author

auduchinok commented Apr 9, 2026

I would rely on people choosing to do "Just My Code" or not, in general.

Yes, IDE settings is going to be the primary way to control it. The problem is when an SRTP function from another assembly gets a specialized definition, the resulting method becomes a part of the user assembly. That's the reason I'm considering adding an attribute like DebuggerNonUserCode or another way to differentiate.

For existing FSharp.Core functions, this might be indeed way too intrusive - are you thinking about handrolling the attributes, or a more general treatment when compiling fslib?

I'm considering keeping inlining for functions in these modules:

  • Microsoft.FSharp.Core.LanguagePrimitives.IntrinsicOperators
  • Microsoft.FSharp.Core.Operators
  • Microsoft.FSharp.Core.Operators.Checked
  • Microsoft.FSharp.Core.Operators.OperatorIntrinsics
  • Microsoft.FSharp.NativeInterop.NativePtr

That would allow to workaround the issue with NoDynamicInvocation and would also hide the most of the basic functions, like not or +, that the user is likely not going to step into anyway.

@auduchinok
Copy link
Copy Markdown
Member Author

auduchinok commented Apr 9, 2026

I've checked all NoDynamicInvocation usages on GitHub and the only ones that are problematic (i.e. hidden by the signature) are the ones in the FSharp.Core modules above, so I think it's safe enough to to whitelist these modules + check the attribute.

@T-Gro
Copy link
Copy Markdown
Member

T-Gro commented Apr 9, 2026

Ok, how do you want to whitelist those?
A module-level attribute ([<InlineEvenDebug>] like), or listing them in the compiler by name?

I agree with the selection, there is nothing surprising inside of those module's functions.

For the specialized SRTP definition, wouldn't [<CompilerGenerated>] work here?

@auduchinok
Copy link
Copy Markdown
Member Author

Ok, how do you want to whitelist those?
A module-level attribute ([] like), or listing them in the compiler by name?

I'm going to try the second approach first, so it could work with older FSharp.Core too.

@auduchinok auduchinok force-pushed the inlineNamedFunctions branch 14 times, most recently from aefc4ce to 11f5c62 Compare April 15, 2026 15:23
@auduchinok auduchinok force-pushed the inlineNamedFunctions branch 3 times, most recently from 5dd26ae to a8c0602 Compare April 18, 2026 09:41
T-Gro

This comment was marked as off-topic.

@auduchinok
Copy link
Copy Markdown
Member Author

auduchinok commented May 7, 2026

Finding 1: InlineIfLambda disabled in debug mode — user-defined CEs affected

It's done on purpose, since stepping through the code reliably seems more important in debug builds.

Finding 2: StateMachineHelpers module missing from force-inline list

It's done on purpose. If there're issues this can be changed in a future PR.

Finding 3: Specialization cache has unbounded growth potential

That's a good one. Is there some existing approach in the compiler that could be used here?

Finding 4: Deleted StateMachineTests test needs replacement

Thanks! This was deleted accidentally. I've checked it locally, and the tests pass. I'll rebase the branch after #19676 is merged and will put these tests back.

Copy link
Copy Markdown
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

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

Review: Optimizer — prefer no inlining in debug builds

This is a well-structured PR with excellent test coverage (~40 new test cases). The goal of making inline functions debuggable is highly valuable. However, there are several issues that should be addressed before merge.


🔴 CRITICAL: State Machine Compilation Regression in Debug Builds

The PR disables InlineIfLambda inlining in debug mode (inlineIfLambda && cenv.settings.alwaysInline). This breaks state machine compilation for task { }, async { }, and all user-defined computation expressions in debug builds.

Evidence:

  • Removed test: StateMachineTests.fs deletes "Debug-mode: mixing resumable and standard computation expressions compiles"
  • tests/fsharp/tests.fs: asyncStackTraces and state-machines-non-optimized tests changed from --optimize- to --optimize+ — these were specifically testing non-optimized behavior
  • TaskGeneratedCode baselines now show callvirt ... TaskBuilder::Run<int32> instead of the state machine MoveNext method

Root cause: Task/async builder methods (TaskBuilder.Run, TaskBuilder.Bind, etc.) are inline with InlineIfLambda parameters but:

  • They're NOT in fslibForceInlineModules (only operators and intrinsics are)
  • They don't have NoDynamicInvocation
  • Therefore shouldForceInlineInDebug returns false
  • Without inlining, if __useResumableCode then evaluates to false at runtime, falling through to the dynamic path

Impact: Every F# user's debug build would:

  • Allocate closures and heap objects where state machines previously avoided them
  • Produce different async stack traces
  • Have degraded debug-mode performance for task { } and async { }

Recommendation: At minimum, shouldForceInlineInDebug should also force-inline vals whose return type involves ResumableCode<_,_> or recognize InlineIfLambda-bearing builder methods. Alternatively, add the FSharp.Core task builder modules to fslibForceInlineModules.


🟠 HIGH: ValInline.InlinedDefinition Uses the Zero Bit Pattern

InlinedDefinition maps to bit pattern 0b00 in ValFlags. Previously, 0b00 was unused (the InlineInfo getter would failwith "unreachable" for it). Zero is the default value for uninitialized data — if any code path creates ValFlags with default zero bits in those positions, the inline info will silently read as InlinedDefinition rather than being caught by an assertion.

While PickledBits correctly remaps it to Always before serialization, the zero-default concern remains for in-memory paths.

Recommendation: Use a different representation (e.g., a separate flag bit) or ensure the zero value remains unused.


🟠 HIGH: taccessPublic on Specialized Debug Vals

sharp let debugVal = Construct.NewVal(debugValName, m, None, debugValTy, Immutable, true, valReprInfo, taccessPublic, ...)

All specialized debug vals are created with taccessPublic regardless of the original function's accessibility. An internal inline function produces a public debug specialization. While these are local let bindings, IlxGen may emit them as public static methods.

Recommendation: Propagate the original val's accessibility: vref.Accessibility.


🟡 MEDIUM: No LanguageFeature Guard for Default-On Behavior Change

This changes default behavior for every Debug build (debug info on + optimizations off = standard dotnet build Debug). There's no gradual opt-in path — projects upgrading to this compiler silently get different debug-build behavior. Consider shipping behind a LanguageFeature flag (off-by-default in preview) initially, or at minimum require explicit MSBuild opt-in for the first release.


🟡 MEDIUM: CheckInlineValueIsComplete Disabled in Debug

When alwaysInline=false, incomplete inline values don't produce errors. A consumer compiling in release mode against a debug-compiled library might encounter errors that weren't reported when the library was built, creating confusing cross-compilation-mode failures.


🟡 MEDIUM: Closure Name Range Manipulation

The closure name collision fix creates synthetic ranges for cross-file copied expressions:
sharp Range.mkFileIndexRange eenv.cloc.Range.FileIndex expr.Range.Start expr.Range.End
This could confuse PDB/debugging (wrong file attribution). Consider using a counter that's keyed by the enclosing type instead.


✅ Positive Observations

  1. Excellent test coverage with ~40 cases covering direct calls, SRTP specialization, cross-assembly, byrefs, accessibility, members, operators, recursive functions
  2. Sound recursion prevention via dontInline map for both concrete and non-concrete type specializations
  3. Correct byref handling — falls back when closure path can't handle byrefs
  4. Cross-assembly design is correct — release consumers of debug-compiled libraries inline normally
  5. Binary compatibility maintained via PickledBits remapping

Summary

The implementation is architecturally sound but needs to address the state machine regression before shipping. The removed/changed state machine tests should be restored, and the InlineIfLambda path needs to remain functional for resumable code patterns in debug builds.

@majocha
Copy link
Copy Markdown
Contributor

majocha commented May 9, 2026

I wonder if it would be possible to just preserve debug points during inlining.
Instead of wholesale disabling inlining which can interfere with CE compilation we could emit original ranges for sequence points.

I did a simple experiment and it allows basic stepping:
image

but this works same source file only.

downside is that inlined methods do not show up in the callstack.

AI says Portable PDB allows sequence points of a single method to jump across source files, but that would need to be implemented. FWIW:

Approach 2: The "PDB Metadata Map" Approach (Recommended)
Portable PDBs fully support a single IL method jumping back and forth across multiple source files. Instead of preventing the inline, we change how the compiler maps the sequence points during the inline process, keeping the layout of the code identical to Release builds but enriching the debugging experience.
This is fundamentally how C++ compilers handle stepping into header-file inline functions, and how modern C# AOT processes plan to handle inlining.
• The Design:

  1. Stop Squashing Debug Points: In Optimizer.fs, when expanding the inline AST, the compiler currently rewrites Expr.DebugPoint nodes, wiping out the original source range and replacing it with the caller's mExpr range. We would change this to retain the original sequence point range.
  2. Emit Multi-Document IL: During IL generation (src/Compiler/AbstractIL/ilwrite.fs), F# will naturally observe these sequence points. The PDB writer will emit the paths mapping back to the original inline function's file.
  3. Local Variable Scopes: F# currently mangles or shadows local variables when inlining. To see variable states, F# would need to ensure the local variable symbols defined within the inline target are enclosed in an IL lexical scope block.
    Handling Cross-Assembly Inlining
    A large feature of F# inline is pulling functions from external assemblies (like FSharp.Core). If we map sequence points using Approach 2, the source file of the imported function will point to an external repository.
    • We lean on SourceLink: As long as the F# pickled ASTs inside FSharp.Core embed the relative source file path, the local PDB will include that external document reference. When a user in Visual Studio presses F11 (Step Into) on Array.map, Visual Studio will hit the external sequence point, download array.fs via SourceLink, and allow them to step through the inlined body line-by-line.

@auduchinok
Copy link
Copy Markdown
Member Author

@majocha Thanks for your suggestions!

I wonder if it would be possible to just preserve debug points during inlining.

That was the first idea that I've investigated and it seems it wouldn't help with improve debugging for any more real-world cases, unfortunately.

downside is that inlined methods do not show up in the callstack.

The main motivation behind this change is to make it possible to reliably step into and through such functions, to see the passed arguments, and so on. It just doesn't work when you don't have real calls and stack frames.

Instead of wholesale disabling inlining which can interfere with CE compilation we could emit original ranges for sequence points.

It seems using the dynamic invocations instead of the fully-inlined state machines can be a good way to improve debugging experience

@majocha
Copy link
Copy Markdown
Contributor

majocha commented May 9, 2026

It seems using the dynamic invocations instead of the fully-inlined state machines can be a good way to improve debugging experience

Yes, true. Just one thing I'd consider is the future runtime async implementation. I haven't given it much thought but it's yet another async mechanism different from state machines. The questions are what would the equivalent of dynamic invocation look like and will it compile fine without inlining?

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

F# .net core debugger: Inline functions don't work as expected in debugger even in Debug builds

4 participants