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

Updating src/tests/Interop/PInvoke/Generics/GenericsNative.Vector* to annotate individual methods as requiring AVX #61259

Merged

Conversation

tannergooding
Copy link
Member

This is an "improved" version of #61229 that only opts specific methods into requiring AVX rather than forcing it for the entire compilation

… annotate individual methods as requiring AVX
@VSadov
Copy link
Member

VSadov commented Nov 5, 2021

can this address #47317 as well?

@tannergooding
Copy link
Member Author

can this address #47317 as well?

Possibly but also possibly not entirely. We'd have to check and see if a method that uses __m256i is compiled assuming AVX/AVX2 is generally available (for Windows).

If it only allows intrinsic usage and doesn't automatically opt into using __m256 based block copy; then it should work for the methods that are actually being called today. Otherwise, we'd need to split into "__m256" vs "user-defined 256-bit struct" and have if (Avx.IsEnabled) { CallM256BasedPInvoke(); } else { CallUserDefined256BitStructPInvoke(); }

@AndyAyersMS
Copy link
Member

We're no longer setting /arch:AVX2 for windows here, so do we pick up some other ambient /arch settting?

@tannergooding
Copy link
Member Author

We're no longer setting /arch:AVX2 for windows here, so do we pick up some other ambient /arch settting?

@AndyAyersMS, Windows doesn't require this at all and never has. It was specified purely so __AVX2__ was a defined macro. I removed the dependency on that in favor of always just using __m256 on XARCH.

If we want/need to handle the case of testing Vector256<T> interop when AVX2 is disabled, then we'll need to have different overloads that explicitly take user-defined structs (which would've been necessary on Unix anyways given the constraints of GCC/Clang).

@AndyAyersMS
Copy link
Member

Ok, thanks.

@tannergooding tannergooding merged commit c737141 into dotnet:main Nov 11, 2021
kant2002 added a commit to kant2002/runtime that referenced this pull request Dec 8, 2021
Just anotate couple other methods which I need, based on dotnet#61259 methodology
AndyAyersMS pushed a commit that referenced this pull request Dec 10, 2021
Just anotate couple other methods which I need, based on #61259 methodology
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2021
@tannergooding tannergooding deleted the FixInteropLibLLVMx64ArchOption branch November 11, 2022 15:21
Sign up for free to 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.

4 participants