From 468dfa6ac6b149bb8936900174807db803f26fa6 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 21 May 2024 15:03:49 +0200 Subject: [PATCH] Revert "Fix inlining regression: https://github.com/dotnet/fsharp/issues/17161" --- eng/Versions.props | 2 +- global.json | 2 +- src/Compiler/Optimize/Optimizer.fs | 17 +++- .../EmittedIL/Inlining/Inlining.fs | 83 ------------------- 4 files changed, 16 insertions(+), 88 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index 8ac9507e013..17d084022bd 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -14,7 +14,7 @@ 8 0 - 301 + 300 0 diff --git a/global.json b/global.json index b1a203d2466..1d7b86d8b37 100644 --- a/global.json +++ b/global.json @@ -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", diff --git a/src/Compiler/Optimize/Optimizer.fs b/src/Compiler/Optimize/Optimizer.fs index 0948fc27324..fafb8184600 100644 --- a/src/Compiler/Optimize/Optimizer.fs +++ b/src/Compiler/Optimize/Optimizer.fs @@ -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) -> @@ -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 @@ -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 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs index 391faa9067a..f716ffdffef 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs @@ -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 [] let ``Match01_RealInternalSignatureOn_fs`` compilation = @@ -143,82 +139,3 @@ let found = data |> List.contains nan }"""] #endif - [] // RealSig - [] // Regular - [] - let ``Inlining field with private module`` (realSig) = - Fsx """ -module private PrivateModule = - let moduleValue = 1 - - let inline getModuleValue () = - moduleValue - -[] -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 - - [] // RealSig - [] // Regular - [] - 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 - - [] // RealSig - [] // Regular - [] - 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 -