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

R2R Test CpuId_R2R_Avx fails locally #88156

Closed
jasper-d opened this issue Jun 28, 2023 · 5 comments · Fixed by #88848
Closed

R2R Test CpuId_R2R_Avx fails locally #88156

jasper-d opened this issue Jun 28, 2023 · 5 comments · Fixed by #88848

Comments

@jasper-d
Copy link
Contributor

Description

Running CpuId_R2R_Avx test fails locally (Ryzen 4750U) when testing Vector512 HW acceleration which this model does not have.

Reproduction Steps

git checkout 19d2e90b # commit is not related, it's just an arbirtrary recent revision
.\build.cmd clr+libs -rc Checked -lc Release
.\src\tests\build.cmd Checked test D:\runtime\src\tests\readytorun\HardwareIntrinsics\X86\CpuId_R2R_Avx.csproj
$env:CORE_ROOT="$PWD\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root"
.\artifacts\tests\coreclr\windows.x64.Checked\readytorun\HardwareIntrinsics\X86\CpuId_R2R_Avx\CpuId_R2R_Avx.cmd

Expected behavior

Test does not fail.

Actual behavior

Test fails with

System.Runtime.Intrinsics.Vector512.IsHardwareAccelerated returned false but the hardware and runtime returned true
Expected: 100
Actual: 0

Regression?

Probably not. I believe the test was added after I ran tests the last time.

Known Workarounds

Disabling AVX512 manually before running the test:

$env:DOTNET_EnableAVX512F=0

Configuration

.NET 19d2e90
Windows 10
AMD Ryzen 7 PRO 4750U with Radeon Graphics
AMD64 Family 23 Model 96 Stepping 1, AuthenticAMD

cpuid:

0x00000000 0x00: eax=0x0000000d ebx=0x68747541 ecx=0x444d4163 edx=0x69746e65
0x00000001 0x00: eax=0x00860f01 ebx=0x06100800 ecx=0xfed83203 edx=0x178bfbff
...
0x00000007 0x00: eax=0x00000000 ebx=0x219c01a9 ecx=0x00400004 edx=0x00000000
...

Other information

It passes with the following change to the test, but I'm not sure if that's the right fix.

bool isAvx512HierarchyDisabled = isHierarchyDisabled;

- bool isAvx512HierarchyDisabled = isHierarchyDisabled;
+ bool isAvx512HierarchyDisabled = isHierarchyDisabled || !Avx512F.IsSupported;

There is also the special handling of certain Intel models which could possibly be extended.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 28, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 28, 2023
@vcsjones vcsjones added area-System.Runtime.Intrinsics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 28, 2023
@ghost
Copy link

ghost commented Jun 28, 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

Description

Running CpuId_R2R_Avx test fails locally (Ryzen 4750U) when testing Vector512 HW acceleration which this model does not have.

Reproduction Steps

git checkout 19d2e90b # commit is not related, it's just an arbirtrary recent revision
.\build.cmd clr+libs -rc Checked -lc Release
.\src\tests\build.cmd Checked test D:\runtime\src\tests\readytorun\HardwareIntrinsics\X86\CpuId_R2R_Avx.csproj
$env:CORE_ROOT="$PWD\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root"
.\artifacts\tests\coreclr\windows.x64.Checked\readytorun\HardwareIntrinsics\X86\CpuId_R2R_Avx\CpuId_R2R_Avx.cmd

Expected behavior

Test does not fail.

Actual behavior

Test fails with

System.Runtime.Intrinsics.Vector512.IsHardwareAccelerated returned false but the hardware and runtime returned true
Expected: 100
Actual: 0

Regression?

Probably not. I believe the test was added after I ran tests the last time.

Known Workarounds

Disabling AVX512 manually before running the test:

$env:DOTNET_EnableAVX512F=0

Configuration

.NET 19d2e90
Windows 10
AMD Ryzen 7 PRO 4750U with Radeon Graphics
AMD64 Family 23 Model 96 Stepping 1, AuthenticAMD

cpuid:

0x00000000 0x00: eax=0x0000000d ebx=0x68747541 ecx=0x444d4163 edx=0x69746e65
0x00000001 0x00: eax=0x00860f01 ebx=0x06100800 ecx=0xfed83203 edx=0x178bfbff
...
0x00000007 0x00: eax=0x00000000 ebx=0x219c01a9 ecx=0x00400004 edx=0x00000000
...

Other information

It passes with the following change to the test, but I'm not sure if that's the right fix.

bool isAvx512HierarchyDisabled = isHierarchyDisabled;

- bool isAvx512HierarchyDisabled = isHierarchyDisabled;
+ bool isAvx512HierarchyDisabled = isHierarchyDisabled || !Avx512F.IsSupported;

There is also the special handling of certain Intel models which could possibly be extended.

Author: jasper-d
Assignees: -
Labels:

area-System.Runtime.Intrinsics, untriaged

Milestone: -

@jasper-d
Copy link
Contributor Author

Reproduces on more recent Ryzens too, e.g.

AMD Ryzen 7 PRO 6850U
CPU Family: 25
Stepping: 1
Model: 68

CPU:
   0x00000000 0x00: eax=0x00000010 ebx=0x68747541 ecx=0x444d4163 edx=0x69746e65
   0x00000001 0x00: eax=0x00a40f41 ebx=0x04100800 ecx=0x7ef8320b edx=0x178bfbff
   0x00000002 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000
   0x00000003 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000
   0x00000004 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000
   0x00000005 0x00: eax=0x00000040 ebx=0x00000040 ecx=0x00000003 edx=0x00000011
   0x00000006 0x00: eax=0x00000004 ebx=0x00000000 ecx=0x00000001 edx=0x00000000
   0x00000007 0x00: eax=0x00000000 ebx=0x219c97a9 ecx=0x0040069c edx=0x00000010

cc: @tannergooding I believe this is your area of expertise.

@tannergooding
Copy link
Member

Some fixes have gone in around this area. I'm working on getting one of my older boxes running so I can test locally myself, but if someone has the availability it would be beneficial to confirm whether the issue is still surfacing for the latest commit from main.

@jasper-d
Copy link
Contributor Author

It still reproduces for me running 2e7f0dc on Ryzen 4750U.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 13, 2023
@tannergooding
Copy link
Member

tannergooding commented Jul 13, 2023

Fix is up here: #88848

The issue was that we weren't including IsSupported reporting as false as part of the isHierarchyDisabled tracking and so even though the other checks all passed, the test thought Vector512 should be supported still.

@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Jul 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants