Stabilize parallel optimizer Val iteration for deterministic synthetic names

.gitignore

@@ -161,3 +161,4 @@ tests/projects/CompilerCompat/local-nuget-packages/
 tests/projects/CompilerCompat/lib-output-*/
 tests/projects/CompilerCompat/**/bin/
 tests/projects/CompilerCompat/**/obj/
+.scratch/

azure-pipelines-PR.yml

@@ -100,7 +100,7 @@ stages:
         helixRepo: dotnet/fsharp
         jobs:
           # Determinism, we want to run it only in PR builds
-        - job: Determinism_Debug
+        - job: Determinism_Release
           condition: eq(variables['Build.Reason'], 'PullRequest')
           variables:
           - name: _SignType
@@ -109,13 +109,6 @@ stages:
             name: $(DncEngPublicBuildPool)
             demands: ImageOverride -equals $(_WindowsMachineQueueName)
           timeoutInMinutes: 90
-          strategy:
-            maxParallel: 2
-            matrix:
-              regular:
-                _experimental_flag: ''
-              experimental_features:
-                _experimental_flag: ''
           steps:
           - checkout: self
             clean: true
@@ -129,15 +122,15 @@ stages:
               workingDirectory: $(Build.SourcesDirectory)
               installationPath: $(Build.SourcesDirectory)/.dotnet
           - script: .\eng\common\dotnet.cmd
-          - script: .\eng\test-determinism.cmd -configuration Debug
-            env:
-              FSHARP_EXPERIMENTAL_FEATURES: $(_experimental_flag)
-            displayName: Determinism tests with Debug configuration
+          - script: .\eng\test-determinism.cmd -configuration Release
+            displayName: Determinism tests (race detector — same flags both builds)
+          - script: .\eng\test-determinism.cmd -configuration Release -mode seq-vs-par
+            displayName: Determinism tests (1-shot diff — sequential vs parallel)
           - task: PublishPipelineArtifact@1
             displayName: Publish Determinism Logs
             inputs:
-              targetPath: '$(Build.SourcesDirectory)/artifacts/log/Debug'
-              artifactName: 'Determinism_Debug Attempt $(System.JobAttempt) Logs'
+              targetPath: '$(Build.SourcesDirectory)/artifacts/log/Release'
+              artifactName: 'Determinism_Release Attempt $(System.JobAttempt) Logs'
             continueOnError: true
             condition: not(succeeded())
 

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -1,7 +1,10 @@
 ### Fixed
 
