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

Assembly optimisations are compiled even with --disable-asm #13759

Open
luke-jr opened this Issue Jul 25, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@luke-jr
Copy link
Member

luke-jr commented Jul 25, 2018

--disable-asm doesn't seem to actually disable the new assembly optimisations.

@MarcoFalke MarcoFalke added this to the 0.17.0 milestone Jul 25, 2018

@luke-jr luke-jr referenced a pull request that will close this issue Jul 28, 2018

Open

Fix --disable-asm for newer assembly checks/code #13788

@MarcoFalke MarcoFalke removed this from the 0.17.0 milestone Aug 2, 2018

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Aug 8, 2018

The only cryptographic code written in assembly is the sse4 1-way variable-length SHA256 code introduced in #10821, and that's what --disable-asm controls. The newer optimized code (introduced in #13191, #13393, and #13386) is written in C++ with intrinsics.

I don't mind changing the scope of --disable-asm to include these as well, but perhaps it should be renamed. The original reason for allowing the asm code to be disabled was that it was introduced relatively late in the 0.16 release cycle, and very hard to review. Those arguments don't apply as much to the intrinsics based code, I believe.

Thinking about it - apart from reviewability - are there any reasons you'd want to disable these? If there are concerns about their correctness, we shouldn't be building by default in releases. If there aren't, we should probably just remove the compile flag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.