Skip to content

[libc] Clean up some include in libc. #110980

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

Merged
merged 1 commit into from
Oct 6, 2024
Merged

[libc] Clean up some include in libc. #110980

merged 1 commit into from
Oct 6, 2024

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Oct 3, 2024

The patch primarily cleans up some incorrect includes. The LIBC_INLINE macro is defined in attributes.h, not config.h. There appears to be no need to change the CMake and Bazel build files.

@c8ef c8ef changed the title Draft [libc] Clean up some include in libc. Oct 3, 2024
@c8ef c8ef marked this pull request as ready for review October 3, 2024 10:47
@llvmbot llvmbot added the libc label Oct 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-libc

Author: None (c8ef)

Changes

The patch primarily cleans up some incorrect includes. The LIBC_INLINE macro is defined in attributes.h, not config.h. There appears to be no need to change the CMake and Bazel build files.


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

11 Files Affected:

  • (modified) libc/src/string/memory_utils/aarch64/inline_memcmp.h (+1-1)
  • (modified) libc/src/string/memory_utils/aarch64/inline_memcpy.h (+1-1)
  • (modified) libc/src/string/memory_utils/aarch64/inline_memmove.h (+1-1)
  • (modified) libc/src/string/memory_utils/generic/aligned_access.h (+1-1)
  • (modified) libc/src/string/memory_utils/generic/byte_per_byte.h (+1-1)
  • (modified) libc/src/string/memory_utils/inline_memcmp.h (+1-1)
  • (modified) libc/src/string/memory_utils/inline_memcpy.h (+1-1)
  • (modified) libc/src/string/memory_utils/inline_memset.h (+1-1)
  • (modified) libc/src/string/memory_utils/x86_64/inline_memcmp.h (+1-1)
  • (modified) libc/src/string/memory_utils/x86_64/inline_memcpy.h (-1)
  • (modified) libc/src/string/memory_utils/x86_64/inline_memmove.h (+1-1)
diff --git a/libc/src/string/memory_utils/aarch64/inline_memcmp.h b/libc/src/string/memory_utils/aarch64/inline_memcmp.h
index 84f5a412307983..35ca077dab526e 100644
--- a/libc/src/string/memory_utils/aarch64/inline_memcmp.h
+++ b/libc/src/string/memory_utils/aarch64/inline_memcmp.h
@@ -8,7 +8,7 @@
 #ifndef LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMCMP_H
 #define LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMCMP_H
 
-#include "src/__support/macros/config.h"       // LIBC_INLINE
+#include "src/__support/macros/attributes.h"   // LIBC_INLINE
 #include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
 #include "src/string/memory_utils/op_aarch64.h"
 #include "src/string/memory_utils/op_generic.h"
diff --git a/libc/src/string/memory_utils/aarch64/inline_memcpy.h b/libc/src/string/memory_utils/aarch64/inline_memcpy.h
index 2b34b675d8af24..11cf022e12b1fd 100644
--- a/libc/src/string/memory_utils/aarch64/inline_memcpy.h
+++ b/libc/src/string/memory_utils/aarch64/inline_memcpy.h
@@ -8,7 +8,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_AARCH64_INLINE_MEMCPY_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_AARCH64_INLINE_MEMCPY_H
 
-#include "src/__support/macros/config.h" // LIBC_INLINE
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
 #include "src/string/memory_utils/op_builtin.h"
 #include "src/string/memory_utils/utils.h"
 
diff --git a/libc/src/string/memory_utils/aarch64/inline_memmove.h b/libc/src/string/memory_utils/aarch64/inline_memmove.h
index def82373fbfbeb..2b238031af49d7 100644
--- a/libc/src/string/memory_utils/aarch64/inline_memmove.h
+++ b/libc/src/string/memory_utils/aarch64/inline_memmove.h
@@ -8,7 +8,7 @@
 #ifndef LIBC_SRC_STRING_MEMORY_UTILS_AARCH64_INLINE_MEMMOVE_H
 #define LIBC_SRC_STRING_MEMORY_UTILS_AARCH64_INLINE_MEMMOVE_H
 
-#include "src/__support/macros/config.h"        // LIBC_INLINE
+#include "src/__support/macros/attributes.h"    // LIBC_INLINE
 #include "src/string/memory_utils/op_aarch64.h" // aarch64::kNeon
 #include "src/string/memory_utils/op_builtin.h"
 #include "src/string/memory_utils/op_generic.h"
