Merge pull request #5731 from VSadov/optFixVb

Ported fix for #5530 to VB

src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.Analyzer.vb

@@ -73,7 +73,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
 
                 ' this is the top of eval stack
                 DeclareLocal(_empty, 0)
-                RecordVarWrite(_empty)
+                RecordDummyWrite(_empty)
             End Sub
 
             Public Shared Function Analyze(
@@ -576,16 +576,25 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                 ' Such call will push the receiver ref before the arguments
                 ' so we need to ensure that arguments cannot use stack temps
                 Dim leftType As TypeSymbol = left.Type
-                Dim cookie As Object = Nothing
+                Dim mayPushReceiver As Boolean = False
                 If right.Kind = BoundKind.ObjectCreationExpression Then
                     Dim ctor = DirectCast(right, BoundObjectCreationExpression).ConstructorOpt
                     If ctor IsNot Nothing AndAlso ctor.ParameterCount <> 0 Then
-                        cookie = GetStackStateCookie()
+                        mayPushReceiver = True
                     End If
                 End If
 
+                If mayPushReceiver Then
+                    'push unknown value just to prevent access to stack locals.
+                    PushEvalStack(Nothing, ExprContext.Address)
+                End If
+
                 right = VisitExpression(node.Right, rhsContext)
 
+                If mayPushReceiver Then
+                    PopEvalStack()
+                End If
+
                 ' if assigning to a local, now it is the time to record the Write
                 If storedAssignmentLocal IsNot Nothing Then
                     ' this assert will fire if code relies on implicit CLR coercions 
@@ -599,12 +608,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                     RecordVarWrite(storedAssignmentLocal.LocalSymbol)
                 End If
 
-                If cookie IsNot Nothing Then
-                    ' There is still RHS on the stack, adjust for that
-                    Me.PopEvalStack()
-                    EnsureStackState(cookie)
-                End If
-
                 Return node.Update(left, Nothing, right, node.SuppressObjectClone, node.Type)
             End Function
 
@@ -1147,7 +1150,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                 Dim dummy As New DummyLocal(Me._container)
                 Me._dummyVariables.Add(dummy, dummy)
                 Me._locals.Add(dummy, New LocalDefUseInfo(Me.StackDepth))
-                RecordVarWrite(dummy)
+                RecordDummyWrite(dummy)
 
                 Return dummy
             End Function
@@ -1166,7 +1169,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                     dummy = New DummyLocal(Me._container)
                     Me._dummyVariables.Add(label, dummy)
                     Me._locals.Add(dummy, New LocalDefUseInfo(Me.StackDepth))
-                    RecordVarWrite(dummy)
+                    RecordDummyWrite(dummy)
                 End If
             End Sub
 
@@ -1241,6 +1244,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
             End Function
 
             Private Sub RecordVarWrite(local As LocalSymbol)
+                Debug.Assert(local.SynthesizedKind <> SynthesizedLocalKind.OptimizerTemp)
+
                 If Not CanScheduleToStack(local) Then
                     Return
                 End If
@@ -1250,26 +1255,32 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                     Return
                 End If
 
-                ' if accessing real val, check stack
-                If Not TypeOf local Is DummyLocal Then
-
-                    ' -1 because real assignment "consumes" value.
-                    Dim evalStack = Me.StackDepth - 1
+                ' check stack
+                ' -1 because real assignment "consumes" value.
+                Dim evalStack = Me.StackDepth - 1
 
-                    If locInfo.StackAtDeclaration <> evalStack Then
-                        ' writing at different eval stack.
-                        locInfo.ShouldNotSchedule()
-                        Return
-                    End If
-                Else
-                    ' dummy must be accessed on same stack.
-                    Debug.Assert(local Is _empty OrElse locInfo.StackAtDeclaration = StackDepth())
+                If locInfo.StackAtDeclaration <> evalStack Then
+                    ' writing at different eval stack.
+                    locInfo.ShouldNotSchedule()
+                    Return
                 End If
 
                 Dim locDef = New LocalDefUseSpan(Me._counter)
                 locInfo.localDefs.Add(locDef)
             End Sub
 
+            Private Sub RecordDummyWrite(local As LocalSymbol)
+                Debug.Assert(local.SynthesizedKind = SynthesizedLocalKind.OptimizerTemp)
+
+                Dim locInfo = _locals(local)
+
+                ' dummy must be accessed on same stack.
+                Debug.Assert(local Is _empty OrElse locInfo.StackAtDeclaration = StackDepth())
+
+                Dim locDef = New LocalDefUseSpan(Me._counter)
+                locInfo.localDefs.Add(locDef)
+            End Sub
+
             Private Sub ShouldNotSchedule(local As LocalSymbol)
                 Dim localDefInfo As LocalDefUseInfo = Nothing
                 If _locals.TryGetValue(local, localDefInfo) Then

src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.LocalDefUseSpan.vb

@@ -39,37 +39,42 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
             End Function
 
             ''' <summary>
-            ''' That said, when current and other use spans are regular spans we can have 
-            ''' only 2 conflict cases:
-            '''      [1, 3) conflicts with [0, 2) 
-            '''      [1, 3) conflicts with [2, 4) 
+            ''' when current And other use spans are regular spans we can have only 2 conflict cases:
+            ''' [1, 3) conflicts with [0, 2)
+            ''' [1, 3) conflicts with [2, 4)
             ''' 
-            ''' specifically: 
-            '''      [1, 3) does not conflict with [0, 1) 
+            ''' NOTE: With regular spans, it is not possible for two spans to share an edge point 
+            ''' unless they belong to the same local. (because we cannot aceess two real locals at the same time)
             ''' 
-            ''' NOTE: with regular spans, it is not possible to have start1 == start2 or 
-            ''' end1 == end2 since at the same node we can access only one real local.
-            ''' 
-            ''' However at the same node we can access one or more dummy locals. So we can 
-            ''' have start1 == start2 and end1 == end2 scenarios, but only if the other span 
-            ''' is a span of a dummy. 
+            ''' specifically:
+            ''' [1, 3) does Not conflict with [0, 1)   since such spans would need to belong to the same local
             ''' </summary>
             Public Function ConflictsWith(other As LocalDefUseSpan) As Boolean
-                ' NOTE: this logic is moved from CS as-is
-                ' TODO: revise the definition of ConflictsWith
-                Dim containsStart As Boolean = other.ContainsStart(Me.Start)
-                Dim containsEnd = other.ContainsEnd(Me.End)
-                Return containsStart Xor containsEnd
+                Return Contains(other.Start) Xor Contains(other.End)
+            End Function
+
+            Private Function Contains(val As Integer) As Boolean
+                Return Me.Start < val AndAlso Me.End > val
             End Function
 
-            Private Function ContainsStart(otherStart As Integer) As Boolean
-                Return Me.Start <= otherStart AndAlso Me.End > otherStart
+            ''' <summary>
+            ''' Dummy locals represent implicit control flow
+            ''' it is not allowed for a regular local span to cross into or 
+            ''' be immediately adjacent to a dummy span.
+            ''' 
+            ''' specifically:
+            ''' [1, 3) does conflict with [0, 1)   since that would imply a value flowing into or out of a span surrounded by a branch/label
+            ''' 
+            ''' </summary>
+            Public Function ConflictsWithDummy(dummy As LocalDefUseSpan) As Boolean
+                Return Includes(dummy.Start) Xor Includes(dummy.End)
             End Function
 
-            Private Function ContainsEnd(otherEnd As Integer) As Boolean
-                Return Me.Start < otherEnd AndAlso Me.End > otherEnd
+            Private Function Includes(val As Integer) As Boolean
+                Return Me.Start <= val AndAlso Me.End >= val
             End Function
 
+
         End Class
 
     End Class

src/Compilers/VisualBasic/Portable/CodeGen/Optimizer/StackScheduler.vb

@@ -63,8 +63,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                 Next
             Next
 
-            ' TODO: perf. This can be simplified to not use a query.
+            Dim dummyCnt = defs.Count
 
+            ' TODO: perf. This can be simplified to not use a query.
             ' order locals by the number of usages, then by the declaration in descending order
             For Each localInfo In From i In info
                                   Where i.Value.localDefs.Count > 0
@@ -85,14 +86,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
                 For Each newDef In localInfo.Value.localDefs
                     Debug.Assert(Not intersects)
 
-                    ' TODO: This piece makes the whole thing O(n^2), revise
-                    For i = 0 To defs.Count - 1
-                        If newDef.ConflictsWith(defs(i)) Then
+                    For i = 0 To dummyCnt - 1
+                        If newDef.ConflictsWithDummy(defs(i)) Then
                             intersects = True
                             Exit For
                         End If
                     Next
 
+                    If Not intersects Then
+                        For i = dummyCnt To defs.Count - 1
+                            If newDef.ConflictsWith(defs(i)) Then
+                                intersects = True
+                                Exit For
+                            End If
+                        Next
+                    End If
+
                     If intersects Then
                         info.Remove(localInfo.Key)
                         Exit For

src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenTests.vb

@@ -13403,5 +13403,73 @@ End Structure
                 )
         End Sub
 