+* Stabilize codegen order under `--parallelcompilation+` so `--deterministic` Release builds produce byte-identical IL across rebuilds: optimizer Val iteration, IlxGen type/method/field/event emit order, anonymous-record extra-binding drain, and `FileIndex` assignment now follow source position rather than thread-scheduling order. ([Issue #19732](https://github.com/dotnet/fsharp/issues/19732), [PR #19810](https://github.com/dotnet/fsharp/pull/19810))
 * Reject non-function bindings for single-case and partial active pattern names with FS1209, matching the existing multi-case behavior. ([PR #19763](https://github.com/dotnet/fsharp/pull/19763))
 * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811))
+* Stabilize codegen order under `--parallelcompilation+`: optimizer Val iteration (`DetupleArgs`, `InnerLambdasToTopLevelFuncs`), `IlxGen.TypeDefsBuilder` type-emit order, per-`TypeDefBuilder` method/field/event order, anonymous-record extra-binding drain order, and `FileIndex` assignment during parallel parsing now follow source position rather than thread-scheduling order. The Release `Determinism` CI job (which builds the compiler twice and compares MD5 hashes) covers this end-to-end; a new in-process differential test asserts that `--parallelcompilation+` and `--parallelcompilation-` produce byte-identical IL. ([Issue #19732](https://github.com/dotnet/fsharp/issues/19732), [PR #19810](https://github.com/dotnet/fsharp/pull/19810))
+* Improve determinism under parallel optimization and code generation: optimizer Val iteration, IlxGen TypeDefsBuilder emit order, anonymous-record extra-binding drain order, and FileIndex assignment during parallel parsing are all now stable across builds. This makes Release MVID reproducible. ([Issue #19732](https://github.com/dotnet/fsharp/issues/19732), [PR #19810](https://github.com/dotnet/fsharp/pull/19810))
 * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776))
 * Fix `[<return: X>]` prefix attributes being silently dropped on class members, and fix false-positive `AllowMultiple=false` errors when `[<X>]` and `[<return: X>]` are applied to the same binding. ([Issue #17904](https://github.com/dotnet/fsharp/issues/17904), [Issue #19020](https://github.com/dotnet/fsharp/issues/19020), [PR #19738](https://github.com/dotnet/fsharp/pull/19738))
 * Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714))

eng/test-determinism.ps1

@@ -2,6 +2,8 @@
 param([string]$configuration = "Debug",
   [string]$msbuildEngine = "vs",
   [string]$altRootDrive = "q:",
+  [ValidateSet("same", "seq-vs-par")]
+  [string]$mode = "same",
   [switch]$help,
   [switch]$norestore,
   [switch]$rebuild)
@@ -15,6 +17,8 @@ function Print-Usage() {
   Write-Host "  -msbuildEngine <value>    Msbuild engine to use to run build ('dotnet', 'vs', or unspecified)."
   Write-Host "  -bootstrapDir             Directory containing the bootstrap compiler"
   Write-Host "  -altRootDrive             The drive we build on (via subst) for verifying pathmap implementation"
+  Write-Host "  -mode <value>             'same' (default): build twice with identical flags (race-detector)"
+  Write-Host "                            'seq-vs-par': first build sequential, second build parallel (deterministic 1-shot diff)"
 }
 
 if ($help) {
@@ -25,7 +29,7 @@ if ($help) {
 # List of binary names that should be skipped because they have a known issue that
 # makes them non-deterministic.
 $script:skipList = @()
-function Run-Build([string]$rootDir, [string]$increment) {
+function Run-Build([string]$rootDir, [string]$increment, [string]$additionalFscFlags = "") {
 
   $logFileName = $increment
 
@@ -62,6 +66,9 @@ function Run-Build([string]$rootDir, [string]$increment) {
   Stop-Processes
 
   Write-Host "Building $solution using $bootstrapDir into '$increment' $incrementDir"
+  if ($additionalFscFlags -ne "") {
+    Write-Host "  AdditionalFscCmdFlags = '$additionalFscFlags'"
+  }
   MSBuild $toolsetBuildProj `
     /p:Configuration=$configuration `
     /p:Projects=$solution `
@@ -86,6 +93,7 @@ function Run-Build([string]$rootDir, [string]$increment) {
     /p:RunAnalyzers=false `
     /p:RunAnalyzersDuringBuild=false `
     /p:BUILDING_USING_DOTNET=false `
+    /p:AdditionalFscCmdFlags="$additionalFscFlags" `
     /bl:$logFilePath
 
   Write-Host "Copy-Item -Path $binDir -Destination $incrementDir -ErrorAction SilentlyContinue -Recurse"
@@ -202,9 +210,9 @@ function Test-MapContents($dataMap) {
   }
 }
 
-function Test-Build([string]$rootDir, $dataMap, [string]$increment) {
+function Test-Build([string]$rootDir, $dataMap, [string]$increment, [string]$additionalFscFlags = "") {
   $logFileName = $increment
-  Run-Build $rootDir -increment $increment
+  Run-Build $rootDir -increment $increment -additionalFscFlags $additionalFscFlags
 
   $errorList = @()
   $allGood = $true
@@ -273,14 +281,31 @@ function Test-Build([string]$rootDir, $dataMap, [string]$increment) {
 }
 
 function Run-Test() {
+  $seqFlags = ""
+  $parFlags = ""
+  if ($mode -eq "seq-vs-par") {
+    # First build: sequential (force single-threaded, disable any parallel test paths)
+    $seqFlags = "--parallelcompilation- --test:ParallelOff --nowarn:75"
+    # Second build: parallel (default in modern fsc, made explicit for clarity)
+    $parFlags = "--parallelcompilation+"
+    Write-Host "Determinism mode: seq-vs-par"
+    Write-Host "  Initial (seq): $seqFlags"
+    Write-Host "  Test1   (par): $parFlags"
+  }
+  else {
+    Write-Host "Determinism mode: same (race-detector; both builds identical)"
+  }
+
   # Run the initial build so that we can populate the maps
-  Run-Build $RepoRoot -increment "Initial" -useBootstrap
+  Run-Build $RepoRoot -increment "Initial" -additionalFscFlags $seqFlags
 
   $dataMap = Record-Binaries $RepoRoot "Initial"
   Test-MapContents $dataMap
 
-  # Run a test against the source in the same directory location
-  Test-Build -rootDir $RepoRoot -dataMap $dataMap -increment "Test1"
+  # Run a test against the source in the same directory location.
+  # In 'same' mode: same flags as Initial (probabilistic race detector).
+  # In 'seq-vs-par' mode: parallel flags, contrasting against the sequential Initial.
+  Test-Build -rootDir $RepoRoot -dataMap $dataMap -increment "Test1" -additionalFscFlags $parFlags
 
   # Run another build in a different source location and verify that path mapping
   # allows the build to be identical.  To do this we'll copy the entire source

src/Compiler/Driver/OptimizeInputs.fs

@@ -437,14 +437,23 @@ let ApplyAllOptimizations
     if tcConfig.extraOptimizationIterations > 0 then
         addPhase "ExtraLoop" extraLoop
 
+    // A per-file naming scope is created at this per-file optimization boundary so that
+    // compiler-generated names from the Detuple and TLR passes are bucketed by the consumer
+    // file currently being optimized, rather than by the (possibly inlined) source range of
+    // each value. This keeps those names deterministic under parallel optimization.
+    // See https://github.com/dotnet/fsharp/issues/19732.
+    let mkFileNamingScope (file: CheckedImplFile) =
+        tcGlobals.CompilerGlobalState.Value.NiceNameGenerator.NewFileScope(file.QualifiedNameOfFile.Range)
+
     let detuple
         ({
              File = file
              PrevPhase = prevPhase
              PrevFile = _prevFile
          }: PhaseInputs)
         : PhaseRes =
-        let file = file |> Detuple.DetupleImplFile ccu tcGlobals
+        let scope = mkFileNamingScope file
+        let file = file |> Detuple.DetupleImplFile scope ccu tcGlobals
         file, prevPhase
 
     if tcConfig.doDetuple then
@@ -457,9 +466,11 @@ let ApplyAllOptimizations
              PrevFile = _prevFile
          }: PhaseInputs)
         : PhaseRes =
+        let scope = mkFileNamingScope file
+
         let file =
             file
-            |> InnerLambdasToTopLevelFuncs.MakeTopLevelRepresentationDecisions ccu tcGlobals
+            |> InnerLambdasToTopLevelFuncs.MakeTopLevelRepresentationDecisions scope ccu tcGlobals
 
         file, prevPhase
 
@@ -511,8 +522,12 @@ let ApplyAllOptimizations
 
     let results, optEnvFirstLoop =
         match tcConfig.optSettings.processingMode with
-        // Parallel optimization breaks determinism - turn it off in deterministic builds.
         | Optimizer.OptimizationProcessingMode.Parallel ->
+            // Determinism under Parallel mode relies on the per-pass sorts in
+            // DetupleArgs.determineTransforms and InnerLambdasToTopLevelFuncs.CreateNewValuesForTLR
+            // (via valSourceOrderKey). Any new pass calling NiceNameGenerator from a
+            // parallel optimizer phase must sort its Val collection the same way.
+            // See https://github.com/dotnet/fsharp/issues/19732.
             let results, optEnvFirstPhase =
                 ParallelOptimization.optimizeFilesInParallel optEnv phases implFiles
 

src/Compiler/Driver/ParseAndCheckInputs.fs

@@ -736,6 +736,13 @@ let ParseInputFilesInParallel (tcConfig: TcConfig, lexResourceManager, sourceFil
     for fileName in sourceFiles do
         checkInputFile tcConfig fileName
 
+    // Pre-register FileIndex values in source-file order. Without this, parallel
+    // parsing races for indices via fileIndexOfFile -> FileIndexTable lock,
+    // producing non-deterministic FileIndex assignments that leak into IL
+    // (via debug info, NiceNameGenerator keys, and sort orders downstream).
+    for fileName in sourceFiles do
+        FileIndex.fileIndexOfFile fileName |> ignore
+
     let sourceFiles = List.zip sourceFiles isLastCompiland
 
     UseMultipleDiagnosticLoggers (sourceFiles, delayLogger, None) (fun sourceFilesWithDelayLoggers ->

src/Compiler/Optimize/DetupleArgs.fs

@@ -5,6 +5,7 @@ module internal FSharp.Compiler.Detuple
 open Internal.Utilities.Collections
 open Internal.Utilities.Library
 open FSharp.Compiler.DiagnosticsLogger
+open FSharp.Compiler.CompilerGlobalState
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.TcGlobals
 open FSharp.Compiler.Text
@@ -496,7 +497,7 @@ type Transform =
 // transform - mkTransform - decided, create necessary stuff
 //-------------------------------------------------------------------------
 
-let mkTransform g (f: Val) m tps x1Ntys retTy (callPattern, tyfringes: (TType list * Val list) list) =
+let mkTransform (scope: PerFileNamingScope) g (f: Val) m tps x1Ntys retTy (callPattern, tyfringes: (TType list * Val list) list) =
     // Create formal choices for x1...xp under callPattern
     let transformedFormals =
         (callPattern, tyfringes)
@@ -547,12 +548,12 @@ let mkTransform g (f: Val) m tps x1Ntys retTy (callPattern, tyfringes: (TType li
     let fCty = mkLambdaTy g tps argTys retTy
 
     let transformedVal =
-        // Ensure that we have an g.CompilerGlobalState
-        assert (g.CompilerGlobalState |> Option.isSome)
-
+        // Names are bucketed by the per-file optimization scope (not by f.Range, which may point at
+        // inlined source from another file) to keep compiler-generated names deterministic under
+        // parallel optimization. f.Range is still used as the Val's source location below.
         mkLocalVal
             f.Range
-            (g.CompilerGlobalState.Value.NiceNameGenerator.FreshCompilerGeneratedName(f.LogicalName, f.Range))
+            (scope.Fresh(f.LogicalName, f.Range))
             fCty
             valReprInfo
 
@@ -638,7 +639,7 @@ let decideFormalSuggestedCP g z tys vss =
 // transform - decideTransform
 //-------------------------------------------------------------------------
 
-let decideTransform g z v callPatterns (m, tps, vss: Val list list, retTy) =
+let decideTransform (scope: PerFileNamingScope) g z v callPatterns (m, tps, vss: Val list list, retTy) =
     let tys = List.map (typeOfLambdaArg m) vss
 
     // NOTE: 'a in arg types may have been instanced at different tuples...
@@ -664,7 +665,7 @@ let decideTransform g z v callPatterns (m, tps, vss: Val list list, retTy) =
     if isTrivialCP callPattern then
         None // no transform
     else
-        Some(v, mkTransform g v m tps tys retTy (callPattern, tyfringes))
+        Some(v, mkTransform scope g v m tps tys retTy (callPattern, tyfringes))
 
 
 //-------------------------------------------------------------------------
@@ -686,7 +687,7 @@ let eligibleVal g m (v: Val) =
     && not //  .IsCompiledAsTopLevel &&
         v.IsCompiledAsTopLevel
 
-let determineTransforms g (z: Results) =
+let determineTransforms (scope: PerFileNamingScope) g (z: Results) =
     let selectTransform (f: Val) sites =
         if not (eligibleVal g f.Range f) then
             None
@@ -702,9 +703,13 @@ let determineTransforms g (z: Results) =
                 | arg1 :: _ -> // consider f
                     let m = arg1.Range // mark of first arg, mostly for error reporting
                     let callPatterns = sitesCPs sites // callPatterns from sites
-                    decideTransform g z f callPatterns (m, tps, vss, retTy) // make transform (if required)
+                    decideTransform scope g z f callPatterns (m, tps, vss, retTy) // make transform (if required)
 
-    let vtransforms = Zmap.chooseL selectTransform z.Uses
+    // See https://github.com/dotnet/fsharp/issues/19732 for why we sort here.
+    let vtransforms =
+        Zmap.toList z.Uses
+        |> List.sortWith (fun (v1, _) (v2, _) -> compare (valSourceOrderKey v1) (valSourceOrderKey v2))
+        |> List.choose (fun (f, sites) -> selectTransform f sites)
     let vtransforms = Zmap.ofList valOrder vtransforms
     vtransforms
 
@@ -948,12 +953,12 @@ let passImplFile penv assembly =
 // entry point
 //-------------------------------------------------------------------------
 
-let DetupleImplFile ccu g expr =
+let DetupleImplFile (scope: PerFileNamingScope) ccu g expr =
     // Collect expr info - wanting usage contexts and bindings
     let z = GetUsageInfoOfImplFile g expr
 
     // For each Val, decide Some "transform", or None if not changing
-    let vtrans = determineTransforms g z
+    let vtrans = determineTransforms scope g z
 
     // Pass over term, rewriting bindings and fixing up call sites, under penv
     let penv =

src/Compiler/Optimize/DetupleArgs.fsi

@@ -3,10 +3,11 @@
 module internal FSharp.Compiler.Detuple
 
 open Internal.Utilities.Collections
+open FSharp.Compiler.CompilerGlobalState
 open FSharp.Compiler.TcGlobals
 open FSharp.Compiler.TypedTree
 
-val DetupleImplFile: CcuThunk -> TcGlobals -> CheckedImplFile -> CheckedImplFile
+val DetupleImplFile: PerFileNamingScope -> CcuThunk -> TcGlobals -> CheckedImplFile -> CheckedImplFile
 
 module GlobalUsageAnalysis =
     val GetValsBoundInExpr: Expr -> Zset<Val>

src/Compiler/Optimize/InnerLambdasToTopLevelFuncs.fs

@@ -818,7 +818,7 @@ let ChooseReqdItemPackings g fclassM topValS  declist reqdItemsMap =
 // REVIEW: could do better here by preserving names
 let MakeSimpleArityInfo tps n = ValReprInfo (ValReprInfo.InferTyparInfo tps, List.replicate n ValReprInfo.unnamedTopArg, ValReprInfo.unnamedRetVal)
 
-let CreateNewValuesForTLR g tlrS arityM fclassM envPackM =
+let CreateNewValuesForTLR (scope: PerFileNamingScope) g tlrS arityM fclassM envPackM =
 
     let createFHat (f: Val) =
         let wf = Zmap.force f arityM ("createFHat - wf", (valL >> showL))
@@ -837,14 +837,18 @@ let CreateNewValuesForTLR g tlrS arityM fclassM envPackM =
         let fHatArity = MakeSimpleArityInfo newTps (envp.ep_aenvs.Length + wf)
 
         let fHatName =
-            // Ensure that we have an g.CompilerGlobalState
-            assert(g.CompilerGlobalState |> Option.isSome)
-            g.CompilerGlobalState.Value.NiceNameGenerator.FreshCompilerGeneratedName(name, m)
+            // Names are bucketed by the per-file optimization scope (not by m, which may point at
+            // inlined source from another file) to keep compiler-generated names deterministic under
+            // parallel optimization. m is still used as the new Val's source location below.
+            scope.Fresh(name, m)
 
         let fHat = mkLocalNameTypeArity f.IsCompilerGenerated m fHatName fHatTy (Some fHatArity)
         fHat
 
-    let fs = Zset.elements tlrS
+    // See https://github.com/dotnet/fsharp/issues/19732 for why we sort here.
+    let fs =
+        Zset.elements tlrS
+        |> List.sortWith (fun v1 v2 -> compare (valSourceOrderKey v1) (valSourceOrderKey v2))
     let ffHats = List.map (fun f -> f, createFHat f) fs
     let fHatM = Zmap.ofList valOrder ffHats
     fHatM
@@ -1345,7 +1349,7 @@ let RecreateUniqueBounds g expr =
 // entry point
 //-------------------------------------------------------------------------
 
-let MakeTopLevelRepresentationDecisions ccu g expr =
+let MakeTopLevelRepresentationDecisions (scope: PerFileNamingScope) ccu g expr =
    try
       // pass1: choose the f to be TLR with arity(f)
       let tlrS, topValS, arityM = Pass1_DetermineTLRAndArities.DetermineTLRAndArities g expr
@@ -1355,7 +1359,7 @@ let MakeTopLevelRepresentationDecisions ccu g expr =
 
       // pass3
       let envPackM = ChooseReqdItemPackings g fclassM topValS  declist reqdItemsMap
-      let fHatM = CreateNewValuesForTLR g tlrS arityM fclassM envPackM
+      let fHatM = CreateNewValuesForTLR scope g tlrS arityM fclassM envPackM
 
       // pass4: rewrite
       if verboseTLR then dprintf "TransExpr(rw)------\n"

src/Compiler/Optimize/InnerLambdasToTopLevelFuncs.fsi

@@ -2,7 +2,9 @@
 
 module internal FSharp.Compiler.InnerLambdasToTopLevelFuncs
 
+open FSharp.Compiler.CompilerGlobalState
 open FSharp.Compiler.TypedTree
 open FSharp.Compiler.TcGlobals
 
-val MakeTopLevelRepresentationDecisions: CcuThunk -> TcGlobals -> CheckedImplFile -> CheckedImplFile
+val MakeTopLevelRepresentationDecisions:
+    PerFileNamingScope -> CcuThunk -> TcGlobals -> CheckedImplFile -> CheckedImplFile