fix regression in debug codegen

src/fsharp/CreateILModule.fs

@@ -270,7 +270,7 @@ module MainModuleBuilder =
             let isDLL = (tcConfig.target = CompilerTarget.Dll || tcConfig.target = CompilerTarget.Module)
             mkILSimpleModule assemblyName ilModuleName isDLL tcConfig.subsystemVersion tcConfig.useHighEntropyVA ilTypeDefs hashAlg locale flags (mkILExportedTypes exportedTypesList) metadataVersion
 
-        let disableJitOptimizations = not (tcConfig.optSettings.jitOpt())
+        let disableJitOptimizations = not tcConfig.optSettings.JitOptimizationsEnabled
 
         let tcVersion = tcConfig.version.GetVersionInfo(tcConfig.implicitIncludeDir)
 

src/fsharp/IlxGen.fs

@@ -194,7 +194,7 @@ type IlxGenOptions =
       mainMethodInfo: Attribs option
 
       /// Indicates if local optimizations are on
-      localOptimizationsAreOn: bool
+      localOptimizationsEnabled: bool
 
       /// Indicates if we are generating debug symbols
       generateDebugSymbols: bool
@@ -2099,7 +2099,6 @@ let CodeGenThen cenv mgbuf (entryPointInfo, methodName, eenv, alreadyUsedArgs, c
     let start = CG.GenerateMark cgbuf "mstart"
     let innerVals = entryPointInfo |> List.map (fun (v, kind) -> (v, (kind, start)))
 
-    (* Call the given code generator *)
     codeGenFunction cgbuf {eenv with withinSEH=false
                                      liveLocals=IntMap.empty()
                                      innerVals = innerVals}
@@ -5624,7 +5623,7 @@ and GenDecisionTreeSuccess cenv cgbuf inplabOpt stackAtTargets eenv es targetIdx
 
         // We have encountered this target before. See if we should generate it now
         let targetCount = targetCounts.[targetIdx]
-        let generateTargetNow = isTargetPostponed && cenv.opts.localOptimizationsAreOn && targetCount = 1 && targetNext.Value = targetIdx
+        let generateTargetNow = isTargetPostponed && cenv.opts.localOptimizationsEnabled && targetCount = 1 && targetNext.Value = targetIdx
         targetCounts.[targetIdx] <- targetCount - 1
 
         // If not binding anything we can go directly to the targetMarkBeforeBinds point
@@ -5683,7 +5682,7 @@ and GenDecisionTreeSuccess cenv cgbuf inplabOpt stackAtTargets eenv es targetIdx
         // In debug mode, postpone all decision tree targets to after the switching.
         // In release mode, if a target is the target of multiple incoming success nodes, postpone it to avoid 
         // making any backward branches
-        let generateTargetNow = cenv.opts.localOptimizationsAreOn && targetCount = 1 && targetNext.Value = targetIdx
+        let generateTargetNow = cenv.opts.localOptimizationsEnabled && targetCount = 1 && targetNext.Value = targetIdx
         targetCounts.[targetIdx] <- targetCount - 1
 
         let genTargetInfoOpt =
@@ -7083,7 +7082,7 @@ and AllocLocal cenv cgbuf eenv compgen (v, ty, isFixed) (scopeMarks: Mark * Mark
      let ranges = if compgen then [] else [(v, scopeMarks)]
      // Get an index for the local
      let j, realloc =
-        if cenv.opts.localOptimizationsAreOn then
+        if cenv.opts.localOptimizationsEnabled then
             cgbuf.ReallocLocal((fun i (_, ty', isFixed') -> not isFixed' && not isFixed && not (IntMap.mem i eenv.liveLocals) && (ty = ty')), ranges, ty, isFixed)
         else
             cgbuf.AllocLocal(ranges, ty, isFixed), false
@@ -7152,7 +7151,7 @@ and AllocTopValWithinExpr cenv cgbuf cloc scopeMarks v eenv =
     // decide whether to use a shadow local or not
     let useShadowLocal =
         cenv.opts.generateDebugSymbols &&
-        not cenv.opts.localOptimizationsAreOn &&
+        not cenv.opts.localOptimizationsEnabled &&
         not v.IsCompilerGenerated &&
         not v.IsMutable &&
         // Don't use shadow locals for things like functions which are not compiled as static values/properties

src/fsharp/IlxGen.fsi

@@ -31,7 +31,7 @@ type internal IlxGenOptions =
       mainMethodInfo: Attribs option
 
       /// Indicates if local optimizations are active
-      localOptimizationsAreOn: bool
+      localOptimizationsEnabled: bool
 
       /// Indicates if we are generating debug symbols or not
       generateDebugSymbols: bool

src/fsharp/OptimizeInputs.fs

@@ -173,7 +173,7 @@ let GenerateIlxCode
           workAroundReflectionEmitBugs=tcConfig.isInteractive // REVIEW: is this still required?
           generateDebugSymbols= tcConfig.debuginfo
           fragName = fragName
-          localOptimizationsAreOn= tcConfig.optSettings.localOpt ()
+          localOptimizationsEnabled= tcConfig.optSettings.LocalOptimizationsEnabled
           testFlagEmitFeeFeeAs100001 = tcConfig.testFlagEmitFeeFeeAs100001
           mainMethodInfo= mainMethodInfo
           ilxBackend = ilxBackend

src/fsharp/Optimizer.fs

@@ -349,59 +349,64 @@ type OptimizationSettings =
         }
 
     /// Determines if JIT optimizations are enabled
-    member x.jitOpt() = match x.jitOptUser with Some f -> f | None -> jitOptDefault
+    member x.JitOptimizationsEnabled = match x.jitOptUser with Some f -> f | None -> jitOptDefault
 
     /// Determines if intra-assembly optimization is enabled
-    member x.localOpt () = match x.localOptUser with Some f -> f | None -> localOptDefault
+    member x.LocalOptimizationsEnabled = match x.localOptUser with Some f -> f | None -> localOptDefault
 
     /// Determines if cross-assembly optimization is enabled
     member x.crossAssemblyOpt () =
-        x.localOpt () && 
+        x.LocalOptimizationsEnabled && 
         x.crossAssemblyOptimizationUser |> Option.defaultValue crossAssemblyOptimizationDefault
 
     /// Determines if we should keep optimization values
     member x.KeepOptimizationValues = x.crossAssemblyOpt ()
 
     /// Determines if we should inline calls
-    member x.InlineLambdas = x.localOpt ()  
+    member x.InlineLambdas = x.LocalOptimizationsEnabled  
 
     /// Determines if we should eliminate unused bindings with no effect 
-    member x.EliminateUnusedBindings = x.localOpt () 
+    member x.EliminateUnusedBindings = x.LocalOptimizationsEnabled 
 
     /// Determines if we should arrange things so we debug points for pipelines x |> f1 |> f2
     /// including locals "<pipe1-input>", "<pipe1-stage1>" and so on.
     /// On by default for debug code.
     member x.DebugPointsForPipeRight =
-        not (x.localOpt ()) &&
+        not x.LocalOptimizationsEnabled &&
         x.debugPointsForPipeRight |> Option.defaultValue debugPointsForPipeRightDefault
 
+    /// Determines if we should eliminate for-loops around an expr if it has no effect 
+    ///
+    /// This optimization is off by default, given tiny overhead of including try/with. See https://github.com/Microsoft/visualfsharp/pull/376
+    member x.EliminateForLoop = x.LocalOptimizationsEnabled
+
     /// Determines if we should eliminate try/with or try/finally around an expr if it has no effect 
     ///
     /// This optimization is off by default, given tiny overhead of including try/with. See https://github.com/Microsoft/visualfsharp/pull/376
     member _.EliminateTryWithAndTryFinally = false 
 
     /// Determines if we should eliminate first part of sequential expression if it has no effect 
-    member x.EliminateSequential = x.localOpt () 
+    member x.EliminateSequential = x.LocalOptimizationsEnabled 
 
     /// Determines if we should determine branches in pattern matching based on known information, e.g.
     /// eliminate a "if true then .. else ... "
-    member x.EliminateSwitch = x.localOpt () 
+    member x.EliminateSwitch = x.LocalOptimizationsEnabled
 
     /// Determines if we should eliminate gets on a record if the value is known to be a record with known info and the field is not mutable
-    member x.EliminateRecdFieldGet = x.localOpt () 
+    member x.EliminateRecdFieldGet = x.LocalOptimizationsEnabled
 
     /// Determines if we should eliminate gets on a tuple if the value is known to be a tuple with known info
