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

Force-disable BMI2 instruction set when building snappy.lib #47

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

mangini
Copy link

@mangini mangini commented Aug 2, 2024

Description

Context

Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this crashes firestore in a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

How this PR works
By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).

Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

@mangini mangini changed the title Investigate how to disable BMI2 in leveldb Investigate how to disable BMI2 in snappy (used by leveldb) Aug 3, 2024
@mangini mangini force-pushed the mangini/leveldb-without-bmi2 branch 2 times, most recently from 7df18e6 to e567284 Compare August 6, 2024 00:45
@mangini mangini changed the title Investigate how to disable BMI2 in snappy (used by leveldb) Force-disable BMI2 instruction set when building snappy.lib Aug 6, 2024
@mangini mangini force-pushed the mangini/leveldb-without-bmi2 branch from a46c953 to ec73e88 Compare August 6, 2024 15:57
@mangini mangini requested a review from compnerd August 6, 2024 15:58
Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

I'm fine with this approach, but this will need to be fast deprecated. BMI2 is not really "optional" at this point. This extension has been around for 11 years and is part of Haswell family. We are already dependent on the previous generation (Nehalem) for cx16. Given that there has been discussions for upgrading to Nehalem and Swift is going to drop support for Windows 10 with it being EOL'ed 10/14/24, this might be a tease.

.github/workflows/bcny-firebase.yml Show resolved Hide resolved
Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Please squash when you merge.

@mangini mangini merged commit c38940e into compnerd/swift Aug 6, 2024
9 checks passed
@mangini mangini deleted the mangini/leveldb-without-bmi2 branch August 6, 2024 23:14
mangini added a commit that referenced this pull request Aug 6, 2024
Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).
bcny-fork-syncer bot pushed a commit that referenced this pull request Aug 21, 2024
Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 7, 2024
Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 11, 2024
Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).
bcny-fork-syncer bot pushed a commit that referenced this pull request Sep 12, 2024
Firebase-cpp-sdk on Windows pulls in firebase-ios-sdk (for firestore code), which pulls in leveldb (for key/value persistence) and Snappy (for efficient data compression). For faster compression, Snappy uses one specific CPU instruction for bit manipulation, bzhi, from the BMI2 instruction set. However, this instruction crashes firestore on a CPU that does not support the BMI2 instructions, which is the case of older CPUs, for example Intel pre-Haswell and AMD pre-Excavator CPUs.

By default, Snappy detects if it should use the BMI2 optimized instruction by performing a compilation test on the cmake configure phase , which obviously returns if the build machine supports BMI2, not the target machines.
Snappy build setup does not support a compiler directive that overrides that specific compilation test, so it can only build for the host machine BMI2 setup.

Unfortunately, due to the very involved dependency chain, there is no good way to patch Snappy's code and the easiest solution in terms of maintenance is to patch the config.h created during the configure step, which is what this PR does. It only changes the automated build process (Github Actions workflow).
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.

2 participants