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

[embree3] Fix Embree 3 static compilation patch #17701

Merged
merged 1 commit into from
May 13, 2021

Conversation

chrismile
Copy link
Contributor

Describe the pull request

  • What does your PR fix?

    ports/embree3/fix-static-usage.patch misses checks for AVX512KNL and AVX512SKX. Without these checks, finding the package in CMake will fail on systems enabling one of these two configurations. The pull request adds the missing checks.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all, no

  • Does your PR follow the maintainer guide?

    Yes.

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes.

@ghost
Copy link

ghost commented May 6, 2021

CLA assistant check
All CLA requirements met.

@NancyLi1013 NancyLi1013 changed the title Fix Embree 3 static compilation patch [embree3] Fix Embree 3 static compilation patch May 7, 2021
@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label May 7, 2021
@NancyLi1013
Copy link
Contributor

Hi @chrismile

Thanks for your fixing. Have you tested the feature checks added in this PR?

@chrismile
Copy link
Contributor Author

Yes, I have tested the feature checks. Once using a manually patched version of the .cmake files, and once using 'vcpkg install embree3' after adding the commit in this PR. I can confirm that a program linking to Embree with AVX512SKX compiles and runs successfully. I can't directly test whether AVX512KNL works, as I do not have access to a machine supporting it. But if this PR successfully fixes the problems with AVX512SKX, then there should be no reason why AVX512KNL shouldn't work.

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label May 8, 2021
@NancyLi1013
Copy link
Contributor

LGTM, thanks for your fixing @chrismile.

@dan-shaw dan-shaw merged commit 1c06693 into microsoft:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants