[libc][NFC] Move float macro into its own header / add target os detection #73311

gchatelet · 2023-11-24T10:33:27Z

Floating point properties are a combination of target OS, target architecture and compiler support.

Adding target OS detection,
Moving floating point type detection to its own file.

This is in preparation of adding support for _Float16 which requires testing compiler version and target architecture.

gchatelet · 2023-11-24T10:33:37Z

FYI @lntue, I was not able to find a non-brittle way to test for _Float16 availability.

For clang, I found __is_identifier but it doesn't work https://godbolt.org/z/oxdK7s9av
Predefined preprocessor are not consistent either.

The only way I could find was to test for individual compiler versions / architecture:

		__float128	_Float16
x86_64	clang	OK	>=15.0.0
x86_64	gcc	OK	>=12.1
armv8	clang	not supported	OK
arm64	gcc	not supported	>=13.1

So I think we'll have to use this ad-hoc logic.

llvmbot · 2023-11-24T10:33:57Z

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

Floating point properties are a combination of target OS, target architecture and compiler support.

Adding target OS detection,
Moving floating point type detection to its own file.

This is in preparation of adding support for _Float16 which requires testing compiler version and target architecture.

Full diff: https://github.com/llvm/llvm-project/pull/73311.diff

14 Files Affected:

(modified) libc/cmake/modules/compiler_features/check_float128.cpp (+1-1)
(modified) libc/docs/dev/code_style.rst (+6-2)
(modified) libc/src/__support/CMakeLists.txt (+1-1)
(modified) libc/src/__support/CPP/CMakeLists.txt (+2)
(modified) libc/src/__support/CPP/type_traits/is_floating_point.h (+1-1)
(modified) libc/src/__support/FPUtil/CMakeLists.txt (+1)
(modified) libc/src/__support/FPUtil/FloatProperties.h (+1-13)
(modified) libc/src/__support/macros/properties/CMakeLists.txt (+16)
(modified) libc/src/__support/macros/properties/compiler.h (-21)
(added) libc/src/__support/macros/properties/float.h (+46)
(added) libc/src/__support/macros/properties/os.h (+40)
(modified) libc/src/math/copysignf128.h (+1-1)
(modified) libc/src/math/generic/copysignf128.cpp (-1)
(modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+17-1)

diff --git a/libc/cmake/modules/compiler_features/check_float128.cpp b/libc/cmake/modules/compiler_features/check_float128.cpp
index 1dcfe80da0a0eb1..8b1e3fe04ed4e17 100644
--- a/libc/cmake/modules/compiler_features/check_float128.cpp
+++ b/libc/cmake/modules/compiler_features/check_float128.cpp
@@ -1,4 +1,4 @@
-#include "src/__support/macros/properties/compiler.h"
+#include "src/__support/macros/properties/float.h"
 
 #ifndef LIBC_COMPILER_HAS_FLOAT128
 #error unsupported
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index a28f7b9d717d4d0..71e6d1a2e4a016c 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -43,10 +43,14 @@ We define two kinds of macros:
 
      * ``architectures.h`` - Target architecture properties.
        e.g., ``LIBC_TARGET_ARCH_IS_ARM``.
+     * ``os.h`` - Target architecture properties.
+       e.g., ``LIBC_TARGET_OS_IS_LINUX``.
+     * ``cpu_features.h`` - Target cpu feature availability.
+       e.g., ``LIBC_TARGET_CPU_HAS_AVX2``.
      * ``compiler.h`` - Host compiler properties.
        e.g., ``LIBC_COMPILER_IS_CLANG``.
