Skip to content
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ positive.exe

# Test result files
tests/**/TestResults/*.trx
TestResults/*.trx

# Standard output/error files in root directory
StandardOutput.txt
Expand Down
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/10.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

### Fixed

* Fix F# compiler to prevent tail call emission when pinned locals are present ([PR #XXXX](https://github.com/dotnet/fsharp/pull/XXXX))
* Fix SignatureHash to include constant values in hash computation ([Issue #18758](https://github.com/dotnet/fsharp/issues/18758))
* Fix parsing errors using anonymous records and units of measures ([PR #18543](https://github.com/dotnet/fsharp/pull/18543))
* Fix parsing errors using anonymous records and code quotations ([PR #18603](https://github.com/dotnet/fsharp/pull/18603))
Expand Down
22 changes: 21 additions & 1 deletion src/Compiler/Checking/TailCallChecks.fs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ type cenv =

/// Values in module that have been marked [<TailCall>]
mustTailCall: Zset<Val>

/// Indicates whether the current method has pinned locals that would prevent tail calls
hasPinnedLocals: bool
}

override x.ToString() = "<cenv>"
Expand Down Expand Up @@ -202,6 +205,7 @@ let CheckForNonTailRecCall (cenv: cenv) expr (tailCall: TailCall) =
&& not (IsValRefIsDllImport cenv.g vref)
&& not isCCall
&& not hasByrefArg
&& not cenv.hasPinnedLocals

noTailCallBlockers // blockers that will prevent the IL level from emitting a tail instruction
else
Expand Down Expand Up @@ -730,11 +734,26 @@ and CheckBinding cenv alwaysCheckNoReraise ctxt (TBind(v, bindRhs, _) as bind) :
| Some info -> info
| _ -> ValReprInfo.emptyValData

// Check if this binding introduces a pinned local
let cenv =
if v.IsFixed then
{ cenv with hasPinnedLocals = true }
else
cenv

CheckLambdas isTop (Some v) cenv v.ShouldInline valReprInfo tailCall alwaysCheckNoReraise bindRhs v.Range v.Type ctxt

and CheckBindings cenv binds =
for bind in binds do
CheckBinding cenv false PermitByRefExpr.Yes bind
let (TBind(v, _, _)) = bind
// Update the environment if this binding is fixed
let currentCenv =
if v.IsFixed then
{ cenv with hasPinnedLocals = true }
else
cenv

CheckBinding currentCenv false PermitByRefExpr.Yes bind

let CheckModuleBinding cenv (isRec: bool) (TBind _ as bind) =

Expand Down Expand Up @@ -871,6 +890,7 @@ let CheckImplFile (g: TcGlobals, amap, reportErrors, implFileContents) =
stackGuard = StackGuard(PostInferenceChecksStackGuardDepth, "CheckImplFile")
amap = amap
mustTailCall = Zset.empty valOrder
hasPinnedLocals = false
}

CheckDefnInModule cenv implFileContents
13 changes: 12 additions & 1 deletion src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2689,6 +2689,10 @@ type CodeGenBuffer(m: range, mgbuf: AssemblyBuilder, methodName, alreadyUsedArgs
j, true
| None -> cgbuf.AllocLocal(ranges, ty, isFixed, canBeReallocd), false

/// Check if any locals have been allocated as pinned/fixed
member _.HasPinnedLocals() =
locals |> Seq.exists (fun (_, _, isFixed, _) -> isFixed)

member _.Close() =

let instrs = codebuf.ToArray()
Expand Down Expand Up @@ -4371,6 +4375,7 @@ and GenApp (cenv: cenv) cgbuf eenv (f, fty, tyargs, curriedArgs, m) sequel =
isDllImport,
isSelfInit,
makesNoCriticalTailcalls,
cgbuf,
sequel
)
else
Expand Down Expand Up @@ -4482,11 +4487,15 @@ and CanTailcall
isDllImport,
isSelfInit,
makesNoCriticalTailcalls,
cgbuf: CodeGenBuffer,
sequel
) =

// Can't tailcall with a struct object arg since it involves a byref
// Can't tailcall with a .NET 2.0 generic constrained call since it involves a byref
// Can't tailcall when there are pinned locals since the stack frame must remain alive
let hasPinnedLocals = cgbuf.HasPinnedLocals()

if
not hasStructObjArg
&& Option.isNone ccallInfo
Expand All @@ -4495,6 +4504,7 @@ and CanTailcall
&& not isDllImport
&& not isSelfInit
&& not makesNoCriticalTailcalls
&& not hasPinnedLocals
&&

// We can tailcall even if we need to generate "unit", as long as we're about to throw the value away anyway as par of the return.
Expand Down Expand Up @@ -4693,7 +4703,7 @@ and GenIndirectCall cenv cgbuf eenv (funcTy, tyargs, curriedArgs, m) sequel =
check ilxClosureApps

let isTailCall =
CanTailcall(false, None, eenv.withinSEH, hasByrefArg, false, false, false, false, sequel)
CanTailcall(false, None, eenv.withinSEH, hasByrefArg, false, false, false, false, cgbuf, sequel)

CountCallFuncInstructions()

Expand Down Expand Up @@ -5431,6 +5441,7 @@ and GenILCall
isDllImport,
false,
makesNoCriticalTailcalls,
cgbuf,
sequel
)

Expand Down
41 changes: 41 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/EmittedIL/TailCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,44 @@ let run() = let mutable x = 0 in foo(&x,&x,5)
"""
]

[<Fact>]
let ``TailCall 06 - No tail call with pinned locals``() =
FSharp """
module TailCall06
let foo(x:int, y) = printfn "%d" x
let run() =
use ptr = fixed [| 1; 2; 3 |]
foo(42, 5)
"""
|> compileWithTailCalls
|> shouldSucceed
|> verifyIL [
"""
IL_0040: ldc.i4.s 42
IL_0042: ldc.i4.5
IL_0043: call void TailCall06::foo<int32>(int32,
!!0)
"""
]

[<Fact>]
let ``TailCall 07 - No tail call with pinned locals in recursive function``() =
FSharp """
module TailCall07
let rec countdown n =
use ptr = fixed [| n |]
if n <= 0 then
0
else
countdown (n - 1)
"""
|> compileWithTailCalls
|> shouldSucceed
|> verifyIL [
"""
.locals init (native int V_0,
int32[] V_1,
int32& pinned V_2)
"""
]

Loading