diff --git a/libc/src/string/memory_utils/generic/aligned_access.h b/libc/src/string/memory_utils/generic/aligned_access.h
index 7e480b8a1c2bb5..49a318d9c8a954 100644
--- a/libc/src/string/memory_utils/generic/aligned_access.h
+++ b/libc/src/string/memory_utils/generic/aligned_access.h
@@ -13,7 +13,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_ALIGNED_ACCESS_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_ALIGNED_ACCESS_H
 
-#include "src/__support/macros/config.h" // LIBC_INLINE
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
 #include "src/string/memory_utils/generic/byte_per_byte.h"
 #include "src/string/memory_utils/op_generic.h" // generic::splat
 #include "src/string/memory_utils/utils.h"      // Ptr, CPtr
diff --git a/libc/src/string/memory_utils/generic/byte_per_byte.h b/libc/src/string/memory_utils/generic/byte_per_byte.h
index eab5964bfe167e..2aecf0126cc438 100644
--- a/libc/src/string/memory_utils/generic/byte_per_byte.h
+++ b/libc/src/string/memory_utils/generic/byte_per_byte.h
@@ -12,7 +12,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_BYTE_PER_BYTE_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_GENERIC_BYTE_PER_BYTE_H
 
-#include "src/__support/macros/config.h"       // LIBC_INLINE
+#include "src/__support/macros/attributes.h"   // LIBC_INLINE
 #include "src/__support/macros/optimization.h" // LIBC_LOOP_NOUNROLL
 #include "src/string/memory_utils/utils.h"     // Ptr, CPtr
 
diff --git a/libc/src/string/memory_utils/inline_memcmp.h b/libc/src/string/memory_utils/inline_memcmp.h
index 82216265c23be9..cb7a07c4496bea 100644
--- a/libc/src/string/memory_utils/inline_memcmp.h
+++ b/libc/src/string/memory_utils/inline_memcmp.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMCMP_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMCMP_H
 
-#include "src/__support/macros/config.h"                   // LIBC_INLINE
+#include "src/__support/macros/attributes.h"               // LIBC_INLINE
 #include "src/__support/macros/properties/architectures.h" // LIBC_TARGET_ARCH_IS_
 #include "src/string/memory_utils/utils.h"                 // Ptr, CPtr
 
diff --git a/libc/src/string/memory_utils/inline_memcpy.h b/libc/src/string/memory_utils/inline_memcpy.h
index 7a77e77438138b..3e84397d3be6a2 100644
--- a/libc/src/string/memory_utils/inline_memcpy.h
+++ b/libc/src/string/memory_utils/inline_memcpy.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMCPY_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMCPY_H
 
-#include "src/__support/macros/config.h"                   // LIBC_INLINE
+#include "src/__support/macros/attributes.h"               // LIBC_INLINE
 #include "src/__support/macros/properties/architectures.h" // LIBC_TARGET_ARCH_IS_
 #include "src/string/memory_utils/utils.h"                 // Ptr, CPtr
 
diff --git a/libc/src/string/memory_utils/inline_memset.h b/libc/src/string/memory_utils/inline_memset.h
index 858e983f00932e..0206a0275e3f1a 100644
--- a/libc/src/string/memory_utils/inline_memset.h
+++ b/libc/src/string/memory_utils/inline_memset.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMSET_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_INLINE_MEMSET_H
 
-#include "src/__support/macros/config.h"                   // LIBC_INLINE
+#include "src/__support/macros/attributes.h"               // LIBC_INLINE
 #include "src/__support/macros/properties/architectures.h" // LIBC_TARGET_ARCH_IS_
 #include "src/string/memory_utils/utils.h"                 // Ptr, CPtr
 
diff --git a/libc/src/string/memory_utils/x86_64/inline_memcmp.h b/libc/src/string/memory_utils/x86_64/inline_memcmp.h
index 7fd101227c8832..5c18be2f0b25d8 100644
--- a/libc/src/string/memory_utils/x86_64/inline_memcmp.h
+++ b/libc/src/string/memory_utils/x86_64/inline_memcmp.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMCMP_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMCMP_H
 
-#include "src/__support/macros/config.h"       // LIBC_INLINE
+#include "src/__support/macros/attributes.h"   // LIBC_INLINE
 #include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
 #include "src/string/memory_utils/op_generic.h"
 #include "src/string/memory_utils/op_x86.h"
