From c1912ceb39b2165a9a3367b0b11361d95edc8467 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Fri, 29 Nov 2024 12:46:38 +0100 Subject: [PATCH] Fix #17903 (#18025) Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com> Co-authored-by: Petr Pokorny Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../.FSharp.Compiler.Service/9.0.200.md | 7 +- src/Compiler/CodeGen/IlxGen.fs | 46 +++++----- src/Compiler/TypedTree/TypedTree.fs | 5 ++ src/Compiler/TypedTree/TypedTree.fsi | 6 ++ src/Compiler/TypedTree/TypedTreeOps.fs | 10 +-- .../AssemblyBoundary/AssemblyBoundary.fs | 90 +++++++++++++++++++ ...ompiler.Service_Release_netstandard2.0.bsl | 2 +- 7 files changed, 137 insertions(+), 29 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md index 232853cfea5..8c19083bb74 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md @@ -11,15 +11,16 @@ * Fix internal error when analyzing incomplete inherit member ([PR #17905](https://github.com/dotnet/fsharp/pull/17905)) * Add warning when downcasting from nullable type to non-nullable ([PR #17965](https://github.com/dotnet/fsharp/pull/17965)) * Fix missing nullness warning in case of method resolution multiple candidates ([PR #17917](https://github.com/dotnet/fsharp/pull/17918)) -* Fix failure to use bound values in `when` clauses of `try-with` in `seq` expressions ([# 17990](https://github.com/dotnet/fsharp/pull/17990)) +* Fix failure to use bound values in `when` clauses of `try-with` in `seq` expressions ([PR #17990](https://github.com/dotnet/fsharp/pull/17990)) +* Fix locals allocating for the special `copyOfStruct` defensive copy ([PR #18025](https://github.com/dotnet/fsharp/pull/18025)) ### Added * Let `dotnet fsi --help` print a link to the documentation website. ([PR #18006](https://github.com/dotnet/fsharp/pull/18006)) * Deprecate places where `seq` can be omitted. ([Language suggestion #1033](https://github.com/fsharp/fslang-suggestions/issues/1033), [PR #17772](https://github.com/dotnet/fsharp/pull/17772)) * Support literal attribute on decimals ([PR #17769](https://github.com/dotnet/fsharp/pull/17769)) -* Added type conversions cache, only enabled for compiler runs, guarded by language version preview ([PR#17668](https://github.com/dotnet/fsharp/pull/17668)) -* Added project property ParallelCompilation which turns on graph based type checking, parallel ILXGen and parallel optimization. By default on for users of langversion=preview ([PR#17948](https://github.com/dotnet/fsharp/pull/17948)) +* Added type conversions cache, only enabled for compiler runs, guarded by language version preview ([PR #17668](https://github.com/dotnet/fsharp/pull/17668)) +* Added project property ParallelCompilation which turns on graph based type checking, parallel ILXGen and parallel optimization. By default on for users of langversion=preview ([PR #17948](https://github.com/dotnet/fsharp/pull/17948)) ### Changed diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 2a1c876b8f5..3d79a8020e9 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -2461,7 +2461,7 @@ let FeeFeeInstr (cenv: cenv) doc = type CodeGenBuffer(m: range, mgbuf: AssemblyBuilder, methodName, alreadyUsedArgs: int) = let g = mgbuf.cenv.g - let locals = ResizeArray<(string * (Mark * Mark)) list * ILType * bool>(10) + let locals = ResizeArray<(string * (Mark * Mark)) list * ILType * bool * bool>(10) let codebuf = ResizeArray(200) let exnSpecs = ResizeArray(10) @@ -2651,18 +2651,18 @@ type CodeGenBuffer(m: range, mgbuf: AssemblyBuilder, methodName, alreadyUsedArgs member _.PreallocatedArgCount = alreadyUsedArgs - member _.AllocLocal(ranges, ty, isFixed) = + member _.AllocLocal(ranges, ty, isFixed, canBeReallocd) = let j = locals.Count - locals.Add((ranges, ty, isFixed)) + locals.Add((ranges, ty, isFixed, canBeReallocd)) j - member cgbuf.ReallocLocal(cond, ranges, ty, isFixed) = + member cgbuf.ReallocLocal(cond, ranges, ty, isFixed, canBeReallocd) = match ResizeArray.tryFindIndexi cond locals with | Some j -> - let prevRanges, _, isFixed = locals[j] - locals[j] <- ((ranges @ prevRanges), ty, isFixed) + let prevRanges, _, isFixed, _ = locals[j] + locals[j] <- ((ranges @ prevRanges), ty, isFixed, canBeReallocd) j, true - | None -> cgbuf.AllocLocal(ranges, ty, isFixed), false + | None -> cgbuf.AllocLocal(ranges, ty, isFixed, canBeReallocd), false member _.Close() = @@ -2772,7 +2772,10 @@ let CodeGenThen (cenv: cenv) mgbuf (entryPointInfo, methodName, eenv, alreadyUse && not cenv.options.localOptimizationsEnabled -> let ilTy = selfArg.Type |> GenType cenv m eenv.tyenv - let idx = cgbuf.AllocLocal([ (selfArg.LogicalName, (start, finish)) ], ilTy, false) + + let idx = + cgbuf.AllocLocal([ (selfArg.LogicalName, (start, finish)) ], ilTy, false, true) + cgbuf.EmitStartOfHiddenCode() CG.EmitInstrs cgbuf (pop 0) Push0 [ mkLdarg0; I_stloc(uint16 idx) ] | _ -> () @@ -2792,7 +2795,7 @@ let CodeGenThen (cenv: cenv) mgbuf (entryPointInfo, methodName, eenv, alreadyUse let localDebugSpecs: ILLocalDebugInfo list = locals - |> List.mapi (fun i (nms, _, _isFixed) -> List.map (fun nm -> (i, nm)) nms) + |> List.mapi (fun i (nms, _, _isFixed, _canBeReallocd) -> List.map (fun nm -> (i, nm)) nms) |> List.concat |> List.map (fun (i, (nm, (start, finish))) -> { @@ -2802,7 +2805,7 @@ let CodeGenThen (cenv: cenv) mgbuf (entryPointInfo, methodName, eenv, alreadyUse let ilLocals = locals - |> List.map (fun (infos, ty, isFixed) -> + |> List.map (fun (infos, ty, isFixed, _canBeReallocd) -> let loc = // in interactive environment, attach name and range info to locals to improve debug experience if cenv.options.isInteractive && cenv.options.generateDebugSymbols then @@ -3441,7 +3444,7 @@ and GenLinearExpr cenv cgbuf eenv expr sequel preSteps (contf: FakeUnit -> FakeU contf Fake | Expr.Let(bind, body, _, _) -> - // Process the debug point and see if there's a replacement technique to process this expression + if preSteps && GenExprPreSteps cenv cgbuf eenv expr sequel then contf Fake else @@ -3837,7 +3840,7 @@ and UnionCodeGen (cgbuf: CodeGenBuffer) = CG.GenerateDelayMark cgbuf "unionCodeGenMark" member _.GenLocal ilTy = - cgbuf.AllocLocal([], ilTy, false) |> uint16 + cgbuf.AllocLocal([], ilTy, false, true) |> uint16 member _.SetMarkToHere m = CG.SetMarkToHere cgbuf m @@ -4673,7 +4676,7 @@ and GenIndirectCall cenv cgbuf eenv (funcTy, tyargs, curriedArgs, m) sequel = let instrs = EraseClosures.mkCallFunc cenv.ilxPubCloEnv - (fun ty -> cgbuf.AllocLocal([], ty, false) |> uint16) + (fun ty -> cgbuf.AllocLocal([], ty, false, true) |> uint16) eenv.tyenv.Count isTailCall ilxClosureApps @@ -9822,6 +9825,13 @@ and GenGetLocalVRef cenv cgbuf eenv m (vref: ValRef) storeSequel = and GenStoreVal cgbuf eenv m (vspec: Val) = GenSetStorage vspec.Range cgbuf (StorageForVal m vspec eenv) +and CanRealloc isFixed eenv ty i (_, ty2, isFixed2, canBeReallocd) = + canBeReallocd + && not isFixed2 + && not isFixed + && not (IntMap.mem i eenv.liveLocals) + && (ty = ty2) + /// Allocate IL locals and AllocLocal cenv cgbuf eenv compgen (v, ty, isFixed) (scopeMarks: Mark * Mark) : int * _ * _ = // The debug range for the local @@ -9829,14 +9839,10 @@ and AllocLocal cenv cgbuf eenv compgen (v, ty, isFixed) (scopeMarks: Mark * Mark // Get an index for the local let j, realloc = if cenv.options.localOptimizationsEnabled then - cgbuf.ReallocLocal( - (fun i (_, ty2, isFixed2) -> not isFixed2 && not isFixed && not (IntMap.mem i eenv.liveLocals) && (ty = ty2)), - ranges, - ty, - isFixed - ) + let canBeReallocd = not (v = WellKnownNames.CopyOfStruct) + cgbuf.ReallocLocal(CanRealloc isFixed eenv ty, ranges, ty, isFixed, canBeReallocd) else - cgbuf.AllocLocal(ranges, ty, isFixed), false + cgbuf.AllocLocal(ranges, ty, isFixed, false), false j, realloc, diff --git a/src/Compiler/TypedTree/TypedTree.fs b/src/Compiler/TypedTree/TypedTree.fs index b5e620c6bd6..1aaa21eb294 100644 --- a/src/Compiler/TypedTree/TypedTree.fs +++ b/src/Compiler/TypedTree/TypedTree.fs @@ -32,6 +32,11 @@ open FSharp.Compiler.TypeProviders open FSharp.Core.CompilerServices #endif +[] +module WellKnownNames = + /// Special name for the defensive copy of a struct, we use it in situations like when we get an address of a field in ax-assembly scenario. + let [] CopyOfStruct = "copyOfStruct" + type Stamp = int64 type StampMap<'T> = Map diff --git a/src/Compiler/TypedTree/TypedTree.fsi b/src/Compiler/TypedTree/TypedTree.fsi index 5f96fba2266..73eeb760b4c 100644 --- a/src/Compiler/TypedTree/TypedTree.fsi +++ b/src/Compiler/TypedTree/TypedTree.fsi @@ -17,6 +17,12 @@ open FSharp.Compiler.TypeProviders open FSharp.Compiler.Xml open FSharp.Core.CompilerServices +[] +module WellKnownNames = + /// Special name for the defensive copy of a struct, we use it in situations like when we get an address of a field in ax-assembly scenario. + [] + val CopyOfStruct: string = "copyOfStruct" + val getNameOfScopeRef: sref: ILScopeRef -> string type Stamp = int64 diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index aa9bbad7f8c..e2c1c0861a9 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -6015,14 +6015,14 @@ and remapExprImpl (ctxt: RemapContext) (compgen: ValCopyFlag) (tmenv: Remap) exp // This is "ok", in the sense that it is always valid to fix these up to be uses // of a temporary local, e.g. // &(E.RF) --> let mutable v = E.RF in &v - + | Expr.Op (TOp.ValFieldGetAddr (rfref, readonly), tinst, [arg], m) when not rfref.RecdField.IsMutable && not (entityRefInThisAssembly ctxt.g.compilingFSharpCore rfref.TyconRef) -> let tinst = remapTypes tmenv tinst let arg = remapExprImpl ctxt compgen tmenv arg - let tmp, _ = mkMutableCompGenLocal m "copyOfStruct" (actualTyOfRecdFieldRef rfref tinst) + let tmp, _ = mkMutableCompGenLocal m WellKnownNames.CopyOfStruct (actualTyOfRecdFieldRef rfref tinst) mkCompGenLet m tmp (mkRecdFieldGetViaExprAddr (arg, rfref, tinst, m)) (mkValAddr m readonly (mkLocalValRef tmp)) | Expr.Op (TOp.UnionCaseFieldGetAddr (uref, cidx, readonly), tinst, [arg], m) when @@ -6031,7 +6031,7 @@ and remapExprImpl (ctxt: RemapContext) (compgen: ValCopyFlag) (tmenv: Remap) exp let tinst = remapTypes tmenv tinst let arg = remapExprImpl ctxt compgen tmenv arg - let tmp, _ = mkMutableCompGenLocal m "copyOfStruct" (actualTyOfUnionFieldRef uref cidx tinst) + let tmp, _ = mkMutableCompGenLocal m WellKnownNames.CopyOfStruct (actualTyOfUnionFieldRef uref cidx tinst) mkCompGenLet m tmp (mkUnionCaseFieldGetProvenViaExprAddr (arg, uref, tinst, cidx, m)) (mkValAddr m readonly (mkLocalValRef tmp)) | Expr.Op (op, tinst, args, m) -> @@ -7252,8 +7252,8 @@ let rec mkExprAddrOfExprAux g mustTakeAddress useReadonlyForGenericArrayAddress // Take a defensive copy let tmp, _ = match mut with - | NeverMutates -> mkCompGenLocal m "copyOfStruct" ty - | _ -> mkMutableCompGenLocal m "copyOfStruct" ty + | NeverMutates -> mkCompGenLocal m WellKnownNames.CopyOfStruct ty + | _ -> mkMutableCompGenLocal m WellKnownNames.CopyOfStruct ty // This local is special in that it ignore byref scoping rules. tmp.SetIgnoresByrefScope() diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/AssemblyBoundary/AssemblyBoundary.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/AssemblyBoundary/AssemblyBoundary.fs index df92ac60f62..011afc282a7 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/AssemblyBoundary/AssemblyBoundary.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/AssemblyBoundary/AssemblyBoundary.fs @@ -88,3 +88,93 @@ module AssemblyBoundary = compilation |> withReferences [typeLib02] |> verifyCompileAndExecution + + [] + let ``copyOfStruct doesn't reallocate local in case of cross-assembly inlining`` () = + let library = + FSharp """ +namespace Library + +#nowarn "346" + +[] +type ID (value : string) = + member _.Value = value + member inline this.Hello(other: ID) = System.Console.WriteLine(this.Value + " " + other.Value) + +type ID2 = { Value : ID } with + member inline this.Hello(other: ID2) = this.Value.Hello other.Value + """ + |> asLibrary + |> withName "Library" + |> withOptimize + + let program = + FSharp """ +open Library + +[] +let main _ = + + let aBar = { Value = ID "first" } + let bBar = { Value = ID "second" } + + aBar.Hello(bBar) + + 0 + """ + |> withReferences [library] + |> withOptimize + + let compilation = + program + |> asExe + |> compile + + compilation + |> shouldSucceed + |> run + |> shouldSucceed + |> verifyOutputContains [| "first second" |] + |> verifyIL + [ """ +.method public static int32 main(string[] _arg1) cil managed +{ + .entrypoint + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.EntryPointAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 5 + .locals init (class [Library]Library.ID2 V_0, + class [Library]Library.ID2 V_1, + valuetype [Library]Library.ID& V_2, + valuetype [Library]Library.ID V_3, + valuetype [Library]Library.ID V_4) + IL_0000: ldstr "first" + IL_0005: newobj instance void [Library]Library.ID::.ctor(string) + IL_000a: newobj instance void [Library]Library.ID2::.ctor(valuetype [Library]Library.ID) + IL_000f: stloc.0 + IL_0010: ldstr "second" + IL_0015: newobj instance void [Library]Library.ID::.ctor(string) + IL_001a: newobj instance void [Library]Library.ID2::.ctor(valuetype [Library]Library.ID) + IL_001f: stloc.1 + IL_0020: ldloc.0 + IL_0021: call instance valuetype [Library]Library.ID [Library]Library.ID2::get_Value() + IL_0026: stloc.3 + IL_0027: ldloca.s V_3 + IL_0029: stloc.2 + IL_002a: ldloc.1 + IL_002b: call instance valuetype [Library]Library.ID [Library]Library.ID2::get_Value() + IL_0030: stloc.s V_4 + IL_0032: ldloc.2 + IL_0033: call instance string [Library]Library.ID::get_Value() + IL_0038: ldstr " " + IL_003d: ldloca.s V_4 + IL_003f: call instance string [Library]Library.ID::get_Value() + IL_0044: call string [runtime]System.String::Concat(string, + string, + string) + IL_0049: call void [runtime]System.Console::WriteLine(string) + IL_004e: ldc.i4.0 + IL_004f: ret +} + """] \ No newline at end of file diff --git a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl index c358adc7976..fc795ecde1b 100644 --- a/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl +++ b/tests/ILVerify/ilverify_FSharp.Compiler.Service_Release_netstandard2.0.bsl @@ -33,7 +33,7 @@ [IL]: Error [StackUnexpected]: : .$FSharpCheckerResults+GetReferenceResolutionStructuredToolTipText@2205::Invoke([FSharp.Core]Microsoft.FSharp.Core.Unit)][offset 0x00000076][found Char] Unexpected type on the stack. [IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.AssemblyContent+traverseMemberFunctionAndValues@176::Invoke([FSharp.Compiler.Service]FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue)][offset 0x0000002B][found Char] Unexpected type on the stack. [IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.AssemblyContent+traverseEntity@218::GenerateNext([S.P.CoreLib]System.Collections.Generic.IEnumerable`1&)][offset 0x000000BB][found Char] Unexpected type on the stack. -[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>, [FSharp.Compiler.Service]FSharp.Compiler.Syntax.SynExpr)][offset 0x00000618][found Char] Unexpected type on the stack. +[IL]: Error [StackUnexpected]: : FSharp.Compiler.EditorServices.ParsedInput+visitor@1423-11::VisitExpr([FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>, [FSharp.Compiler.Service]FSharp.Compiler.Syntax.SynExpr)][offset 0x00000620][found Char] Unexpected type on the stack. [IL]: Error [StackUnexpected]: : .$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Core.Unit>)][offset 0x00000032][found Char] Unexpected type on the stack. [IL]: Error [StackUnexpected]: : .$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Core.Unit>)][offset 0x0000003B][found Char] Unexpected type on the stack. [IL]: Error [StackUnexpected]: : .$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2,Microsoft.FSharp.Core.Unit>)][offset 0x00000064][found Char] Unexpected type on the stack.