Skip to content

[cmake] Hardcode some check_include_file checks #104706

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.

Sign up for GitHub

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
Endilll merged 16 commits into from
Jan 16, 2025
Merged

[cmake] Hardcode some check_include_file checks #104706

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a92b98b
Header-centric implementation
Endilll Aug 18, 2024
0d468fd
Move to platform-centric approach
Endilll Aug 18, 2024
31f51e4
Move `LLVM_USING_GLIBC` back
Endilll Aug 18, 2024
25c080a
Add z/OS values
Endilll Aug 19, 2024
d226369
Adjust AIX values
Endilll Aug 20, 2024
82bcfdb
Don't define ZOS
Endilll Aug 20, 2024
608037b
Merge remote-tracking branch 'upstream/main' into cmake-header-checks
Endilll Aug 21, 2024
b25fbf6
Remove `HAVE_LINK_H`
Endilll Aug 21, 2024
4ebf4ac
Inline checks for `CMAKE_SYSTEM_NAME`
Endilll Aug 21, 2024
dc4c5d0
Merge remote-tracking branch 'upstream/main' into cmake-header-checks
Endilll Jan 4, 2025
d5478cf
Fill in UNISTD_H for PURE_WINDOWS
Endilll Jan 15, 2025
51278c1
Merge AIX and Cygwin with the majority of platforms
Endilll Jan 15, 2025
75759a6
Merge remote-tracking branch 'upstream/main' into cmake-header-checks
Endilll Jan 15, 2025
bc5391e
Merge remote-tracking branch 'upstream/main' into cmake-header-checks
Endilll Jan 15, 2025
43dfb84
Factor in recent clean-up by MaskRay
Endilll Jan 15, 2025
2294e6a
Merge remote-tracking branch 'upstream/main' into cmake-header-checks
Endilll Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion llvm/cmake/config-ix.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ if (CMAKE_SYSTEM_NAME STREQUAL "NetBSD")
set(NETBSD 1)
endif()

# Confirmed in
# https://github.com/llvm/llvm-project/pull/104706#issuecomment-2297153534
if (CMAKE_SYSTEM_NAME MATCHES "OS390")
set(ZOS 1)
Copy link
Contributor

@perry-ca perry-ca Aug 19, 2024

Choose a reason for hiding this comment

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

The port of cmake to z/OS, in Modules/Platform/OS390.cmake, already has this line in it. Will this be a problem?

set(OS390 1)
set(ZOS 1)

endif()

if (CMAKE_SYSTEM_NAME STREQUAL "OpenBSD")
set(OPENBSD 1)
endif()
Expand Down Expand Up @@ -148,9 +154,30 @@ elseif (PURE_WINDOWS)
set(HAVE_SYS_TYPES_H 1)
set(HAVE_SYSEXITS_H 0)
set(HAVE_TERMIOS_H 0)
elseif (ZOS)
# Confirmed in
# https://github.com/llvm/llvm-project/pull/104706#issuecomment-2297109613
set(HAVE_DLFCN_H 1)
set(HAVE_ERRNO_H 1)
set(HAVE_FCNTL_H 1)
set(HAVE_FENV_H 1)
set(HAVE_LINK_H 0)
set(HAVE_MACH_MACH_H 0)
set(HAVE_MALLOC_MALLOC_H 0)
set(HAVE_PTHREAD_H 1)
set(HAVE_SIGNAL_H 1)
set(HAVE_SYS_IOCTL_H 1)
set(HAVE_SYS_MMAN_H 1)
set(HAVE_SYS_PARAM_H 0)
set(HAVE_SYS_RESOURCE_H 1)
set(HAVE_SYS_STAT_H 1)
set(HAVE_SYS_TIME_H 1)
set(HAVE_SYS_TYPES_H 1)
set(HAVE_SYSEXITS_H 0)
set(HAVE_TERMIOS_H 1)
set(HAVE_UNISTD_H 1)
else()
Copy link
Collaborator

@llvm-beanz llvm-beanz Aug 19, 2024

Choose a reason for hiding this comment

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

In theory, once you set the values, the check queries will early-out because they detect the cached value. Having it always call the check functions (even if the value is set) may make it easy to add platforms where some of these are known to exist, but others may not be.

Copy link
Contributor

@perry-ca perry-ca Aug 19, 2024

Choose a reason for hiding this comment

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

I was referring to the set(ZOS 1) line. Since the cmake platform files are already defining ZOS should we be defining it in the llvm cmake files?

Copy link
Contributor Author

@Endilll Endilll Aug 19, 2024

Choose a reason for hiding this comment

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

I was referring to the set(ZOS 1) line. Since the cmake platform files are already defining ZOS should we be defining it in the llvm cmake files?

If ZOS is always provided on z/OS CMake for one reason or another, we better skip setting it indeed. But it's news for me.

Copy link
Contributor Author

@Endilll Endilll Aug 19, 2024

Choose a reason for hiding this comment

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

In theory, once you set the values, the check queries will early-out because they detect the cached value.

I've never felt I understood cache variables enough to feel comfortable relying on them. But I agree, this should be the case.

Having it always call the check functions (even if the value is set) may make it easy to add platforms where some of these are known to exist, but others may not be.

Are you sure this would serve a practical purpose? If one cares enough to set some of them beforehand to improve configuration times, I don't see how they would not check the rest.

Copy link
Contributor Author

@Endilll Endilll Jan 4, 2025

Choose a reason for hiding this comment

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

In theory, once you set the values, the check queries will early-out because they detect the cached value.

I agree that this is the behavior (https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/CheckIncludeFile.cmake?ref_type=heads#L46), but I don't see it documented at https://cmake.org/cmake/help/latest/module/CheckIncludeFile.html. So I'm reluctant to rely on this.

# Other platforms that we don't promise support for.
# Notable exception is z/OS, which can't be detected via CMAKE_SYSTEM_NAME.
check_include_file(dlfcn.h HAVE_DLFCN_H)
check_include_file(errno.h HAVE_ERRNO_H)
check_include_file(fcntl.h HAVE_FCNTL_H)
Expand Down