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

Some code improvements. #32

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

besser82
Copy link
Contributor

@besser82 besser82 commented Oct 31, 2018

Change 'uint512_u' to be a type, not a symbol.

OSX / XCode complains about duplicate symbols.


Rename 'uint512_u' to 'gost34112012_uint512_u' to avoid clashes.


Rename the 'ALIGN' macro to 'GOST3411_ALIGN'.

macOS may define 'ALIGN' with a different meaning if some system
headers are included.


Fix '-Wstringop-truncation'.


Use the unaligned version of '_mm_store_si128'.

This is needed for ix86 architectures, as for some unidentified
reason the memory address may not be 16 bytes aligned properly.

The information from the Intel Intrinsics Guide shows no speed
penalty between using the aligned or the unaligned version of
this function.


Remove trailing whitespace.

@besser82 besser82 force-pushed the typedef_uint512_u branch 4 times, most recently from f0bf464 to b27bade Compare November 1, 2018 11:03
@besser82 besser82 changed the title Change 'uint512_u' to be a type, not a symbol. Some code improvements. Nov 1, 2018
Copy link
Owner

@adegtyarev adegtyarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you used a CLI tool for syntax checking?

I'd like to integrate that kind of checking to the CI/CD process with Travis. Any ideas?

Copy link
Owner

@adegtyarev adegtyarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abb9919 merged via 3cbee1f with rebase of the source branch. Thanks!

@db4
Copy link

db4 commented Nov 26, 2018

Once you're doing this, please also remove

/* Restore the Floating-point status on the CPU */
_mm_empty();

This command is useless in SSE2 mode (it's only needed to switch from MMX to x87), and it becomes an undefined function when MSVC compiler is used.

@db4
Copy link

db4 commented Nov 26, 2018

Also _mm_cvtsi64_si128 should not be used in 32-bit mode:

xmm4 = _mm_cvtsi64_si128((long long) r0); \
tmm4 = _mm_cvtsi64_si128((long long) r1); \

that translates to 64-bit movq, that is only available in x64 mode. Somehow GCC is able to translate it to several SSE instructions, but that's clearly an extension (MSVC refuses to do that). So the code compiles for Win64 but fails for Win32.

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.

4 participants