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

[libc] Add simple features.h with implementation macro #69402

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

alfredfo
Copy link
Member

@alfredfo alfredfo commented Oct 18, 2023

In the future this should probably be autogenerated so it defines library version.

Only added config/linux/* at the moment with an
if(LLVM_LIBC_FULL_BUILD) statement as I don't think this makes sense for overlay builds.

See: Discussion in #libc https://discord.com/channels/636084430946959380/636732994891284500/1163979080979460176

@michaelrj-google

@llvmbot llvmbot added the libc label Oct 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 18, 2023

@llvm/pr-subscribers-libc

Author: None (alfredfo)

Changes

In the future this should probably be autogenerated so it defines library version.

Only added config/linux/* at the moment with an
if(LLVM_LIBC_FULL_BUILD statement as I don't think this makes sense for overlay builds.

See: Discussion in #libc https://discord.com/channels/636084430946959380/636732994891284500/1163979080979460176

@michaelrj-google


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

7 Files Affected:

  • (modified) libc/config/linux/aarch64/headers.txt (+6)
  • (modified) libc/config/linux/riscv/headers.txt (+6)
  • (modified) libc/config/linux/x86_64/headers.txt (+6)
  • (modified) libc/include/CMakeLists.txt (+9)
  • (added) libc/include/features.h.def (+17)
  • (modified) libc/include/llvm-libc-macros/CMakeLists.txt (+6)
  • (added) libc/include/llvm-libc-macros/features-macros.h (+14)
diff --git a/libc/config/linux/aarch64/headers.txt b/libc/config/linux/aarch64/headers.txt
index 6e30c7c29b2c4bf..75016344aba1d46 100644
--- a/libc/config/linux/aarch64/headers.txt
+++ b/libc/config/linux/aarch64/headers.txt
@@ -19,3 +19,9 @@ set(TARGET_PUBLIC_HEADERS
     libc.include.time
     libc.include.unistd
 )
+
+if(LLVM_LIBC_FULL_BUILD)
+  list(APPEND TARGET_PUBLIC_HEADERS
+    libc.include.features
+  )
+endif()
diff --git a/libc/config/linux/riscv/headers.txt b/libc/config/linux/riscv/headers.txt
index aaa75a9dd08cbd7..b9b1ee32fa8be07 100644
--- a/libc/config/linux/riscv/headers.txt
+++ b/libc/config/linux/riscv/headers.txt
@@ -39,3 +39,9 @@ set(TARGET_PUBLIC_HEADERS
     libc.include.sys_utsname
     libc.include.sys_wait
 )
+
+if(LLVM_LIBC_FULL_BUILD)
+  list(APPEND TARGET_PUBLIC_HEADERS
+    libc.include.features
+  )
+endif()
diff --git a/libc/config/linux/x86_64/headers.txt b/libc/config/linux/x86_64/headers.txt
index aaa75a9dd08cbd7..b9b1ee32fa8be07 100644
--- a/libc/config/linux/x86_64/headers.txt
+++ b/libc/config/linux/x86_64/headers.txt
@@ -39,3 +39,9 @@ set(TARGET_PUBLIC_HEADERS
     libc.include.sys_utsname
     libc.include.sys_wait
 )
+
+if(LLVM_LIBC_FULL_BUILD)
+  list(APPEND TARGET_PUBLIC_HEADERS
+    libc.include.features
+  )
+endif()
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index fd2cb9351419ca8..9d170603ffa45cd 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -46,6 +46,15 @@ add_gen_header(
     .llvm-libc-types.mode_t
 )
 
+add_gen_header(
+  features
+  DEF_FILE features.h.def
+  GEN_HDR features.h
+  DEPENDS
+    .llvm_libc_common_h
+    .llvm-libc-macros.features_macros
+)
+
 add_gen_header(
   fenv
   DEF_FILE fenv.h.def
diff --git a/libc/include/features.h.def b/libc/include/features.h.def
new file mode 100644
index 000000000000000..444a7da3db18f06
--- /dev/null
+++ b/libc/include/features.h.def
@@ -0,0 +1,17 @@
+//===-- C standard library header features.h ---------------------------------===//
+//
+// 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_FEATURES_H
+#define LLVM_LIBC_FEATURES_H
+
+#include <__llvm-libc-common.h>
+#include <llvm-libc-macros/features-macros.h>
+
+%%public_api()
+
+#endif // LLVM_LIBC_FEATURES_H
diff --git a/libc/include/llvm-libc-macros/CMakeLists.txt b/libc/include/llvm-libc-macros/CMakeLists.txt
index 8a225889a6f1f87..9c9e6bfd1256453 100644
--- a/libc/include/llvm-libc-macros/CMakeLists.txt
+++ b/libc/include/llvm-libc-macros/CMakeLists.txt
@@ -46,6 +46,12 @@ add_macro_header(
     fcntl-macros.h
 )
 
+add_macro_header(
+  features_macros
+  HDR
+    features-macros.h
+)
+
 add_macro_header(
   fenv_macros
   HDR
diff --git a/libc/include/llvm-libc-macros/features-macros.h b/libc/include/llvm-libc-macros/features-macros.h
new file mode 100644
index 000000000000000..b6941c9547286c0
--- /dev/null
+++ b/libc/include/llvm-libc-macros/features-macros.h
@@ -0,0 +1,14 @@
+//===-- Definition of macros from features.h ----------------------------------===//
+//
+// 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_MACROS_FEATURES_MACROS_H
+#define __LLVM_LIBC_MACROS_FEATURES_MACROS_H
+
+#define __LLVMLIBC__ 1
+
+#endif // __LLVM_LIBC_MACROS_FEATURES_MACROS_H

@github-actions
Copy link

github-actions bot commented Oct 18, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

alfredfo added a commit to alfredfo/gnuconfig that referenced this pull request Oct 18, 2023
Depends on llvm/llvm-project#69402

Signed-off-by: Alfred Persson Forsberg <cat@catcream.org>
alfredfo added a commit to alfredfo/gnuconfig that referenced this pull request Oct 18, 2023
Depends on llvm/llvm-project#69402

Signed-off-by: Alfred Persson Forsberg <cat@catcream.org>
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

The patch is good with some nits

libc/config/linux/aarch64/headers.txt Outdated Show resolved Hide resolved
libc/config/linux/riscv/headers.txt Outdated Show resolved Hide resolved
libc/config/linux/x86_64/headers.txt Outdated Show resolved Hide resolved
libc/include/llvm-libc-macros/features-macros.h Outdated Show resolved Hide resolved
libc/include/features.h.def Outdated Show resolved Hide resolved
#ifndef __LLVM_LIBC_MACROS_FEATURES_MACROS_H
#define __LLVM_LIBC_MACROS_FEATURES_MACROS_H

#define __LLVMLIBC__ 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why __LLVMLIBC__ instead of __LLVM_LIBC__? Is there some prior art here?

Copy link
Member Author

@alfredfo alfredfo Oct 18, 2023

Choose a reason for hiding this comment

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

I just picked one, also wondered whether LLVM_LIBC or LLVMLIBC fit better here. Could change it to __LLVM_LIBC__ if you prefer

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with either option. I'd lean slightly more towards __LLVM_LIBC__ since I think that's clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, merged with __LLVM_LIBC__

In the future this should probably be autogenerated so it defines library version.

Only added config/linux/* at the moment with an
if(LLVM_LIBC_FULL_BUILD statement as I don't think this makes sense for overlay builds.

See: Discussion in #libc https://discord.com/channels/636084430946959380/636732994891284500/1163979080979460176
@alfredfo alfredfo merged commit 74b0465 into llvm:main Oct 19, 2023
3 checks passed
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

overall LGTM

@@ -0,0 +1,17 @@
//===-- C standard library header features.h -------------------------------===//
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this line is one character too long

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed: 67770cb

#ifndef __LLVM_LIBC_MACROS_FEATURES_MACROS_H
#define __LLVM_LIBC_MACROS_FEATURES_MACROS_H

#define __LLVMLIBC__ 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with either option. I'd lean slightly more towards __LLVM_LIBC__ since I think that's clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants