Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Fix inlining regression: https://github.com/dotnet/fsharp/issues/17161" #17200

Merged
merged 1 commit into from
May 21, 2024
Merged

Revert "Fix inlining regression: https://github.com/dotnet/fsharp/issues/17161" #17200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<!-- F# Version components -->
<FSMajorVersion>8</FSMajorVersion>
<FSMinorVersion>0</FSMinorVersion>
<FSBuildVersion>301</FSBuildVersion>
<FSBuildVersion>300</FSBuildVersion>
<FSRevisionVersion>0</FSRevisionVersion>
<!-- -->
<!-- F# Language version -->
Expand Down
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"xcopy-msbuild": "17.8.1-2"
},
"native-tools": {
"perl": "5.38.2.2"
"perl": "5.38.0.1"
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "8.0.0-beta.24204.3",
Expand Down
17 changes: 14 additions & 3 deletions src/Compiler/Optimize/Optimizer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2356,7 +2356,15 @@ let rec OptimizeExpr cenv (env: IncrementalOptimizationEnv) expr =
OptimizeConst cenv env expr (c, m, ty)

| Expr.Val (v, _vFlags, m) ->
OptimizeVal cenv env expr (v, m)
if not (v.Accessibility.IsPrivate) then
OptimizeVal cenv env expr (v, m)
else
expr,
{ TotalSize = 10
FunctionSize = 1
HasEffect = false
MightMakeCriticalTailcall=false
Info=UnknownValue }


| Expr.Quote (ast, splices, isFromQueryExpression, m, ty) ->
Expand Down Expand Up @@ -3074,6 +3082,9 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, shouldInline, inlineIfLambda,
let fvs = freeInExpr CollectLocals expr
if fvs.UsesMethodLocalConstructs then
// Discarding lambda for binding because uses protected members --- TBD: Should we warn or error here
None
elif fvs.FreeLocals |> Seq.exists(fun v -> v.Accessibility.IsPrivate ) then
// Discarding lambda for binding because uses private members --- TBD: Should we warn or error here
None
else
let exprCopy = CopyExprForInlining cenv inlineIfLambda expr m
Expand Down Expand Up @@ -4112,10 +4123,10 @@ and OptimizeBinding cenv isRec env (TBind(vref, expr, spBind)) =
let fvs = freeInExpr CollectLocals body
if fvs.UsesMethodLocalConstructs then
// Discarding lambda for binding because uses protected members
UnknownValue
UnknownValue
elif fvs.FreeLocals.ToArray() |> Seq.fold(fun acc v -> if not acc then v.Accessibility.IsPrivate else acc) false then
// Discarding lambda for binding because uses private members
UnknownValue
UnknownValue
else
ivalue

Expand Down
83 changes: 0 additions & 83 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ module Inlining =
|> ignoreWarnings
|> verifyILBaseline

let withRealInternalSignature realSig compilation =
compilation
|> withOptions [if realSig then "--realsig+" else "--realsig-" ]

// SOURCE=Match01.fs SCFLAGS="-a --optimize+" COMPILE_ONLY=1 POSTCMD="..\\CompareIL.cmd Match01.dll" # Match01.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"Match01_RealInternalSignatureOn.fs"|])>]
let ``Match01_RealInternalSignatureOn_fs`` compilation =
Expand Down Expand Up @@ -143,82 +139,3 @@ let found = data |> List.contains nan
}"""]
#endif

[<InlineData(true)>] // RealSig
[<InlineData(false)>] // Regular
[<Theory>]
let ``Inlining field with private module`` (realSig) =
Fsx """
module private PrivateModule =
let moduleValue = 1

let inline getModuleValue () =
moduleValue

[<EntryPoint>]
let main argv =
// [FS1118] Failed to inline the value 'getModuleValue' marked 'inline', perhaps because a recursive value was marked 'inline'
// (fixed by making PrivateModule internal instead of private)
PrivateModule.getModuleValue () |> ignore
0
"""
|> withOptimize
|> withRealInternalSignature realSig
|> asExe
|> compileAndRun
|> shouldSucceed

[<InlineData(true)>] // RealSig
[<InlineData(false)>] // Regular
[<Theory>]
let ``Inlining field with private class`` (realSig) =
Fsx """
type private FirstType () =
member this.FirstMethod () = ()

type private SecondType () =
member this.SecondMethod () =
let inline callFirstMethod (first: FirstType) =
first.FirstMethod ()

callFirstMethod (FirstType())

printfn $"{(SecondType ()).SecondMethod()}"
"""
|> withOptimize
|> withRealInternalSignature realSig
|> asExe
|> compileAndRun
|> shouldSucceed

[<InlineData(true)>] // RealSig
[<InlineData(false)>] // Regular
[<Theory>]
let ``Inlining deep local functions field with private class`` (realSig) =
Fsx """
type private FirstType () =
member this.FirstMethod () = ()

type private SecondType () =
member this.SecondMethod () =
let inline callFirstMethod (first: FirstType) =
first.FirstMethod ()

let inline callFirstMethodDeeper (first: FirstType) =
callFirstMethod (first)

let inline callFirstMethodMoreDeeper (first: FirstType) =
callFirstMethodDeeper (first)

let inline callFirstMethodMostDeeply (first: FirstType) =
callFirstMethodMoreDeeper (first)

callFirstMethodMostDeeply (FirstType())

printfn $"{(SecondType ()).SecondMethod()}"
"""
|> withOptimize
|> withRealInternalSignature realSig
|> asExe
|> compileAndRun
|> shouldSucceed

Loading