Improve Bazel config files#1063
Conversation
The value is the default, and no longer seems to be necessary.
This now probes the current host's capabilities for AVX and SSE features. For example, using `bazel build --config=avx` will get the right AVX (AVX2, AVX512F, etc.) based on what the host reports it supports.
This provides a way to pass `-march=native` to the build.
With the availability of the `--config=native` option, this can be simplified.
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Bazel build configuration to centralize and automate SIMD instruction set detection and flag selection across Linux, macOS, and Windows. It introduces a new compiler probing mechanism that removes the dependency on the py-cpuinfo Python package. Feedback identifies several critical issues in the newly added unit tests, including incorrect unpacking of return values and inaccurate expectations for compiler flags. Furthermore, the feature detection heuristic for Windows was noted as incomplete.
| asserts.equals(env, ["-mavx"], avx_copts) | ||
| asserts.equals(env, ["-msse2"], sse_copts) | ||
|
|
||
| has_avx, has_sse, features_str = get_feature_booleans(features) |
There was a problem hiding this comment.
The function get_feature_booleans returns 5 values (has_avx, has_avx2, has_avx512, has_sse, cpu_features_str), but this line only attempts to unpack 3. This will cause a runtime error in Starlark when running tests.
| has_avx, has_sse, features_str = get_feature_booleans(features) | |
| has_avx, has_avx2, has_avx512, has_sse, features_str = get_feature_booleans(features) |
| asserts.false(env, features["avx512f"]) | ||
|
|
||
| avx_copts, sse_copts = get_compiler_flags("linux", features) | ||
| asserts.equals(env, ["-mavx2", "-mfma", "-mbmi2"], avx_copts) |
There was a problem hiding this comment.
The test expectation for avx_copts is incorrect. The get_compiler_flags function in compiler_probe.bzl (line 191) returns ["-mavx2", "-mfma"] for AVX2. The -mbmi2 flag is handled separately in _compiler_probe_impl and added via BMI2_COPTS in bazel_utils.bzl. This test will fail as written.
| asserts.equals(env, ["-mavx2", "-mfma", "-mbmi2"], avx_copts) | |
| asserts.equals(env, ["-mavx2", "-mfma"], avx_copts) |
| asserts.true(env, features["bmi2"]) | ||
|
|
||
| avx_copts, _ = get_compiler_flags("linux", features) | ||
| asserts.equals(env, ["-mavx512f", "-mbmi2"], avx_copts) |
There was a problem hiding this comment.
The test expectation for avx_copts is incorrect for AVX512. get_compiler_flags returns ["-mavx512f"] (line 189). The -mbmi2 flag is not part of the avx_copts returned by that function. This test will fail.
| asserts.equals(env, ["-mavx512f", "-mbmi2"], avx_copts) | |
| asserts.equals(env, ["-mavx512f"], avx_copts) |
| asserts.true(env, features["sse4"]) | ||
|
|
||
| avx_copts, sse_copts = get_compiler_flags("darwin", features) | ||
| asserts.equals(env, ["-mavx2", "-mfma", "-mbmi2"], avx_copts) |
There was a problem hiding this comment.
Similar to the Linux AVX2 test, the macOS test expectation incorrectly includes -mbmi2 in avx_copts. This will cause the test to fail.
| asserts.equals(env, ["-mavx2", "-mfma", "-mbmi2"], avx_copts) | |
| asserts.equals(env, ["-mavx2", "-mfma"], avx_copts) |
| for feat, search_terms in feature_map.items(): | ||
| if is_win: | ||
| # Heuristic fallback for Windows hosts. | ||
| features[feat] = feat in ("avx2", "sse4", "sse2") |
There was a problem hiding this comment.
The heuristic for Windows feature detection is incomplete. It hardcodes avx2, sse4, and sse2 but misses avx. This will cause get_compiler_flags to fall back to /arch:AVX even when avx2 is detected as true, because the elif features.get("avx2") block at line 183 will be reached, but the features dict itself is inconsistent with real hardware capabilities (where AVX2 implies AVX).
| features[feat] = feat in ("avx2", "sse4", "sse2") | |
| features[feat] = feat in ("avx", "avx2", "sse4", "sse2") |
1 participant
No description provided.