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

RocksDB fails to build on M1 Apple Silicon due to incorrect ifdefs #7710

Closed
hdevalence opened this issue Nov 24, 2020 · 5 comments
Closed

RocksDB fails to build on M1 Apple Silicon due to incorrect ifdefs #7710

hdevalence opened this issue Nov 24, 2020 · 5 comments
Assignees

Comments

@hdevalence
Copy link

Note: Please use Issues only for bug reports. For questions, discussions, feature requests, etc. post to dev group: https://groups.google.com/forum/#!forum/rocksdb or https://www.facebook.com/groups/rocksdb.dev

Expected behavior

RocksDB builds on M1 Apple Silicon (in this case via rust-rocksdb bindings)

Actual behavior

RocksDB does not build, erroring as follows:

  running: "c++" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-I" "rocksdb/include/" "-I" "rocksdb/" "-I" "rocksdb/third-party/gtest-1.8.1/fused-src/" "-I" "snappy/" "-I" "lz4/lib/" "-I" "zstd/lib/" "-I" "zstd/lib/dictBuilder/" "-I" "zlib/" "-I" "bzip2/" "-I" "." "-Wall" "-Wextra" "-std=c++11" "-Wno-unused-parameter" "-DSNAPPY=1" "-DLZ4=1" "-DZSTD=1" "-DZLIB=1" "-DBZIP2=1" "-DNDEBUG=1" "-DOS_MACOSX=1" "-DROCKSDB_PLATFORM_POSIX=1" "-DROCKSDB_LIB_IO_POSIX=1" "-o" "/Users/hdevalence/code/zcash/zebra/target/debug/build/librocksdb-sys-10baa88e9458bc13/out/rocksdb/options/options_parser.o" "-c" "rocksdb/options/options_parser.cc"
  cargo:warning=rocksdb/util/crc32c.cc:502:18: error: use of undeclared identifier 'isSSE42'
  cargo:warning=  has_fast_crc = isSSE42();
  cargo:warning=                 ^
  cargo:warning=rocksdb/util/crc32c.cc:1232:7: error: use of undeclared identifier 'isSSE42'
  cargo:warning=  if (isSSE42()) {
  cargo:warning=      ^
  cargo:warning=rocksdb/util/crc32c.cc:1233:9: error: use of undeclared identifier 'isPCLMULQDQ'
  cargo:warning=    if (isPCLMULQDQ()) {
  cargo:warning=        ^
  cargo:warning=3 errors generated.
  exit code: 1

A few lines up is this ifdef (pointed out by @teor2345:

// Detect if ARM64 CRC or not.

It looks like these functions are meant to always be defined, and return false when the intrinsics don't exist, but instead they are missing. Or, perhaps there is some other variable that should be defined on this platform that isn't being set?

Steps to reproduce the behavior

Presumably this is more widely applicable, but here are the specific steps I took:

  • Use a Mac with an M1 chip
  • Build cmake from source using its bootstrap
  • Build LLVM from the bootstrapped cmake (to avoid an x86 cmake running under rosetta2 from misdetecting the host target)
  • Set clang-sys environment variables to point to LLVM https://github.com/KyleMayes/clang-sys#environment-variables
  • Build Rust code using the rocksdb crate as a dependency
@hdevalence
Copy link
Author

Hmm, it looks like here

#elif defined(__linux__) && defined(HAVE_ARM64_CRC)
, there's a path for ARM64_CRC, but only on Linux. Is there some Linux-specific behavior intended?

@hdevalence
Copy link
Author

This patch seems to resolve the issue, I'm not sure why ARM64 support is linux-conditional: hdevalence@ed1882e

More context in this comment: rust-rocksdb/rust-rocksdb#482 (comment)

@adamretter
Copy link
Collaborator

adamretter commented Nov 24, 2020

@hdevalence Hi Henry I have been seeing the same issue on the Apple DTK (A12Z). I started looking into it but didn't spend much time on it yet.

I'm not sure why ARM64 support is linux-conditional

I suspect that was simply because the ARM64 platforms that were available for testing/dev were Linux.

This patch seems to resolve the issue

I will apply the patch and run some tests on ARM Mac and ARM Linux and report back...

@adamretter
Copy link
Collaborator

@hdevalence For the DTK I needed a few more changes than you suggested.

Would you be willing to try the attached patch please?
arm64_crc.patch.txt

@adamretter adamretter self-assigned this Nov 24, 2020
facebook-github-bot pushed a commit that referenced this issue Dec 4, 2020
Summary:
Closes - #7710

I tested this on an Apple DTK (Developer Transition Kit) with an Apple A12Z Bionic CPU and macOS Big Sur (11.0.1).

Previously the arm64 specific CRC optimisations were limited to Linux only OS... Well now Apple Silicon is also arm64 but runs macOS ;-)

Pull Request resolved: #7714

Reviewed By: ltamasi

Differential Revision: D25287349

Pulled By: pdillinger

fbshipit-source-id: 639b168bf0ac2652907531e9604936ac4974b577
@adamretter
Copy link
Collaborator

Closed by #7714

heckj pushed a commit to heckj/rocksdb that referenced this issue Jan 9, 2021
Summary:
Closes - facebook#7710

I tested this on an Apple DTK (Developer Transition Kit) with an Apple A12Z Bionic CPU and macOS Big Sur (11.0.1).

Previously the arm64 specific CRC optimisations were limited to Linux only OS... Well now Apple Silicon is also arm64 but runs macOS ;-)

Pull Request resolved: facebook#7714

Reviewed By: ltamasi

Differential Revision: D25287349

Pulled By: pdillinger

fbshipit-source-id: 639b168bf0ac2652907531e9604936ac4974b577
codingrhythm pushed a commit to SafetyCulture/rocksdb that referenced this issue Mar 5, 2021
Summary:
Closes - facebook#7710

I tested this on an Apple DTK (Developer Transition Kit) with an Apple A12Z Bionic CPU and macOS Big Sur (11.0.1).

Previously the arm64 specific CRC optimisations were limited to Linux only OS... Well now Apple Silicon is also arm64 but runs macOS ;-)

Pull Request resolved: facebook#7714

Reviewed By: ltamasi

Differential Revision: D25287349

Pulled By: pdillinger

fbshipit-source-id: 639b168bf0ac2652907531e9604936ac4974b577
changvvb pushed a commit to changvvb/rocksdb that referenced this issue Mar 12, 2021
Summary:
Closes - facebook#7710

I tested this on an Apple DTK (Developer Transition Kit) with an Apple A12Z Bionic CPU and macOS Big Sur (11.0.1).

Previously the arm64 specific CRC optimisations were limited to Linux only OS... Well now Apple Silicon is also arm64 but runs macOS ;-)

Pull Request resolved: facebook#7714

Reviewed By: ltamasi

Differential Revision: D25287349

Pulled By: pdillinger

fbshipit-source-id: 639b168bf0ac2652907531e9604936ac4974b577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants