Skip to content

Commit ac14120

Browse files
authored
Optimizer: eliminate unused Unchecked.defaultof<'T> bindings (dotnet#19758)
* Fix EI_ilzero optimization: treat Unchecked.defaultof as effect-free for concrete types Unchecked.defaultof<'T> (compiled as EI_ilzero) was unconditionally treated as having an effect, preventing the optimizer from eliminating unused bindings like `let _ = Unchecked.defaultof<decimal>`. Fix: In ExprHasEffect and OptimizeExprOpFallback, EI_ilzero is now considered effect-free when all type args are concrete (not type parameters). When type variables are present (e.g. SRTP ^T), it is conservatively treated as having an effect to prevent orphaned type variables during IL generation (FS0073).
1 parent 4d1291a commit ac14120

6 files changed

Lines changed: 190 additions & 19 deletions

File tree

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* Diagnostic FS0027 now emits a parameter-specific message (suggesting a `let mutable x = x` shadow or `byref<_>`) instead of the illegal `let mutable x = expression` shadow when the assignment target is a function or method parameter. ([Issue #15803](https://github.com/dotnet/fsharp/issues/15803), [PR #19866](https://github.com/dotnet/fsharp/pull/19866))
99
* Recursive `inline` functions and members now emit a single clear error (FS3890) instead of a misleading FS1113/FS1114 optimizer cascade. ([Issue #17991](https://github.com/dotnet/fsharp/issues/17991), [PR #19803](https://github.com/dotnet/fsharp/pull/19803))
1010
* Report `FS0037 Duplicate definition of type or module` at type-check time when two sibling modules in a `module rec` / `namespace rec` group share a name, instead of letting the duplicate slip through to IL emit where it surfaced as the cryptic `FS2014: duplicate entry . in type index table`. ([Issue #6694](https://github.com/dotnet/fsharp/issues/6694), [PR #19913](https://github.com/dotnet/fsharp/pull/19913))
11+
* Unused `Unchecked.defaultof<'T>` bindings are now eliminated under optimization at their use sites. This removes redundant `initobj`/`ldnull;pop` sequences left behind after inlining SRTP helpers that use `Unchecked.defaultof` as dummy arguments to drive static-member-constraint resolution. Such bindings are preserved inside `inline` bodies (whose optimized form is pickled as cross-assembly optimization info and re-inlined by consumers) and whenever the erased type still references unsolved type variables, so no `FS0073` regression is introduced. ([Issue #18128](https://github.com/dotnet/fsharp/issues/18128), [PR #19758](https://github.com/dotnet/fsharp/pull/19758))
1112
* Semantic classification no longer marks recursive object self-references (`as this`, `let rec` self-refs) as mutable. ([Issue #5229](https://github.com/dotnet/fsharp/issues/5229))
1213
* Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955))
1314
* Preserve source range for type errors on empty-bodied computation expressions (e.g. `foo {}`) in pipelines, function arguments, and type-annotated contexts, instead of reporting `unknown(1,1)`. ([Issue #19550](https://github.com/dotnet/fsharp/issues/19550), [PR #19849](https://github.com/dotnet/fsharp/pull/19849))

src/Compiler/CodeGen/IlxGen.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4708,7 +4708,7 @@ and GenApp (cenv: cenv) cgbuf eenv (f, fty, tyargs, curriedArgs, m) sequel =
47084708
(eenv, laterArgs)
47094709
||> List.mapFold (fun eenv laterArg ->
47104710
// Only save arguments that have effects
4711-
if Optimizer.ExprHasEffect g laterArg then
4711+
if Optimizer.ExprHasEffect Optimizer.EffectContext.Emit g laterArg then
47124712
let ilTy = laterArg |> tyOfExpr g |> GenType cenv m eenv.tyenv
47134713

47144714
let locName =

src/Compiler/Optimize/Optimizer.fs

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,36 +1617,68 @@ let SplitValuesByIsUsedOrHasEffect cenv fvs x =
16171617

16181618
let IlAssemblyCodeInstrHasEffect i =
16191619
match i with
1620-
| ( AI_nop | AI_ldc _ | AI_add | AI_sub | AI_mul | AI_xor | AI_and | AI_or
1621-
| AI_ceq | AI_cgt | AI_cgt_un | AI_clt | AI_clt_un | AI_conv _ | AI_shl
1622-
| AI_shr | AI_shr_un | AI_neg | AI_not | AI_ldnull )
1620+
| AI_nop | AI_ldc _ | AI_add | AI_sub | AI_mul | AI_xor | AI_and | AI_or
1621+
| AI_ceq | AI_cgt | AI_cgt_un | AI_clt | AI_clt_un | AI_conv _ | AI_shl
1622+
| AI_shr | AI_shr_un | AI_neg | AI_not | AI_ldnull
16231623
| I_ldstr _ | I_ldtoken _ -> false
16241624
| _ -> true
1625-
1625+
16261626
let IlAssemblyCodeHasEffect instrs = List.exists IlAssemblyCodeInstrHasEffect instrs
16271627

1628-
let rec ExprHasEffect g expr =
1628+
/// The context an expression's effects are analyzed in. It governs whether a fully-ground
1629+
/// `Unchecked.defaultof` (an `EI_ilzero` witness) may be treated as effect-free and its unused
1630+
/// binding dropped: safe when emitting here, but not inside an `inline` value's body, whose
1631+
/// optimized form is pickled as cross-assembly info and re-inlined at each use site — dropping
1632+
/// the witness there corrupts that info and trips FS0073 in the consumer's IlxGen (e.g. the SRTP
1633+
/// witness/dummy arguments used pervasively by FSharpPlus).
1634+
[<RequireQualifiedAccess>]
1635+
type EffectContext =
1636+
/// Emitting IL here, at a fully instantiated use site.
1637+
| Emit
1638+
/// Analyzing the pickled body of an `inline` value, re-inlined at other sites.
1639+
| InlineBody
1640+
1641+
let ILAsmWithIlzeroHasEffect context instrs tyargs =
1642+
let hasIlzero = instrs |> List.exists (function EI_ilzero _ -> true | _ -> false)
1643+
1644+
if not hasIlzero then
1645+
IlAssemblyCodeHasEffect instrs
1646+
else
1647+
// A fully-ground ilzero is a pure default-value load whose unused binding may be dropped,
1648+
// but only at emission and only when no tyarg still holds a free typar: such a binding is
1649+
// the sole pin for those typars and dropping it leaves them unsolved (FS0073).
1650+
let ilzeroIsSafeToDrop =
1651+
match context with
1652+
| EffectContext.Emit -> tyargs |> List.forall (fun ty -> Zset.isEmpty (freeInType CollectTypars ty).FreeTypars)
1653+
| EffectContext.InlineBody -> false
1654+
1655+
let otherInstrsHaveEffect =
1656+
instrs |> List.exists (function EI_ilzero _ -> false | i -> IlAssemblyCodeInstrHasEffect i)
1657+
1658+
otherInstrsHaveEffect || not ilzeroIsSafeToDrop
1659+
1660+
let rec ExprHasEffect context g expr =
16291661
match stripDebugPoints expr with
16301662
| Expr.Val (vref, _, _) -> vref.IsTypeFunction || vref.IsMutable
16311663
| Expr.Quote _
16321664
| Expr.Lambda _
16331665
| Expr.TyLambda _
16341666
| Expr.Const _ -> false
16351667
// type applications do not have effects, with the exception of type functions
1636-
| Expr.App (f0, _, _, [], _) -> IsTyFuncValRefExpr f0 || ExprHasEffect g f0
1637-
| Expr.Op (op, _, args, m) -> ExprsHaveEffect g args || OpHasEffect g m op
1638-
| Expr.LetRec (binds, body, _, _) -> BindingsHaveEffect g binds || ExprHasEffect g body
1639-
| Expr.Let (bind, body, _, _) -> BindingHasEffect g bind || ExprHasEffect g body
1668+
| Expr.App (f0, _, _, [], _) -> IsTyFuncValRefExpr f0 || ExprHasEffect context g f0
1669+
| Expr.Op (op, tyargs, args, m) -> ExprsHaveEffect context g args || OpHasEffect context g m op tyargs
1670+
| Expr.LetRec (binds, body, _, _) -> BindingsHaveEffect context g binds || ExprHasEffect context g body
1671+
| Expr.Let (bind, body, _, _) -> BindingHasEffect context g bind || ExprHasEffect context g body
16401672
// REVIEW: could add Expr.Obj on an interface type - these are similar to records of lambda expressions
16411673
| _ -> true
16421674

1643-
and ExprsHaveEffect g exprs = List.exists (ExprHasEffect g) exprs
1675+
and ExprsHaveEffect context g exprs = List.exists (ExprHasEffect context g) exprs
16441676

1645-
and BindingsHaveEffect g binds = List.exists (BindingHasEffect g) binds
1677+
and BindingsHaveEffect context g binds = List.exists (BindingHasEffect context g) binds
16461678

1647-
and BindingHasEffect g bind = bind.Expr |> ExprHasEffect g
1679+
and BindingHasEffect context g bind = bind.Expr |> ExprHasEffect context g
16481680

1649-
and OpHasEffect g m op =
1681+
and OpHasEffect context g m op tyargs =
16501682
match op with
16511683
| TOp.Tuple _ -> false
16521684
| TOp.AnonRecd _ -> false
@@ -1660,7 +1692,7 @@ and OpHasEffect g m op =
16601692
| TOp.UnionCaseTagGet _ -> false
16611693
| TOp.UnionCaseProof _ -> false
16621694
| TOp.UnionCaseFieldGet (ucref, n) -> isUnionCaseFieldMutable g ucref n
1663-
| TOp.ILAsm (instrs, _) -> IlAssemblyCodeHasEffect instrs
1695+
| TOp.ILAsm (instrs, _) -> ILAsmWithIlzeroHasEffect context instrs tyargs
16641696
| TOp.TupleFieldGet _ -> false
16651697
| TOp.ExnFieldGet (ecref, n) -> isExnFieldMutable ecref n
16661698
| TOp.RefAddrGet _ -> false
@@ -1687,6 +1719,9 @@ and OpHasEffect g m op =
16871719
| TOp.LValueOp _ (* conservative *)
16881720
| TOp.ValFieldSet _ -> true
16891721

1722+
let effectContextOf (cenv: cenv) =
1723+
if cenv.optimizing then EffectContext.Emit else EffectContext.InlineBody
1724+
16901725

16911726
let TryEliminateBinding cenv _env bind e2 _m =
16921727
let g = cenv.g
@@ -1717,7 +1752,7 @@ let TryEliminateBinding cenv _env bind e2 _m =
17171752
match argsr with
17181753
| Expr.Val (VRefLocal vspec2, _, _) :: argsr2
17191754
when valEq vspec1 vspec2 && IsUniqueUse vspec2 (List.rev rargsl@argsr2) -> Some(List.rev rargsl, argsr2)
1720-
| argsrh :: argsrt when not (ExprHasEffect g argsrh) -> GetImmediateUseContext (argsrh :: rargsl) argsrt
1755+
| argsrh :: argsrt when not (ExprHasEffect (effectContextOf cenv) g argsrh) -> GetImmediateUseContext (argsrh :: rargsl) argsrt
17211756
| _ -> None
17221757

17231758
let (DebugPoints(e2, recreate0)) = e2
@@ -2603,7 +2638,7 @@ and OptimizeExprOp cenv env (op, tyargs, args, m) =
26032638
newExpr,
26042639
{ TotalSize = 1
26052640
FunctionSize = 1
2606-
HasEffect = OpHasEffect g m newOp
2641+
HasEffect = OpHasEffect (effectContextOf cenv) g m newOp tyargs
26072642
MightMakeCriticalTailcall = false
26082643
Info = ValueOfExpr newExpr }
26092644

@@ -2680,7 +2715,7 @@ and OptimizeExprOpFallback cenv env (op, tyargs, argsR, m) arginfos value_ =
26802715
let argsFSize = AddFunctionSizes arginfos
26812716
let argEffects = OrEffects arginfos
26822717
let argValues = List.map (fun x -> x.Info) arginfos
2683-
let effect = OpHasEffect g m op
2718+
let effect = OpHasEffect (effectContextOf cenv) g m op tyargs
26842719
let cost, value_ =
26852720
match op with
26862721
| TOp.UnionCase c -> 2, MakeValueInfoForUnionCase c (Array.ofList argValues)

src/Compiler/Optimize/Optimizer.fsi

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,16 @@ val AbstractOptimizationInfoToEssentials: (CcuOptimizationInfo -> CcuOptimizatio
110110
/// Combine optimization infos
111111
val UnionOptimizationInfos: seq<ImplFileOptimizationInfo> -> CcuOptimizationInfo
112112

113+
/// The context an expression's effects are analyzed in: emitting IL here, or analyzing the
114+
/// pickled body of an `inline` value. Governs whether a fully-ground `Unchecked.defaultof`
115+
/// binding may be eliminated.
116+
[<RequireQualifiedAccess>]
117+
type EffectContext =
118+
| Emit
119+
| InlineBody
120+
113121
/// Check if an expression has an effect
114-
val ExprHasEffect: TcGlobals -> Expr -> bool
122+
val ExprHasEffect: EffectContext -> TcGlobals -> Expr -> bool
115123

116124
val internal u_CcuOptimizationInfo: ReaderState -> CcuOptimizationInfo
117125

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.
2+
3+
namespace EmittedIL
4+
5+
open Xunit
6+
open FSharp.Test.Compiler
7+
8+
module ``UncheckedDefaultofOptimization`` =
9+
10+
// https://github.com/dotnet/fsharp/issues/18128
11+
// Unused `Unchecked.defaultof<concreteType>` bindings should be eliminated under optimization.
12+
// Pins both the absence of `initobj` and that no decimal local slot is allocated for any of the
13+
// three discarded bindings.
14+
[<Fact>]
15+
let ``Unused Unchecked.defaultof bindings of concrete types are eliminated`` () =
16+
FSharp """
17+
module Test
18+
open System
19+
let f (n: float32) =
20+
Console.WriteLine n
21+
let _ = Unchecked.defaultof<decimal>
22+
let _ = Unchecked.defaultof<decimal>
23+
let _ = Unchecked.defaultof<decimal>
24+
let n' = n * 2.f
25+
Console.WriteLine n'
26+
"""
27+
|> withOptimize
28+
|> asLibrary
29+
|> compile
30+
|> shouldSucceed
31+
|> verifyILNotPresent [ "initobj"; "valuetype [runtime]System.Decimal" ]
32+
33+
// https://github.com/dotnet/fsharp/issues/18128
34+
// The FSharpPlus-style SRTP witness pattern from the issue. Elimination happens at the (fully
35+
// instantiated) use site: `doWork` reduces to a direct multiplication with the `nil<PreOps>` and
36+
// `nil< ^b >` witness bindings gone. The assertion is scoped to `doWork` rather than the whole
37+
// module because the inline functions also emit a compiler-generated dynamic-invocation stub which
38+
// legitimately retains an `ldnull` (see the guard below and issue #19758 - the witness bindings of
39+
// an *inline* body must be preserved so cross-assembly consumers can re-inline and specialize them).
40+
[<Fact>]
41+
let ``Unused Unchecked.defaultof SRTP witness bindings are eliminated at the use site`` () =
42+
FSharp """
43+
module Test
44+
45+
open System.ComponentModel
46+
47+
[<AbstractClass; Sealed; EditorBrowsable(EditorBrowsableState.Never)>]
48+
type PreOps =
49+
static member inline Double (n: float<'u>) : float<'u> = n * 2.
50+
static member inline Double (n: float32<'u>) : float32<'u> = n * 2.f
51+
52+
#nowarn "64"
53+
module PreludeOperators =
54+
let inline private nil<'T> = Unchecked.defaultof<'T>
55+
let inline double (x: ^a) =
56+
let inline _call (_: ^M, input: ^I, _: ^R) = ((^M or ^I) : (static member Double : ^I -> ^R) input)
57+
_call (nil<PreOps>, x, nil< ^b >)
58+
59+
open PreludeOperators
60+
let doWork (n: float) = double n
61+
"""
62+
|> withOptimize
63+
|> asLibrary
64+
|> compile
65+
|> shouldSucceed
66+
|> verifyIL [
67+
""".method public static float64 doWork(float64 n) cil managed
68+
{
69+
70+
.maxstack 8
71+
IL_0000: ldarg.0
72+
IL_0001: ldc.r8 2.
73+
IL_000a: mul
74+
IL_000b: ret
75+
}"""
76+
]
77+
78+
// https://github.com/dotnet/fsharp/issues/18128
79+
// Soundness pin: eliminating an unused `Unchecked.defaultof<T>` must not introduce a new reference
80+
// to T in the enclosing method. `defaultof` of a reference type lowers to `ldnull` (not `newobj`),
81+
// so removing the binding cannot suppress an observable cctor call - `f`'s body must contain no
82+
// reference to `WithCctor` at all.
83+
[<Fact>]
84+
let ``Eliminated Unchecked.defaultof leaves no reference to T in the caller`` () =
85+
FSharp """
86+
module Test
87+
88+
type WithCctor() =
89+
static do failwith "cctor must not run"
90+
91+
let f () =
92+
let _ = Unchecked.defaultof<WithCctor>
93+
42
94+
"""
95+
|> withOptimize
96+
|> asLibrary
97+
|> compile
98+
|> shouldSucceed
99+
|> verifyIL [
100+
""".method public static int32 f() cil managed
101+
{
102+
103+
.maxstack 8
104+
IL_0000: ldc.i4.s 42
105+
IL_0002: ret
106+
}"""
107+
]
108+
109+
// https://github.com/dotnet/fsharp/issues/18128
110+
// Regression for SRTP witness/dummy-argument patterns (e.g. FSharpPlus) where eliminating an
111+
// `Unchecked.defaultof` binding whose type still references unsolved typars would trip FS0073 in
112+
// IlxGen. Such bindings must be kept; only fully-ground ilzero bindings are removed.
113+
[<Fact>]
114+
let ``Unused Unchecked.defaultof bindings of unsolved generic types do not cause FS0073`` () =
115+
FSharp """
116+
module Test
117+
let inline witness () = Unchecked.defaultof<'T>
118+
let inline run< ^T when ^T: (static member Zero: ^T)> () =
119+
let _ = (witness () : ^T)
120+
LanguagePrimitives.GenericZero< ^T>
121+
let v : int = run< int> ()
122+
"""
123+
|> withOptimize
124+
|> asLibrary
125+
|> compile
126+
|> shouldSucceed

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@
252252
<!--<Compile Include="EmittedIL\StructGettersReadOnly.fs" />-->
253253
<Compile Include="EmittedIL\TailCalls.fs" />
254254
<Compile Include="EmittedIL\TupleElimination.fs" />
255+
<Compile Include="EmittedIL\UncheckedDefaultofOptimization.fs" />
255256
<Compile Include="EmittedIL\TypeTestsInPatternMatching.fs" />
256257
<Compile Include="EmittedIL\WhileLoops.fs" />
257258
<Compile Include="EmittedIL\ArgumentNames.fs" />

0 commit comments

Comments
 (0)