Skip to content

Remove InstructionSet_VectorT256 if AVX2 is disabled via env var - #89413

Merged
EgorBo merged 1 commit into
dotnet:mainfrom
EgorBo:fix-SetCpuInfo
Jul 25, 2023
Merged

Remove InstructionSet_VectorT256 if AVX2 is disabled via env var#89413
EgorBo merged 1 commit into
dotnet:mainfrom
EgorBo:fix-SetCpuInfo

Conversation

@EgorBo

Copy link
Copy Markdown
Member

Fixes#89408

@ghostghost added the area-VM-coreclr label Jul 24, 2023
@ghostghost assigned EgorBoJul 24, 2023
Comment threadsrc/coreclr/inc/corinfoinstructionset.h Outdated
Comment threadsrc/coreclr/vm/codeman.cpp Outdated
@jkotas

Copy link
Copy Markdown
Member

Is it something that I have somehow regressed by #89342 or was this a pre-existing bug?

@EgorBo

Copy link
Copy Markdown
MemberAuthor

Is it something that I have somehow regressed by #89342 or was this a pre-existing bug?

Probably, although, the issue reproduces only on CLR (NAOT and R2R are fine) and only if the cpu physically supports AVX2 but it's disabled by the config

Comment threadsrc/coreclr/vm/codeman.cpp Outdated
Comment threadsrc/coreclr/vm/codeman.cpp Outdated

@tannergoodingtannergooding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to have a test checking Vector<T>.IsHardwareAccelerated as well. I'm guessing we're missing that in CpuId.cs and are only checking the size instead

@jkotas

Copy link
Copy Markdown
Member

Probably, although, the issue reproduces only on CLR (NAOT and R2R are fine) and only if the cpu physically supports AVX2 but it's disabled by the config

Thank you for fixing it!

@EgorBo

Copy link
Copy Markdown
MemberAuthor

Might be good to have a test checking Vector<T>.IsHardwareAccelerated as well. I'm guessing we're missing that in CpuId.cs and are only checking the size instead

IsHardwareAccelerated didn't break, did it? The only way to test it is to use FILECHECK tests and check disasm - I'll think more about it as part of the quality week to improve test coverage for hw acceleration on asm level for SIMD crossplat APIs

@EgorBo
EgorBo merged commit d1fc57e into dotnet:mainJul 25, 2023
@EgorBo
EgorBo deleted the fix-SetCpuInfo branch July 25, 2023 08:20
@ghostghost locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All Vector<T> operators use software fallbacks when AVX is not presented

3 participants

@EgorBo@jkotas@tannergooding