-    member x.EliminateTupleFieldGet = x.localOpt () 
+    member x.EliminateTupleFieldGet = x.LocalOptimizationsEnabled
 
     /// Determines if we should eliminate gets on a union if the value is known to be that union case and the particular field has known info
-    member x.EliminateUnionCaseFieldGet () = x.localOpt () 
+    member x.EliminateUnionCaseFieldGet () = x.LocalOptimizationsEnabled
 
     /// Determines if we should eliminate non-compiler generated immediate bindings 
-    member x.EliminateImmediatelyConsumedLocals() = x.localOpt () 
+    member x.EliminateImmediatelyConsumedLocals() = x.LocalOptimizationsEnabled
 
     /// Determines if we should expand "let x = (exp1, exp2, ...)" bindings as prior tmps 
     /// Also if we should expand "let x = Some exp1" bindings as prior tmps 
-    member x.ExpandStructuralValues() = x.localOpt () 
+    member x.ExpandStructuralValues() = x.LocalOptimizationsEnabled 
 
 type cenv =
     { g: TcGlobals
@@ -2453,7 +2458,7 @@ and OptimizeFastIntegerForLoop cenv env (spStart, v, e1, dir, e2, e3, m) =
     let einfos = [e1info;e2info;e3info] 
     let eff = OrEffects einfos 
     (* neither bounds nor body has an effect, and loops always terminate, hence eliminate the loop *)
-    if not eff then 
+    if cenv.settings.EliminateForLoop && not eff then 
         mkUnit cenv.g m, { TotalSize=0; FunctionSize=0; HasEffect=false; MightMakeCriticalTailcall=false; Info=UnknownValue }
     else
         let exprR = mkFor cenv.g (spStart, v, e1R, dir, e2R, e3R, m) 
@@ -3333,7 +3338,7 @@ and OptimizeDebugPipeRights cenv env expr =
             xs0R 
             inputVals
             pipesExprR
-    expr, pipesInfo
+    expr, { pipesInfo with HasEffect=true}
 
 and OptimizeFSharpDelegateInvoke cenv env (invokeRef, f0, f0ty, tyargs, args, m) =
     let g = cenv.g
@@ -3529,7 +3534,7 @@ and OptimizeMatch cenv env (spMatch, exprm, dtree, targets, m, ty) =
 
 and OptimizeMatchPart2 cenv (spMatch, exprm, dtreeR, targetsR, dinfo, tinfos, m, ty) =
     let newExpr, newInfo = RebuildOptimizedMatch (spMatch, exprm, m, ty, dtreeR, targetsR, dinfo, tinfos)
-    let newExpr2 = if not (cenv.settings.localOpt()) then newExpr else CombineBoolLogic newExpr
+    let newExpr2 = if not cenv.settings.LocalOptimizationsEnabled then newExpr else CombineBoolLogic newExpr
     newExpr2, newInfo
 
 and CombineMatchInfos dinfo tinfo = 
@@ -3754,7 +3759,7 @@ and OptimizeModuleExpr cenv env x =
 
         let _renaming, hidden as rpi = ComputeRemappingFromImplementationToSignature cenv.g def mty 
         let def = 
-            if not (cenv.settings.localOpt()) then def else 
+            if not cenv.settings.LocalOptimizationsEnabled then def else 
 
             let fvs = freeInModuleOrNamespace CollectLocals def 
             let dead = 

src/fsharp/Optimizer.fsi

@@ -43,9 +43,9 @@ type OptimizationSettings =
     }
 
 
-    member jitOpt: unit -> bool 
+    member JitOptimizationsEnabled: bool 
 
-    member localOpt: unit -> bool 
+    member LocalOptimizationsEnabled: bool 
 
     static member Defaults: OptimizationSettings
 

src/fsharp/fsi/fsi.fs

@@ -1114,7 +1114,7 @@ type internal FsiDynamicCompiler
 
     let valuePrinter = FsiValuePrinter(fsi, tcConfigB, tcGlobals, generateDebugInfo, resolveAssemblyRef, outWriter)
 
-    let assemblyBuilder,moduleBuilder = mkDynamicAssemblyAndModule (assemblyName, tcConfigB.optSettings.localOpt(), generateDebugInfo, fsiCollectible)
+    let assemblyBuilder,moduleBuilder = mkDynamicAssemblyAndModule (assemblyName, tcConfigB.optSettings.LocalOptimizationsEnabled, generateDebugInfo, fsiCollectible)
 
     let rangeStdin = rangeN stdinMockFilename 0
 