diff --git a/libc/src/string/memory_utils/x86_64/inline_memcpy.h b/libc/src/string/memory_utils/x86_64/inline_memcpy.h
index 718fd2f7179c64..2b2c6e6fbc5466 100644
--- a/libc/src/string/memory_utils/x86_64/inline_memcpy.h
+++ b/libc/src/string/memory_utils/x86_64/inline_memcpy.h
@@ -9,7 +9,6 @@
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMCPY_H
 
 #include "src/__support/macros/attributes.h"   // LIBC_INLINE_VAR
-#include "src/__support/macros/config.h"       // LIBC_INLINE
 #include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
 #include "src/string/memory_utils/op_builtin.h"
 #include "src/string/memory_utils/op_x86.h"
diff --git a/libc/src/string/memory_utils/x86_64/inline_memmove.h b/libc/src/string/memory_utils/x86_64/inline_memmove.h
index d81e833598f790..cec8e969b123ed 100644
--- a/libc/src/string/memory_utils/x86_64/inline_memmove.h
+++ b/libc/src/string/memory_utils/x86_64/inline_memmove.h
@@ -8,7 +8,7 @@
 #ifndef LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMMOVE_H
 #define LLVM_LIBC_SRC_STRING_MEMORY_UTILS_X86_64_INLINE_MEMMOVE_H
 
-#include "src/__support/macros/config.h" // LIBC_INLINE
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
 #include "src/string/memory_utils/op_builtin.h"
 #include "src/string/memory_utils/op_generic.h"
 #include "src/string/memory_utils/op_x86.h"

@c8ef
Copy link
Contributor Author

c8ef commented Oct 3, 2024

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the cleanup. Did you verify this still compiles without issue?

@c8ef
Copy link
Contributor Author

c8ef commented Oct 4, 2024

Yes, it works fine. I verified the CMake build locally, and the CI confirms it works great too. Regarding the Bazel build, we can see from the following snippet that it already includes the __support_macros_attributes dependency, so I don't think we need to modify it either.

libc_support_library(
name = "string_memory_utils",
hdrs = [
"src/string/memory_utils/op_aarch64.h",
"src/string/memory_utils/op_builtin.h",
"src/string/memory_utils/op_generic.h",
"src/string/memory_utils/op_riscv.h",
"src/string/memory_utils/op_x86.h",
"src/string/memory_utils/utils.h",
],
textual_hdrs = [
"src/string/memory_utils/aarch64/inline_bcmp.h",
"src/string/memory_utils/aarch64/inline_memcmp.h",
"src/string/memory_utils/aarch64/inline_memcpy.h",
"src/string/memory_utils/aarch64/inline_memmove.h",
"src/string/memory_utils/aarch64/inline_memset.h",
"src/string/memory_utils/generic/aligned_access.h",
"src/string/memory_utils/generic/byte_per_byte.h",
"src/string/memory_utils/inline_bcmp.h",
"src/string/memory_utils/inline_bzero.h",
"src/string/memory_utils/inline_memcmp.h",
"src/string/memory_utils/inline_memcpy.h",
"src/string/memory_utils/inline_memmem.h",
"src/string/memory_utils/inline_memmove.h",
"src/string/memory_utils/inline_memset.h",
"src/string/memory_utils/inline_strcmp.h",
"src/string/memory_utils/inline_strstr.h",
"src/string/memory_utils/riscv/inline_bcmp.h",
"src/string/memory_utils/riscv/inline_memcmp.h",
"src/string/memory_utils/riscv/inline_memcpy.h",
"src/string/memory_utils/riscv/inline_memmove.h",
"src/string/memory_utils/riscv/inline_memset.h",
"src/string/memory_utils/x86_64/inline_bcmp.h",
"src/string/memory_utils/x86_64/inline_memcmp.h",
"src/string/memory_utils/x86_64/inline_memcpy.h",
"src/string/memory_utils/x86_64/inline_memmove.h",
"src/string/memory_utils/x86_64/inline_memset.h",
],
deps = [
":__support_common",
":__support_cpp_array",
":__support_cpp_bit",
":__support_cpp_cstddef",
":__support_cpp_type_traits",
":__support_macros_attributes",
":__support_macros_optimization",
":__support_macros_properties_architectures",
":__support_macros_properties_cpu_features",
],
)

@c8ef
Copy link
Contributor Author

c8ef commented Oct 5, 2024

Hi @nickdesaulniers, I’d like to know if this patch is good to go or needs further adjustments. If it looks good to you, I’d be happy to land it.

@c8ef c8ef requested review from lntue and michaelrj-google October 6, 2024 11:38
@lntue lntue merged commit 2cc9795 into llvm:main Oct 6, 2024
11 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 6, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building libc at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/6663

