Skip to content

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

Merged
vkrasnov merged 1 commit intocloudflare:gcc.amd64from
felixbuenemann:fix-macos-crc32-pclmul-assembly
May 27, 2018
Merged

Fix compile of crc32-pclmul_asm on macOS High Sierra#8
vkrasnov merged 1 commit intocloudflare:gcc.amd64from
felixbuenemann:fix-macos-crc32-pclmul-assembly

Conversation

@felixbuenemann
Copy link
Copy Markdown

@felixbuenemann felixbuenemann commented Dec 3, 2017

  • .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.

@felixbuenemann
Fix compile of crc32-pclmul_asm on macOS
c129738 
* .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
Copy Markdown

vkrasnov commented Dec 3, 2017

Can you check https://github.com/cloudflare/zlib/tree/vlad/aarch64 first?

@felixbuenemann
Copy link
Copy Markdown
Author

felixbuenemann commented Dec 3, 2017 •
edited
Loading

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
Copy Markdown

vkrasnov commented Dec 3, 2017

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

@felixbuenemann
Copy link
Copy Markdown
Author

felixbuenemann commented Dec 12, 2017

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

@kornelski
Copy link
Copy Markdown

kornelski commented May 9, 2018

This is still a problem.

@neurolabusc
Copy link
Copy Markdown

neurolabusc commented May 21, 2018

@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
@LloydW93 LloydW93 mentioned this pull request Sep 22, 2022
Closed
fhanau pushed a commit to fhanau/zlib that referenced this pull request Feb 27, 2023
@pthatcherg
Merge pull request cloudflare#8 from pthatcherg/reference-ietf-doc
8b899b5 
Reference IETF docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@felixbuenemann @vkrasnov @kornelski @neurolabusc