-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[bazel][libc] Remove target compatibility restrictions for float128 #169292
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
Conversation
|
@llvm/pr-subscribers-libc Author: Chandler Carruth (chandlerc) ChangesThe restrictions here aren't nearly as much about the OS as the compiler and architecture, but the Bazel restriction was OS-based. Everything seems to work well on even Arm64 macOS, and I would expect most BSDs and other OSes to work well with Clang's support on x86-64. The source code here already handles detecting when there is compiler support for the type. And the users of this don't Full diff: https://github.com/llvm/llvm-project/pull/169292.diff 1 Files Affected:
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index bd48222856f22..e3962d9b5f95b 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -171,11 +171,6 @@ libc_support_library(
libc_support_library(
name = "llvm_libc_types_float128",
hdrs = ["include/llvm-libc-types/float128.h"],
- target_compatible_with = select({
- "@platforms//os:linux": [],
- "@platforms//os:windows": [],
- "//conditions:default": ["@platforms//:incompatible"],
- }),
deps = [":llvm_libc_macros_float_macros"],
)
|
The restrictions here aren't nearly as much about the OS as the compiler and architecture, but the Bazel restriction was OS-based. Everything seems to work well on even Arm64 macOS, and I would expect most BSDs and other OSes to work well with Clang's support on x86-64. The source code here already handles detecting when there is compiler support for the type. And the users of this don't `select` or do anything else to conditionally include the header, so it seems better to not restrict access to the header from the build system, and instead continue making the source code compatible or a no-op on relevant configurations.
94325ba to
5b3f738
Compare
|
(rebasing -- seemed to pick a bad baseline that didn't build MLIR cleanly, but unrelated to this PR) |
|
A bunch of targets were tagged w/ this in #86174, but it looks like this one was tagged by mistake, or maybe the sources didn't use to properly guard linux-specific bits. |
|
Thanks for review! Bazel seems still busted for MLIR, unrelated to this PR, so merging. |
…lvm#169292) The restrictions here aren't nearly as much about the OS as the compiler and architecture, but the Bazel restriction was OS-based. Everything seems to work well on even Arm64 macOS, and I would expect most BSDs and other OSes to work well with Clang's support on x86-64. The source code here already handles detecting when there is compiler support for the type. And the users of this don't `select` or do anything else to conditionally include the header, so it seems better to not restrict access to the header from the build system, and instead continue making the source code compatible or a no-op on relevant configurations.
The restrictions here aren't nearly as much about the OS as the compiler and architecture, but the Bazel restriction was OS-based. Everything seems to work well on even Arm64 macOS, and I would expect most BSDs and other OSes to work well with Clang's support on x86-64.
The source code here already handles detecting when there is compiler support for the type. And the users of this don't
selector do anything else to conditionally include the header, so it seems better to not restrict access to the header from the build system, and instead continue making the source code compatible or a no-op on relevant configurations.