Skip to content

SHA-512 accelerated by x86#731

Open
MarekKnapek wants to merge 10 commits intolibtom:developfrom
MarekKnapek:sha512x86
Open

SHA-512 accelerated by x86#731
MarekKnapek wants to merge 10 commits intolibtom:developfrom
MarekKnapek:sha512x86

Conversation

@MarekKnapek
Copy link
Copy Markdown
Contributor

SHA-512 accelerated by x86.

@MarekKnapek
Copy link
Copy Markdown
Contributor Author

I have no idea what is the problem on ci with the amalgamed build. How do I run this myself?

Comment thread src/hashes/sha2/sha384_desc.c
Comment thread src/hashes/sha2/sha512_224_desc.c
Comment thread src/hashes/sha2/sha512_256_desc.c
Comment thread src/hashes/sha2/sha512_desc.c
Comment thread src/hashes/sha2/sha512_224_desc.c Outdated
Comment thread src/hashes/sha2/sha512_256_desc.c Outdated
Copy link
Copy Markdown
Member

@sjaeckel sjaeckel left a comment

Choose a reason for hiding this comment

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

Basically LGTM, besides this one comment for now. I didn't look in further detail yet, but will do that tomorrow.

By merging #726 there was a merge conflict, which I've just resolved while rebasing this branch and therefore I've force-pushed.

Comment thread src/hashes/sha2/sha512_x86.c Outdated
Comment on lines +47 to +48
#define K sha512_x86_k
static const ulong64 sha512_x86_k[80] LTC_ALIGN(32) = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#define K sha512_x86_k
static const ulong64 sha512_x86_k[80] LTC_ALIGN(32) = {
#define K sha512_x86_k
LTC_ALIGN_MSVC(32)
static const ulong64 K[80] LTC_ALIGN(32) = {

Please note that's two changes.

Regarding the LTC_ALIGN_MSVC() macro addition: or isn't that required because this feature is only enabled for newer MSVC versions, which already have support for attribute((aligned()))?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think newer MSVC versions have support for GCC-style __attribute__, they do have support for MSVC-style __declspec and C-standard style alignas.

I have put the same alignment specifiers into the generic portable version of the SHA-512 table. In hopes that the compiler will recognize the two constants are exactly the same and de-duplicate them.

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