Skip to content

Enable SHA assembly on macosx-aarch64#1226

Draft
4a6f656c wants to merge 1 commit into
libressl:masterfrom
4a6f656c:sha-macosx-aarch64
Draft

Enable SHA assembly on macosx-aarch64#1226
4a6f656c wants to merge 1 commit into
libressl:masterfrom
4a6f656c:sha-macosx-aarch64

Conversation

@4a6f656c
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread update.sh
echo copying libcrypto source
rm -f crypto/*.c crypto/*.h
touch crypto/empty.c
git checkout crypto/crypto_assembly.h
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems weird. Why is this needed?

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.

For reasons unknown we remove all *.h files two lines earlier... we won't need this once crypto_assembly.h is upstream though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The rm above could be replaced with git clean -f -- crypto/*.c crypto/*.h - the original intent was just to delete untracked files that had been copied in from a previous build, but adding a tracked .h file changes that assumption

@@ -0,0 +1,299 @@
Index: crypto/sha/sha1_aarch64_ce.S
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just check in a standalone version of sha1_aarch64_ce_darwin.S at this point? Is it necessary to have all of this indirection for the sake of having one file instead of two? How often is this implementation going to change anyway?

Things only gets worse to impossible with armasm64 from VS, and once we start touching the stack like in sha512 and handling SEH. The complexity of trying to reinvent perlasm in cpp seems it would dwarf any benefits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I could get mingw64 sort of working with this macro approach, but that's about it.

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'm not against having separate sha1_aarch64_ce_darwin.S and sha1_aarch64_ce_windows.S or whatever, when there is significant divergence - there are a few options, including some form of templating/generation that we then use to create the .S and check them in (as opposed to perlasm being very opaque and part of the build process).

This branch was really just a proof-of-concept - for darwin I think the differences are resolvable and I was planning on upstreaming the majority of this diff. The mingw64/windows side is the one that I've not yet looked in to.

@busterb
Copy link
Copy Markdown
Contributor

busterb commented Apr 14, 2026

I rebased this on master and applied a few fixes; would you like me to just push to this PR, open a new one, etc.?

This needs a rebase since it no longer works on the latest openbsd, or I'd just PR diffs alone. Changes here: master...busterb:portable:pr-1226

Comment thread crypto/CMakeLists.txt
endif()

if(HOST_ASM_MACOSX_AARCH64)
set(CRYPTO_SRC ${CRYPTO_SRC} sha/sha1_amd64.c sha/sha256_amd64.c sha/sha512_amd64.c)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wrong architecture here.

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