-     * ``cpu_features.h`` - Target cpu apu feature availability.
-       e.g., ``LIBC_TARGET_CPU_HAS_AVX2``.
+     * ``float.h`` - Float type compiler support.
+       e.g., ``LIBC_COMPILER_HAS_FLOAT128``.
 
    * ``src/__support/macros/config.h`` - Important compiler and platform
      features. Such macros can be used to produce portable code by
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt
index 8f66209ab54c550..b939fae3be791da 100644
--- a/libc/src/__support/CMakeLists.txt
+++ b/libc/src/__support/CMakeLists.txt
@@ -42,7 +42,7 @@ add_header_library(
     .named_pair
     libc.src.__support.CPP.type_traits
     libc.src.__support.macros.attributes
-    libc.src.__support.macros.properties.compiler
+    libc.src.__support.macros.config
 )
 
 add_header_library(
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index 6853bfa3b304a34..c75fae05fedfcd8 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -147,6 +147,8 @@ add_header_library(
     type_traits/true_type.h
     type_traits/type_identity.h
     type_traits/void_t.h
+  DEPENDS
+    libc.src.__support.macros.properties.float
 )
 
 add_header_library(
diff --git a/libc/src/__support/CPP/type_traits/is_floating_point.h b/libc/src/__support/CPP/type_traits/is_floating_point.h
index bcd204102997540..3a5260bcab11ee7 100644
--- a/libc/src/__support/CPP/type_traits/is_floating_point.h
+++ b/libc/src/__support/CPP/type_traits/is_floating_point.h
@@ -11,7 +11,7 @@
 #include "src/__support/CPP/type_traits/is_same.h"
 #include "src/__support/CPP/type_traits/remove_cv.h"
 #include "src/__support/macros/attributes.h"
-#include "src/__support/macros/properties/compiler.h"
+#include "src/__support/macros/properties/float.h"
 
 namespace LIBC_NAMESPACE::cpp {
 
diff --git a/libc/src/__support/FPUtil/CMakeLists.txt b/libc/src/__support/FPUtil/CMakeLists.txt
index 7cfa4481079e152..58a182eaa797bc7 100644
--- a/libc/src/__support/FPUtil/CMakeLists.txt
+++ b/libc/src/__support/FPUtil/CMakeLists.txt
@@ -28,6 +28,7 @@ add_header_library(
   HDRS
     FloatProperties.h
   DEPENDS
+    libc.src.__support.macros.properties.float
     libc.src.__support.uint128
 )
 
diff --git a/libc/src/__support/FPUtil/FloatProperties.h b/libc/src/__support/FPUtil/FloatProperties.h
index 3be4b518be33843..223d919801cd044 100644
--- a/libc/src/__support/FPUtil/FloatProperties.h
+++ b/libc/src/__support/FPUtil/FloatProperties.h
@@ -10,22 +10,10 @@
 #define LLVM_LIBC_SRC___SUPPORT_FPUTIL_FLOATPROPERTIES_H
 
 #include "src/__support/UInt128.h"
-#include "src/__support/macros/properties/architectures.h" // LIBC_TARGET_ARCH_XXX
+#include "src/__support/macros/properties/float.h" // LIBC_COMPILER_HAS_FLOAT128
 
 #include <stdint.h>
 
-// https://developer.arm.com/documentation/dui0491/i/C-and-C---Implementation-Details/Basic-data-types
-// https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
-// https://docs.amd.com/bundle/HIP-Programming-Guide-v5.1/page/Programming_with_HIP.html
-#if defined(_WIN32) || defined(__arm__) || defined(__NVPTX__) ||               \
-    defined(__AMDGPU__) || (defined(__APPLE__) && defined(__aarch64__))
-#define LONG_DOUBLE_IS_DOUBLE
-#endif
-
-#if !defined(LONG_DOUBLE_IS_DOUBLE) && defined(LIBC_TARGET_ARCH_IS_X86)
-#define SPECIAL_X86_LONG_DOUBLE
-#endif
-
 namespace LIBC_NAMESPACE {
 namespace fputil {
 
diff --git a/libc/src/__support/macros/properties/CMakeLists.txt b/libc/src/__support/macros/properties/CMakeLists.txt
index f6c88c34ebb5a03..e37cdb78bfa2c57 100644
--- a/libc/src/__support/macros/properties/CMakeLists.txt
+++ b/libc/src/__support/macros/properties/CMakeLists.txt
@@ -10,6 +10,12 @@ add_header_library(
     compiler.h
 )
 
+add_header_library(
+  os
+  HDRS
+    os.h
+)
+
 add_header_library(
   cpu_features
   HDRS
@@ -17,3 +23,13 @@ add_header_library(
   DEPENDS
     .architectures
 )
+
+add_header_library(
+  float
+  HDRS
+    float.h
+  DEPENDS
+    .architectures
+    .compiler
+    .os
+)
diff --git a/libc/src/__support/macros/properties/compiler.h b/libc/src/__support/macros/properties/compiler.h
index 3805d1f9a7a5055..a7a2822bf6a14a6 100644
--- a/libc/src/__support/macros/properties/compiler.h
+++ b/libc/src/__support/macros/properties/compiler.h
@@ -21,25 +21,4 @@
 #define LIBC_COMPILER_IS_MSC
 #endif
 
-// Check compiler features
-#if defined(FLT128_MANT_DIG)
-// C23 _Float128 type is available.
-#define LIBC_COMPILER_HAS_FLOAT128
-#define LIBC_FLOAT128_IS_C23
-using float128 = _Float128;
-
-#elif defined(__SIZEOF_FLOAT128__)
-// Builtin __float128 is available.
-#define LIBC_COMPILER_HAS_FLOAT128
-#define LIBC_FLOAT128_IS_BUILTIN
-using float128 = __float128;
-
-#elif (defined(__linux__) && defined(__aarch64__))
-// long double on Linux aarch64 is 128-bit floating point.
-#define LIBC_COMPILER_HAS_FLOAT128
-#define LIBC_FLOAT128_IS_LONG_DOUBLE
-using float128 = long double;
-
-#endif
-
 #endif // LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_COMPILER_H
diff --git a/libc/src/__support/macros/properties/float.h b/libc/src/__support/macros/properties/float.h
new file mode 100644
index 000000000000000..c40ca6120e4753e
--- /dev/null
+++ b/libc/src/__support/macros/properties/float.h
@@ -0,0 +1,46 @@
+//===-- Float type support --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Floating point properties are a combination of compiler support, target OS
+// and target architecture.
+
+#ifndef LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_FLOAT_H
+#define LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_FLOAT_H
+
+#include "src/__support/macros/properties/architectures.h"
+#include "src/__support/macros/properties/compiler.h"
+#include "src/__support/macros/properties/os.h"
+
+// https://developer.arm.com/documentation/dui0491/i/C-and-C---Implementation-Details/Basic-data-types
+// https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms
+// https://docs.amd.com/bundle/HIP-Programming-Guide-v5.1/page/Programming_with_HIP.html
+#if defined(LIBC_TARGET_OS_IS_WINDOWS) ||                                      \
+    (defined(LIBC_TARGET_OS_IS_MACOS) &&                                       \
+     defined(LIBC_TARGET_ARCH_IS_AARCH64)) ||                                  \
+    defined(LIBC_TARGET_ARCH_IS_ARM) || defined(LIBC_TARGET_ARCH_IS_NVPTX) ||  \
+    defined(LIBC_TARGET_ARCH_IS_AMDGPU)
+#define LONG_DOUBLE_IS_DOUBLE
+#endif
+
+#if !defined(LONG_DOUBLE_IS_DOUBLE) && defined(LIBC_TARGET_ARCH_IS_X86)
+#define SPECIAL_X86_LONG_DOUBLE
+#endif
+
+// Check compiler features
+#if defined(FLT128_MANT_DIG)
+#define LIBC_COMPILER_HAS_FLOAT128
+using float128 = _Float128;
+#elif defined(__SIZEOF_FLOAT128__)
+#define LIBC_COMPILER_HAS_FLOAT128
+using float128 = __float128;
+#elif (defined(__linux__) && defined(__aarch64__))
+#define LIBC_COMPILER_HAS_FLOAT128
+#define LIBC_FLOAT128_IS_LONG_DOUBLE
+using float128 = long double;
+#endif
+
+#endif // LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_FLOAT_H
diff --git a/libc/src/__support/macros/properties/os.h b/libc/src/__support/macros/properties/os.h
new file mode 100644
index 000000000000000..92e68b3e6612a82
--- /dev/null
+++ b/libc/src/__support/macros/properties/os.h
@@ -0,0 +1,40 @@
+//===-- Target OS detection -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_OS_H
+#define LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_OS_H
+
+#if (defined(__freebsd__) || defined(__FreeBSD__))
+#define LIBC_TARGET_OS_IS_FREEBSD
+#endif
+
+#if defined(__ANDROID__)
+#define LIBC_TARGET_OS_IS_ANDROID
+#endif
+
+#if defined(__linux__) && !defined(LIBC_TARGET_OS_IS_FREEBSD) &&               \
+    !defined(LIBC_TARGET_OS_IS_ANDROID)
+#define LIBC_TARGET_OS_IS_LINUX
+#endif
+
+#if (defined(_WIN64) || defined(_WIN32))
+#define LIBC_TARGET_OS_IS_WINDOWS
+#endif
+
+#if (defined(__apple__) || defined(__APPLE__) || defined(__MACH__))
+// From https://stackoverflow.com/a/49560690
+#include "TargetConditionals.h"
+#if defined(TARGET_OS_OSX)
+#define LIBC_TARGET_OS_IS_MACOS
+#endif
+#if defined(TARGET_OS_IPHONE)
+// This is set for any non-Mac Apple products (IOS, TV, WATCH)
+#define LIBC_TARGET_OS_IS_IPHONE
+#endif
+#endif
+
+#endif // LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_OS_H
diff --git a/libc/src/math/copysignf128.h b/libc/src/math/copysignf128.h
index 9448d8205dd7523..5e40657de33be3b 100644
--- a/libc/src/math/copysignf128.h
+++ b/libc/src/math/copysignf128.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_MATH_COPYSIGNF128_H
 #define LLVM_LIBC_SRC_MATH_COPYSIGNF128_H
 
-#include "src/__support/macros/properties/compiler.h"
+#include "src/__support/macros/properties/float.h"
 
 namespace LIBC_NAMESPACE {
 
diff --git a/libc/src/math/generic/copysignf128.cpp b/libc/src/math/generic/copysignf128.cpp
index 07e5caa223a5345..2fe36d52d01dcaa 100644
--- a/libc/src/math/generic/copysignf128.cpp
+++ b/libc/src/math/generic/copysignf128.cpp
@@ -9,7 +9,6 @@
 #include "src/math/copysignf128.h"
 #include "src/__support/FPUtil/ManipulationFunctions.h"
 #include "src/__support/common.h"
-#include "src/__support/macros/properties/compiler.h"
 
 namespace LIBC_NAMESPACE {
 
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 19573d83b12b3be..e7dc978b0dbe847 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -75,6 +75,21 @@ libc_support_library(
     hdrs = ["src/__support/macros/properties/compiler.h"],
 )
 
+libc_support_library(
+    name = "__support_macros_properties_os",
+    hdrs = ["src/__support/macros/properties/os.h"],
+)
+
+libc_support_library(
+    name = "__support_macros_properties_float",
+    hdrs = ["src/__support/macros/properties/float.h"],
+    deps = [
+        ":__support_macros_properties_architectures",
+        ":__support_macros_properties_compiler",
+        ":__support_macros_properties_os",
+    ],
+)
+
 libc_support_library(
     name = "__support_macros_properties_cpu_features",
     hdrs = ["src/__support/macros/properties/cpu_features.h"],
@@ -308,7 +323,7 @@ libc_support_library(
     deps = [
         ":__support_macros_attributes",
         ":__support_macros_config",
-        ":__support_macros_properties_compiler",
+        ":__support_macros_properties_float",
     ],
 )
 
@@ -657,6 +672,7 @@ libc_support_library(
     name = "__support_fputil_float_properties",
     hdrs = ["src/__support/FPUtil/FloatProperties.h"],
     deps = [
+        ":__support_macros_properties_float",
         ":__support_uint128",
     ],
 )

libc/docs/dev/code_style.rst

legrosbuffle · 2023-11-24T12:18:30Z

libc/src/__support/CPP/CMakeLists.txt

@@ -147,6 +147,8 @@ add_header_library(
    type_traits/true_type.h
    type_traits/type_identity.h
    type_traits/void_t.h
+  DEPENDS
+    libc.src.__support.macros.properties.float


This is missing a bunch of deps, you might want to add them too (in a separate patch)

Thx for noticing I'll fix this in a separate patch.

legrosbuffle · 2023-11-24T12:21:43Z

libc/src/math/copysignf128.h

@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_MATH_COPYSIGNF128_H
 #define LLVM_LIBC_SRC_MATH_COPYSIGNF128_H

-#include "src/__support/macros/properties/compiler.h"
+#include "src/__support/macros/properties/float.h"


You probably want to add a cmake dep for this.

I'll do this is a separate patch as well because libc/src/math/CMakeLists.txt needs to be tweaked to pass dependencies.

…ction

gchatelet requested a review from legrosbuffle November 24, 2023 10:33

llvmbot added the libc label Nov 24, 2023

legrosbuffle approved these changes Nov 24, 2023

View reviewed changes

gchatelet added 3 commits November 24, 2023 14:44

[libc][NFC] Move float macro into its own header / add target os dete…

12d890e

…ction

Update libc/docs/dev/code_style.rst

a3a3848

Sort doc items lexicographically

9bf7bab

gchatelet force-pushed the refactor_fp_properties branch from 06d686c to 9bf7bab Compare November 24, 2023 14:57

gchatelet merged commit 5e5a22c into llvm:main Nov 24, 2023

gchatelet deleted the refactor_fp_properties branch November 24, 2023 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc][NFC] Move float macro into its own header / add target os detection #73311

[libc][NFC] Move float macro into its own header / add target os detection #73311

Uh oh!

gchatelet commented Nov 24, 2023 •

edited

Loading

Uh oh!

gchatelet commented Nov 24, 2023 •

edited

Loading

Uh oh!

llvmbot commented Nov 24, 2023

Uh oh!

Uh oh!

legrosbuffle Nov 24, 2023

Uh oh!

gchatelet Nov 24, 2023

Uh oh!

legrosbuffle Nov 24, 2023

Uh oh!

gchatelet Nov 24, 2023

Uh oh!

Uh oh!

[libc][NFC] Move float macro into its own header / add target os detection #73311

[libc][NFC] Move float macro into its own header / add target os detection #73311

Uh oh!

Conversation

gchatelet commented Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gchatelet commented Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 24, 2023

Uh oh!

Uh oh!

legrosbuffle Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

gchatelet Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

legrosbuffle Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

gchatelet Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gchatelet commented Nov 24, 2023 •

edited

Loading

gchatelet commented Nov 24, 2023 •

edited

Loading