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

OSR stress failure in System.Diagnostics.DiagnosticsSource.Tests #67078

Closed
AndyAyersMS opened this issue Mar 24, 2022 · 3 comments · Fixed by #67131
Closed

OSR stress failure in System.Diagnostics.DiagnosticsSource.Tests #67078

AndyAyersMS opened this issue Mar 24, 2022 · 3 comments · Fixed by #67131
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@AndyAyersMS
Copy link
Member

From libraries pgo CI job: https://dev.azure.com/dnceng/public/_test/analytics?definitionId=1002&contextType=build

;; x64 windows

set COMPlus_JitRandomOnStackReplacement=15
set COMPlus_OSR_HitLimit=2
set COMPlus_TC_OnStackReplacement=1
set COMPlus_TC_OnStackReplacement_InitialCounter=1
set COMPlus_TC_QuickJitForLoops=1
set COMPlus_TieredCompilation=1

  Starting:    System.Diagnostics.DiagnosticSource.Tests (parallel test collections = on, max threads = 12)
    System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel [FAIL]
      Assert.Equal() Failure
      Expected: LastValue { }
      Actual:   LastValue { }
      Stack Trace:
        C:\repos\runtime0\src\libraries\System.Diagnostics.DiagnosticSource\tests\AggregationManagerTests.cs(48,0): at System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel()

Have isolated this via COMPlus_JitEnablePatchpointRange to this method:

System.Diagnostics.Metrics.LabelInstructionInterpretter`2[ObjectSequence1,__Canon][System.Diagnostics.Metrics.ObjectSequence1,System.__Canon]::GetAggregator, IL size = 255, hash=0xfe17726d Tier1-OSR @0xaa
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Diagnostics.Tracing untriaged New issue has not been triaged by the area owner labels Mar 24, 2022
@ghost
Copy link

ghost commented Mar 24, 2022

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

From libraries pgo CI job: https://dev.azure.com/dnceng/public/_test/analytics?definitionId=1002&contextType=build

;; x64 windows

set COMPlus_JitRandomOnStackReplacement=15
set COMPlus_OSR_HitLimit=2
set COMPlus_TC_OnStackReplacement=1
set COMPlus_TC_OnStackReplacement_InitialCounter=1
set COMPlus_TC_QuickJitForLoops=1
set COMPlus_TieredCompilation=1

  Starting:    System.Diagnostics.DiagnosticSource.Tests (parallel test collections = on, max threads = 12)
    System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel [FAIL]
      Assert.Equal() Failure
      Expected: LastValue { }
      Actual:   LastValue { }
      Stack Trace:
        C:\repos\runtime0\src\libraries\System.Diagnostics.DiagnosticSource\tests\AggregationManagerTests.cs(48,0): at System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel()

Have isolated this via COMPlus_JitEnablePatchpointRange to this method:

System.Diagnostics.Metrics.LabelInstructionInterpretter`2[ObjectSequence1,__Canon][System.Diagnostics.Metrics.ObjectSequence1,System.__Canon]::GetAggregator, IL size = 255, hash=0xfe17726d Tier1-OSR @0xaa
Author: AndyAyersMS
Assignees: -
Labels:

area-System.Diagnostics.Tracing, untriaged

Milestone: -

@AndyAyersMS AndyAyersMS added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 24, 2022
@ghost
Copy link

ghost commented Mar 24, 2022

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

From libraries pgo CI job: https://dev.azure.com/dnceng/public/_test/analytics?definitionId=1002&contextType=build

;; x64 windows

set COMPlus_JitRandomOnStackReplacement=15
set COMPlus_OSR_HitLimit=2
set COMPlus_TC_OnStackReplacement=1
set COMPlus_TC_OnStackReplacement_InitialCounter=1
set COMPlus_TC_QuickJitForLoops=1
set COMPlus_TieredCompilation=1

  Starting:    System.Diagnostics.DiagnosticSource.Tests (parallel test collections = on, max threads = 12)
    System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel [FAIL]
      Assert.Equal() Failure
      Expected: LastValue { }
      Actual:   LastValue { }
      Stack Trace:
        C:\repos\runtime0\src\libraries\System.Diagnostics.DiagnosticSource\tests\AggregationManagerTests.cs(48,0): at System.Diagnostics.Metrics.Tests.AggregationStoreTests.GetOneLabel()

Have isolated this via COMPlus_JitEnablePatchpointRange to this method:

System.Diagnostics.Metrics.LabelInstructionInterpretter`2[ObjectSequence1,__Canon][System.Diagnostics.Metrics.ObjectSequence1,System.__Canon]::GetAggregator, IL size = 255, hash=0xfe17726d Tier1-OSR @0xaa
Author: AndyAyersMS
Assignees: -
Labels:

area-System.Diagnostics.Tracing, area-CodeGen-coreclr, untriaged

Milestone: -

@AndyAyersMS AndyAyersMS removed area-System.Diagnostics.Tracing untriaged New issue has not been triaged by the area owner labels Mar 24, 2022
@AndyAyersMS AndyAyersMS self-assigned this Mar 24, 2022
@AndyAyersMS AndyAyersMS added this to the 7.0.0 milestone Mar 24, 2022
@AndyAyersMS
Copy link
Member Author

The problem is that we're too aggressive with struct promotion (likely a result of #65903).

Relatively simple repro case

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

interface IFoo 
{
    public Span<object> AsSpan();
}

public struct ObjectSequence1 : IFoo
{
    public object Value1;
    
    public Span<object> AsSpan()
    {
        return MemoryMarshal.CreateSpan(ref Value1, 1);
    }
}

public struct ObjectSequenceMany : IFoo
{
    public object[] _values;

    public Span<object> AsSpan()
    {
        return _values.AsSpan();
    }

    public ObjectSequenceMany(object[] x)
    {
        _values = x;
    }
}

public class InvalidOSRPromotion
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool G<T>(int n) where T : IFoo
    {
        T values = default;

        if (values is ObjectSequenceMany)
        {
            values = (T)(object)new ObjectSequenceMany(new object[5]);
        }

        Span<object> indexedValues = values.AsSpan();

        for (int i = 0; i < n; i++)
        {
            indexedValues[i] = "foo";
        }

        if (values is ObjectSequence1)
        {
            return (indexedValues[0] == ((ObjectSequence1)(object)values).Value1);
        }
        
        return false;
    }

    public static int Main()
    {
        return G<ObjectSequence1>(1) ? 100 : -1;
    }
}

run this with

set COMPlus_TieredCompilation=1
set COMPlus_OSR_HitLimit=1
set COMPlus_TC_OnStackReplacement=1
set COMPlus_TC_OnStackReplacement_InitialCounter=1

The OSR method will unsoundly promote the values local and erroneously return false from G<ObjectSequence1>.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 25, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant