dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.18k stars 4.72k forks source link

[tvOS][arm64] Vector128Tests.Vector128NIntSumTest/Vector128NUIntSumTest failed on the rolling build #63746

Open MaximLipnin opened 2 years ago

MaximLipnin commented 2 years ago
<test name="System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests.Vector128NIntSumTest" type="System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests" method="Vector128NIntSumTest" time="0.0060218" result="Fail">
    <failure exception-type="Xunit.Sdk.EqualException">
        <message><![CDATA[Assert.Equal() Failure\nExpected: 2\nActual:   40452]]></message>
        <stack-trace><![CDATA[   at System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests.Vector128NIntSumTest()
    at System.Reflection.RuntimeMethodInfo.InvokeWorker(Object obj, BindingFlags invokeAttr, Span`1 parameters)]]></stack-trace>
    </failure>
</test>
<test name="System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests.Vector128NUIntSumTest" type="System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests" method="Vector128NUIntSumTest" time="0.0017315" result="Fail">
    <failure exception-type="Xunit.Sdk.EqualException">
        <message><![CDATA[Assert.Equal() Failure\nExpected: 2\nActual:   40452]]></message>
        <stack-trace><![CDATA[   at System.Runtime.Intrinsics.Tests.Vectors.Vector128Tests.Vector128NUIntSumTest()
    at System.Reflection.RuntimeMethodInfo.InvokeWorker(Object obj, BindingFlags invokeAttr, Span`1 parameters)]]></stack-trace>
    </failure>
</test>

https://dev.azure.com/dnceng/public/_build/results?buildId=1550469&view=ms.vss-test-web.build-test-results-tab&runId=43767104&resultId=112398&paneView=dotnet-dnceng.dnceng-anon-build-release-tasks.helix-anon-test-information-tab

@steveisok @akoeplinger

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics See info in area-owners.md if you want to be subscribed.

Issue Details
``` ``` https://dev.azure.com/dnceng/public/_build/results?buildId=1550469&view=ms.vss-test-web.build-test-results-tab&runId=43767104&resultId=112398&paneView=dotnet-dnceng.dnceng-anon-build-release-tasks.helix-anon-test-information-tab @steveisok @akoeplinger
Author: MaximLipnin
Assignees: -
Labels: `area-System.Runtime.Intrinsics`, `untriaged`
Milestone: -
ghost commented 2 years ago

Tagging subscribers to 'os-tvos': @steveisok, @akoeplinger See info in area-owners.md if you want to be subscribed.

Issue Details
``` ``` https://dev.azure.com/dnceng/public/_build/results?buildId=1550469&view=ms.vss-test-web.build-test-results-tab&runId=43767104&resultId=112398&paneView=dotnet-dnceng.dnceng-anon-build-release-tasks.helix-anon-test-information-tab @steveisok @akoeplinger
Author: MaximLipnin
Assignees: -
Labels: `area-System.Runtime.Intrinsics`, `untriaged`, `os-tvos`
Milestone: -
danmoseley commented 2 years ago

@tannergooding

steveisok commented 2 years ago

fyi - tvOS arm64 is our device run.

steveisok commented 2 years ago

@vargaz do you think this is going to work as expected at all on the device?

danmoseley commented 2 years ago

I'm curious about the expected VectorNN story on devices and WASM as well.

imhameed commented 2 years ago

@vargaz do you think this is going to work as expected at all on the device?

I'm curious about the expected VectorNN story on devices

NEON is a part of ARMv8-A so Vector64/Vector128 should be fully supported on arm64 tvOS devices. These tests all passed on arm64 Linux hardware on CI, when the LLVM AOT and LLVM FullAOT lanes were still enabled per-PR. Additionally they passed when I ran them by hand last year on an arm64 Linux machine and on an arm64 Linux VM running on an M1 mac. I have an Apple TV device on hand so I can look into this.

No idea about Wasm though.

tannergooding commented 2 years ago

NEON is a part of ARMv8-A so Vector64/Vector128 should be fully supported on arm64 tvOS devices

Just noting that these APIs in particular are new and have software fallbacks. They won't be accelerated on Mono yet and so the software fallback is what's likely failing here.

The software fallback is: https://source.dot.net/#System.Private.CoreLib/Vector128.cs,f8444b31977d1b83:

[Intrinsic]
public static T Sum<T>(Vector128<T> vector)
    where T : struct
{
    T sum = default;

    for (int index = 0; index < Vector128<T>.Count; index++)
    {
        sum = Scalar<T>.Add(sum, vector.GetElementUnsafe(index));
    }

    return sum;
}

The Scalar<T>.Add method is ultimately going to be doing:

// ...
else if (typeof(T) == typeof(nint))
{
    return (T)(object)((nint)(object)left + (nint)(object)right);
}
else if (typeof(T) == typeof(nuint))
{
    return (T)(object)((nuint)(object)left + (nuint)(object)right);
}
// ...

And GetElementUnsafe is doing:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static T GetElementUnsafe<T>(in this Vector128<T> vector, int index)
    where T : struct
{
    Debug.Assert((index >= 0) && (index < Vector128<T>.Count));
    return Unsafe.Add(ref Unsafe.As<Vector128<T>, T>(ref Unsafe.AsRef(in vector)), index);
}

Also noting however that this is the "safe" fallback implementation used by Vector<T> which has been running and passing for several months now.

vargaz commented 2 years ago

There is another set of test failures on tvos which appears related: https://github.com/dotnet/runtime/issues/61920