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

Unify assembler files between Linux and Windows. #188

Open
wants to merge 3 commits into
base: preUnify
Choose a base branch
from
Open

Conversation

lundman
Copy link

@lundman lundman commented Nov 27, 2022

Since we can ask clang to call assembler functions with sysv_abi on Windows, we can now unify those files into one copy, instead of a separate copy for each platform.

This PR is created to explore the idea of unifying the assembler files. Is that "desirable"?

The unrelated tunables/sysctl/module_param_call changes are needed to enable assembler (the benchmarking etc)

Initially only blake3_sse2.S has been updated. Chosen by a kabal of 7 members as least offensive (criteria unknown).

This is a continuation of (or perhaps return to) work that illumos started. All assembler files should include <sys/asm_linkage.h>.

This file will now include the arch specific header, for example <sys/ia32/asm_linkage.h>. This file has been moved into platform specific location. Ie;

include/os/linux/spl/sys/ia32/asm_linkage.h
include/os/macos/spl/sys/ia32/asm_linkage.h
include/os/windows/spl/sys/ia32/asm_linkage.h
(and so on, imagine the same for aarch64 etc).

and the same in libspl:
lib/libspl/include/os/linux/sys/ia32/asm_linkage.h

So some header duplication between kernel and userland, like we already have.

In these headers files, macros are defined:
ENTRY_NP()
SET_SIZE()

to be used by the assembler files. Importantly
for Windows, we also define a new
ASMABI
which is set to "attributes((sysv_abi))". In C, when we extern prototype the assembler functions, we add ASMABI to it so the compiler will know to juggle the parameters and registers used.
(The Unix rdi, rsi, rcx, rdx, r8, r9 versus
Windows rdx, rdc, r8, r9, stack, stack ordering problem.)

For example:

extern void ASMABI zfs_blake3_compress_in_place_sse2(uint32_t cv[8],
const uint8_t block[BLAKE3_BLOCK_LEN], uint8_t block_len,
uint64_t counter, uint8_t flags);

This does mean we need to include spl headers when compiling module/icp and libicp. But it isn't to strange that the illumos-crypto-psomething uses SolarisPlatformLsomething.

Right now, macOS and Windows uses a separate copy of all assembler files, with Linux bits commented out. So it is "more" work to complete this PR. It is no work to do nothing.

But it is generally desirable to remove code duplication?

It now also sets sysv_abi for Linux. I do not know if this is going to be an issue. It felt "more correct" to do so since the assembler files definitely are written that way, and calling them any other way will not work (well). I think Linux developers should decide here.

Do we hate it? Is it worth continuing?
I do not care one bit about what the macros are named (okok, let's not test that statement). So if there are better names, speak.

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.

1 participant