+
+        <Fact()>
+        Public Sub InplaceCtorUsesLocal()
+            Dim c = CompileAndVerify(
+<compilation>
+    <file name="a.vb">
+
+        <![CDATA[
+
+Module Module1
+    Private arr As S1() = New S1(1) {}
+
+    Structure S1
+        Public a, b As Integer
+
+        Public Sub New(a As Integer, b As Integer)
+            Me.a = a
+            Me.b = b
+        End Sub
+
+        Public Sub New(a As Integer)
+            Me.a = a
+        End Sub
+
+    End Structure
+
+    Sub Main()
+        Dim arg = System.Math.Max(1, 2)
+        Dim val = New S1(arg, arg)
+        arr(0) = val
+        System.Console.WriteLine(arr(0).a)
+    End Sub
+End Module
+
+]]>
+    </file>
+</compilation>, options:=TestOptions.ReleaseExe,
+                expectedOutput:="2")
+
+            c.VerifyIL("Module1.Main",
+            <![CDATA[
+{
+  // Code size       51 (0x33)
+  .maxstack  3
+  .locals init (Integer V_0, //arg
+                Module1.S1 V_1) //val
+  IL_0000:  ldc.i4.1
+  IL_0001:  ldc.i4.2
+  IL_0002:  call       "Function System.Math.Max(Integer, Integer) As Integer"
+  IL_0007:  stloc.0
+  IL_0008:  ldloca.s   V_1
+  IL_000a:  ldloc.0
+  IL_000b:  ldloc.0
+  IL_000c:  call       "Sub Module1.S1..ctor(Integer, Integer)"
+  IL_0011:  ldsfld     "Module1.arr As Module1.S1()"
+  IL_0016:  ldc.i4.0
+  IL_0017:  ldloc.1
+  IL_0018:  stelem     "Module1.S1"
+  IL_001d:  ldsfld     "Module1.arr As Module1.S1()"
+  IL_0022:  ldc.i4.0
+  IL_0023:  ldelema    "Module1.S1"
+  IL_0028:  ldfld      "Module1.S1.a As Integer"
+  IL_002d:  call       "Sub System.Console.WriteLine(Integer)"
+  IL_0032:  ret
+}
+]]>)
+        End Sub
+
     End Class
 End Namespace