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

Update Falcon & McEliece (adding AVX) #920

Merged
merged 6 commits into from
Feb 23, 2021
Merged

Update Falcon & McEliece (adding AVX) #920

merged 6 commits into from
Feb 23, 2021

Conversation

baentsch
Copy link
Member

Could fix #896 and #917

Please review initial commit to see changes that led to full PR (running copy_from_upstream)

@baentsch
Copy link
Member Author

@jschanck, @dstebila As the code is now building (improving on #896 (comment) ) would you have time/interest to take a look as to what's happening here with address-sanitizer in (newly added) McEliece-AVX code ?

@bhess Are you OK with the changes done to copy_from_upstream (in d0fc088)

Anyone, maybe @xvzcf (as I have no experience whatsoever with this): Any idea what this is?

/usr/bin/ld: src/kem/classic_mceliece/CMakeFiles/classic_mceliece_348864_avx.dir/pqclean_mceliece348864_avx/transpose_64x256_sp_asm.S.o: relocation R_X86_64_PC32 against symbol `PQCLEAN_MCELIECE348864_AVX_MASK5_0' can not be used when making a shared object; recompile with -fPIC

@bhess
Copy link
Member

bhess commented Feb 22, 2021

@bhess Are you OK with the changes done to copy_from_upstream (in d0fc088)

Looks good.

@dstebila
Copy link
Member

@jschanck, @dstebila As the code is now building (improving on #896 (comment) ) would you have time/interest to take a look as to what's happening here with address-sanitizer in (newly added) McEliece-AVX code ?

It seems like one of the keywords used in this file is a language extension, which clang with -Wpendatic doesn't like.

One fix is to add the following to .CMake/compiler_opts.cmake in the AddressSanitizer section:

add_compile_options(-Wno-language-extension-token)

I verified locally that the CircleCI AddressSanitizer workflow passes with that option.

@baentsch baentsch marked this pull request as ready for review February 23, 2021 12:54
@dstebila
Copy link
Member

Oh, don't forget to update the algorithm data sheets in the docs folder.

@baentsch baentsch requested a review from dstebila February 23, 2021 14:18
@baentsch baentsch merged commit 6040f55 into main Feb 23, 2021
@baentsch baentsch deleted the audit-falconmceliece branch February 23, 2021 15:46
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.

Missing AVX2 version of Classic McEliece
3 participants