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

Missing CI test coverage for Vector512 #88233

Closed
MihaZupan opened this issue Jun 30, 2023 · 2 comments · Fixed by #88531
Closed

Missing CI test coverage for Vector512 #88233

MihaZupan opened this issue Jun 30, 2023 · 2 comments · Fixed by #88531
Labels
area-System.Runtime.Intrinsics avx512 Related to the AVX-512 architecture
Milestone

Comments

@MihaZupan
Copy link
Member

The following test passed CI without any errors in #88166:

[Fact]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static void SanityCheckAO()
{
    if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported)
        throw new Exception("Yay?");
}

Dumping the CpuId, it looks like all processors in CI that report Avx512BW.IsSupported == true also report isGenuineIntel=True familyID=6 extendedModelID=5 model=5.

if (xarchCpuInfo.FamilyId == 0x06)
{
if (xarchCpuInfo.ExtendedModelId == 0x05)
{
if (xarchCpuInfo.Model == 0x05)
{
// * Skylake (Server)
// * Cascade Lake
// * Cooper Lake
CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_VECTOR512_THROTTLING);

With #86655, those no longer report Vector512.IsHardwareAccelerated, therefore we have no CI coverage of such code paths?

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 30, 2023
@ghost
Copy link

ghost commented Jun 30, 2023

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

Issue Details

The following test passed CI without any errors in #88166:

[Fact]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
public static void SanityCheckAO()
{
    if (Vector512.IsHardwareAccelerated && Avx512BW.IsSupported)
        throw new Exception("Yay?");
}

Dumping the CpuId, it looks like all processors in CI that report Avx512BW.IsSupported == true also report isGenuineIntel=True familyID=6 extendedModelID=5 model=5.

if (xarchCpuInfo.FamilyId == 0x06)
{
if (xarchCpuInfo.ExtendedModelId == 0x05)
{
if (xarchCpuInfo.Model == 0x05)
{
// * Skylake (Server)
// * Cascade Lake
// * Cooper Lake
CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_VECTOR512_THROTTLING);

With #86655, those no longer report Vector512.IsHardwareAccelerated, therefore we have no CI coverage of such code paths?

Author: MihaZupan
Assignees: -
Labels:

area-System.Runtime.Intrinsics

Milestone: -

@tannergooding
Copy link
Member

We should add an outerloop leg to the AVX-512 run that sets DOTNET_PreferredVectorBitWidth=512

We are getting some stress coverage still, just randomly based on compStressCompile(STRESS_GENERIC_VARN, 20) and noting this only impacts Vector512.IsHardwareAccelerated lightup. It doesn't impact direct usage or validation of the System.Runtime.Intrinsics.X86 code paths.

@BruceForstall BruceForstall added the avx512 Related to the AVX-512 architecture label Jul 1, 2023
@BruceForstall BruceForstall added this to the 8.0.0 milestone Jul 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 1, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 7, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.Intrinsics avx512 Related to the AVX-512 architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants