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.

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

Fix #17903 #18025

Merged
merged 7 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
46 changes: 26 additions & 20 deletions src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ILInstr>(200)
let exnSpecs = ResizeArray<ILExceptionSpec>(10)

Expand Down Expand Up @@ -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) =
vzarytovskii marked this conversation as resolved.
Show resolved Hide resolved
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() =

Expand Down Expand Up @@ -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) ]
| _ -> ()
Expand All @@ -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))) ->
{
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -9822,21 +9825,24 @@ 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
let ranges = if compgen then [] else [ (v, scopeMarks) ]
// 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,
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ open FSharp.Compiler.TypeProviders
open FSharp.Core.CompilerServices
#endif

[<RequireQualifiedAccess>]
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 [<Literal>] CopyOfStruct = "copyOfStruct"

type Stamp = int64

type StampMap<'T> = Map<Stamp, 'T>
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/TypedTree/TypedTree.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ open FSharp.Compiler.TypeProviders
open FSharp.Compiler.Xml
open FSharp.Core.CompilerServices

[<RequireQualifiedAccess>]
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.
[<Literal>]
val CopyOfStruct: string = "copyOfStruct"

val getNameOfScopeRef: sref: ILScopeRef -> string

type Stamp = int64
Expand Down
10 changes: 5 additions & 5 deletions src/Compiler/TypedTree/TypedTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) ->
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,93 @@ module AssemblyBoundary =
compilation
|> withReferences [typeLib02]
|> verifyCompileAndExecution

[<Fact>]
let ``copyOfStruct doesn't reallocate local in case of cross-assembly inlining`` () =
let library =
FSharp """
namespace Library

#nowarn "346"

[<Struct>]
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

[<EntryPoint>]
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
}
"""]
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$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<FSharp.Compiler.EditorServices.AssemblySymbol>&)][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.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [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.Compiler.Syntax.SyntaxNode>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<FSharp.Compiler.Syntax.SynExpr,Microsoft.FSharp.Core.FSharpOption`1<FSharp.Compiler.EditorServices.CompletionContext>>, [FSharp.Compiler.Service]FSharp.Compiler.Syntax.SynExpr)][offset 0x00000620][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x00000032][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x0000003B][found Char] Unexpected type on the stack.
[IL]: Error [StackUnexpected]: : <StartupCode$FSharp-Compiler-Service>.$ServiceLexing+clo@921-525::Invoke([FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<System.Tuple`3<FSharp.Compiler.Parser+token,int32,int32>,Microsoft.FSharp.Core.Unit>)][offset 0x00000064][found Char] Unexpected type on the stack.
Expand Down
Loading