Here is the relevant piece of the build log for the reference
Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug53727.cpp (866 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug50022.cpp (867 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug47654.cpp (868 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (869 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (870 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (871 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (872 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (873 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (874 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (875 of 879)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1236.158239

Kyvangka1610 added a commit to Kyvangka1610/llvm-project that referenced this pull request Oct 6, 2024
* commit 'FETCH_HEAD':
  [X86] combineAndLoadToBZHI - don't do an return early return if we fail to match a load
  [X86] replace-load-and-with-bzhi.ll - add commuted test cases to show failure to fold
  [X86] replace-load-and-with-bzhi.ll - cleanup check-prefixes to use X86/X64 for 32/64-bit targets
  [ExecutionEngine] Avoid repeated hash lookups (NFC) (llvm#111275)
  [ByteCode] Avoid repeated hash lookups (NFC) (llvm#111273)
  [StaticAnalyzer] Avoid repeated hash lookups (NFC) (llvm#111272)
  [CodeGen] Avoid repeated hash lookups (NFC) (llvm#111274)
  [RISCV] Simplify fixed-vector-fp.ll run lines. NFC
  [libc++][format][1/3] Adds more benchmarks. (llvm#101803)
  [X86] combineOrXorWithSETCC - avoid duplicate SDLoc/operands code. NFC.
  [X86] convertIntLogicToFPLogic - avoid duplicate SDLoc/operands code. NFC.
  [libc] Clean up some include in `libc`. (llvm#110980)
  [X86] combineBitOpWithPACK - avoid duplicate SDLoc/operands code. NFC.
  [X86] combineBitOpWithMOVMSK - avoid duplicate SDLoc/operands code. NFC.
  [X86] combineBitOpWithShift - avoid duplicate SDLoc/operands code. NFC.
  [x86] combineMul - use computeKnownBits directly to find MUL_IMM constant splat.
  [X86] combineSubABS - avoid duplicate SDLoc. NFC.
  [ValueTypes][RISCV] Add v1bf16 type (llvm#111112)
  [VPlan] Add additional FOR hoisting test.
  [clang-tidy] Create bugprone-bitwise-pointer-cast check (llvm#108083)
  [InstCombine] Canonicalize more geps with constant gep bases and constant offsets. (llvm#110033)
  [LV] Honor uniform-after-vectorization in setVectorizedCallDecision.
  [ELF] Pass Ctx & to Arch/
  [ELF] Pass Ctx & to Arch/
  [libc++] Fix a typo (llvm#111239)
  [X86] For minsize memset/memcpy, use byte or double-word accesses (llvm#87003)
  [RISCV] Unify RVBShift_ri and RVBShiftW_ri with Shift_ri and ShiftW_ri. NFC (llvm#111263)
  Revert "Reapply "[AMDGPU][GlobalISel] Fix load/store of pointer vectors, buffer.*.pN (llvm#110714)" (llvm#111059)"
  [libc] Add missing include to __support/StringUtil/tables/stdc_errors.h. (llvm#111271)
  [libc] remove errno.h includes (llvm#110934)
  [NFC][rtsan] Update docs to include [[clang::blocking]] (llvm#111249)
  [RISCV] Give ZEXT_H_RV32 and ZEXT_H_RV64 R-type format to match PACK. NFC
  [mlir][SPIRV] Fix build (2) (llvm#111265)
  [mlir][SPIRV] Fix build error (llvm#111264)
  [mlir][NFC] Mark type converter in `populate...` functions as `const` (llvm#111250)
  [Basic] Avoid repeated hash lookups (NFC) (llvm#111228)
  [RISCV] Use THShift_ri class instead of RVBShift_ri for TH_TST instruction. NFC
  [VPlan] Only generate first lane for VPPredInstPHI if no others used.
  [ELF] Don't call getPPC64TargetInfo outside Driver. NFC
  [GISel] Don't preserve NSW flag when converting G_MUL of INT_MIN to G_SHL. (llvm#111230)
  [APInt] Slightly simplify APInt::ashrSlowCase. NFC (llvm#111220)
  [Sema] Avoid repeated hash lookups (NFC) (llvm#111227)
  [Affine] Avoid repeated hash lookups (NFC) (llvm#111226)
  [Driver] Avoid repeated hash lookups (NFC) (llvm#111225)
  [clang][test] Remove a broken bytecode test
  [ELF] Pass Ctx &
  [ELF] Pass Ctx & to Relocations

Signed-off-by: kyvangka1610 <[email protected]>
@c8ef c8ef deleted the config_dot_h branch October 6, 2024 23:51
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.

5 participants