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

Resolve some McEliece AVX build issues #932

Merged
merged 4 commits into from
Mar 12, 2021

Conversation

jschanck
Copy link
Contributor

@jschanck jschanck commented Mar 3, 2021

The first commit makes it possible to do an OQS_MINIMAL_BUILD with just the avx McEliece code. The minimal build feature uses an allowlist for platform specific symbol suffixes, the commit just adds "_avx" to that list.

The second commit is an attempt to resolve #930. Declaring the constants with ".hidden" keeps them out of the global offset table and is an alternative to using "-Wl,-Bsymbolic". CI passing here will not indicate whether this resolves #930, but this solution is preferable to Bsymbolic in any case.

The third commit replaces casts to __m256i_u* with casts to __m256i*. The __m256i_u type is a gcc extension, and was preventing us from building the McEliece avx code with clang <9.

The fourth commit is just a nit-pick.

@jschanck jschanck requested review from dstebila and xvzcf as code owners March 3, 2021 17:54
@dstebila
Copy link
Member

dstebila commented Mar 3, 2021

If I understand correctly, this is all a temporary fix for now, since it would get wiped out by copy_from_upstream and long-term solution is to either push these changes back into PQClean or add patches in copy_from_upstream.

@dstebila dstebila added this to the 0.5.1 RC1 milestone Mar 3, 2021
@jschanck
Copy link
Contributor Author

jschanck commented Mar 4, 2021

If I understand correctly, this is all a temporary fix for now, since it would get wiped out by copy_from_upstream and long-term solution is to either push these changes back into PQClean or add patches in copy_from_upstream.

Yes. I will try to push 98cc2a5 and fb5cb7a back up to PQClean.

@jschanck
Copy link
Contributor Author

jschanck commented Mar 4, 2021

Just noticed Douglas' 0.5.1 tag. @baentsch were you expecting this for 0.5.0, or is 0.5.1 OK?

@dstebila
Copy link
Member

dstebila commented Mar 4, 2021

Just noticed Douglas' 0.5.1 tag. @baentsch were you expecting this for 0.5.0, or is 0.5.1 OK?

Ah, I thought the conclusion of yesterday's call was that Michael would build OSSL3 provider with AVX disabled for McEliece, and that he could do that without anything new. Maybe I misunderstood.

@baentsch
Copy link
Member

baentsch commented Mar 4, 2021

the conclusion of yesterday's call was that Michael would build OSSL3 provider with AVX disabled for McEliece

I also understood it that way such that we don't need an 0.5.0-rc2. Accordingly oqs-provider is built without this (McEliece-AVX) disablement only for post-0.5.0. liboqs. So good for me to merge as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants