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

Fix compile of crc32-pclmul_asm on macOS High Sierra #8

Merged

Conversation

felixbuenemann
Copy link

  • .type and .size are ELF/COFF specific so drop them
  • .globl + .hidden equivalent for macOS is .private_extern
  • assembly symbol names are not mangled on macOS, so we need to add _ prefix

This is similar to #6, but only contains the fixes that are actually required to compile on macOS 10.13.

I have run make test and make test64 and all tests are passing fine.

* .type and .size are ELF/COFF specific so drop them
* .globl + .hidden equivalent for macOS is .private_extern
* symbol name are not mangled on macOS, so we need to prefix _
@vkrasnov
Copy link

vkrasnov commented Dec 3, 2017

@felixbuenemann
Copy link
Author

felixbuenemann commented Dec 3, 2017

I tried compiling vlad/aarch64 branch, but it has the same errors as gcc.amd64 branch:

contrib/amd64/crc32-pclmul_asm.S:117:28: error: unknown directive
.globl crc32_pclmul_le_16; .hidden crc32_pclmul_le_16; .type crc32_pclmul_le_16, @function; crc32_pclmul_le_16:
                           ^
contrib/amd64/crc32-pclmul_asm.S:117:56: error: unknown directive
.globl crc32_pclmul_le_16; .hidden crc32_pclmul_le_16; .type crc32_pclmul_le_16, @function; crc32_pclmul_le_16:
                                                       ^
contrib/amd64/crc32-pclmul_asm.S:258:1: error: unknown directive
.size crc32_pclmul_le_16, .-crc32_pclmul_le_16
^
make: *** [crc32-pclmul_asm.o] Error 1

If I cherry-pick c129738 from this PR into vlad/aarch64 compilation works fine and tests are ok.

@vkrasnov
Copy link

vkrasnov commented Dec 3, 2017

Ooops, looks like I forgot to commit the fix, only changed the commit message :)

@felixbuenemann
Copy link
Author

@vkrasnov Have you found the missing commit for mac compatibility?

@kornelski
Copy link

This is still a problem.

@neurolabusc
Copy link

@kornelski I solved this by applying @RJVB's Mac and compatibility fixes #6.

While RJVB's code works, it does make some assumptions, e.g. that unsigned int is a 32-bit value. This might not be true for all 16 and 64 bit processors. Therefore, for backward compatibility I would have used the following code to ensure uint8_t, uint32_t, uint64_t are defined.

#ifndef uint32_t 
 #include <stdint.h>
#endif

@vkrasnov vkrasnov merged commit 0c6bb2b into cloudflare:gcc.amd64 May 27, 2018
@felixbuenemann felixbuenemann deleted the fix-macos-crc32-pclmul-assembly branch May 27, 2018 10:47
fhanau pushed a commit to fhanau/zlib that referenced this pull request Feb 27, 2023
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