tests/fsharp/Compiler/CodeGen/EmittedIL/BooleanLogic.fs

@@ -6,10 +6,10 @@ open FSharp.Test
 open NUnit.Framework
 
 [<TestFixture>]
-module ``BooleanLogic`` =
+module BooleanLogic =
 
     [<Test>]
-    let ``BooleanOrs``() =
+    let BooleanOrs() =
         CompilerAssert.CompileLibraryAndVerifyILWithOptions [|"-g"; "--optimize+"|]
             """
 module BooleanOrs
@@ -54,3 +54,89 @@ let compute (x: int) =
             """
             ])
 
+[<TestFixture>]
+// We had a regression in debug code regression where we were falsely marking pipelines
+// as non-side-effecting, causing them to be eliminated in loops.
+//
+// After the fix 
+//   1. pipelines are correctly marked as having effect
+//   2. we don't eliminate loops anyway
+module DontEliminateForLoopsInDebugCode =
+
+    [<Test>]
+    // See https://github.com/dotnet/fsharp/pull/12021
+    let Regression12021() =
+        CompilerAssert.CompileLibraryAndVerifyILWithOptions [|"-g"; "--optimize-"|]
+            """
+module DontEliminateForLoops
+
+let unsolved = [true]
+let ApplyDefaults () = 
+
+        for priority = 0 to 10 do
+            unsolved |> List.iter (fun tp -> System.Console.WriteLine())
+
+            """
+            (fun verifier -> verifier.VerifyIL [
+            """
+.method public static void  ApplyDefaults() cil managed
+{
+
+  .maxstack  5
+  .locals init (int32 V_0,
+           class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool> V_1,
+           class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<bool,class [FSharp.Core]Microsoft.FSharp.Core.Unit> V_2,
+           class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool> V_3,
+           class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool> V_4,
+           class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool> V_5,
+           bool V_6)
+  IL_0000:  ldc.i4.0
+  IL_0001:  stloc.0
+  IL_0002:  br.s       IL_004b
+
+  IL_0004:  call       class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool> DontEliminateForLoops::get_unsolved()
+  IL_0009:  stloc.1
+  IL_000a:  ldsfld     class DontEliminateForLoops/ApplyDefaults@8 DontEliminateForLoops/ApplyDefaults@8::@_instance
+  IL_000f:  stloc.2
+  IL_0010:  ldloc.1
+  IL_0011:  stloc.3
+  IL_0012:  ldloc.3
+  IL_0013:  stloc.s    V_4
+  IL_0015:  ldloc.s    V_4
+  IL_0017:  call       instance class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<!0> class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool>::get_TailOrNull()
+  IL_001c:  stloc.s    V_5
+  IL_001e:  ldloc.s    V_5
+  IL_0020:  ldnull
+  IL_0021:  cgt.un
+  IL_0023:  brfalse.s  IL_0047
+
+  IL_0025:  ldloc.s    V_4
+  IL_0027:  call       instance !0 class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool>::get_HeadOrDefault()
+  IL_002c:  stloc.s    V_6
+  IL_002e:  ldloc.2
+  IL_002f:  ldloc.s    V_6
+  IL_0031:  callvirt   instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2<bool,class [FSharp.Core]Microsoft.FSharp.Core.Unit>::Invoke(!0)
+  IL_0036:  pop
+  IL_0037:  ldloc.s    V_5
+  IL_0039:  stloc.s    V_4
+  IL_003b:  ldloc.s    V_4
+  IL_003d:  call       instance class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<!0> class [FSharp.Core]Microsoft.FSharp.Collections.FSharpList`1<bool>::get_TailOrNull()
+  IL_0042:  stloc.s    V_5
+  IL_0044:  nop
+  IL_0045:  br.s       IL_001e
+
+  IL_0047:  ldloc.0
+  IL_0048:  ldc.i4.1
+  IL_0049:  add
+  IL_004a:  stloc.0
+  IL_004b:  ldloc.0
+  IL_004c:  ldc.i4.1
+  IL_004d:  ldc.i4.s   10
+  IL_004f:  add
+  IL_0050:  blt.s      IL_0004
+
+  IL_0052:  ret
+} 
+            """
+            ])
+