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.

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

Jump to bottom

Add CMake support; Resolves #565 #566

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Add CMake support; Resolves #565 #566

wants to merge 36 commits into from

Conversation

FunMiles
Copy link

@FunMiles FunMiles commented Feb 1, 2023

The initial PR is way incomplete. It does work for MacOS :

  • It does a proper and clean install of the library with the required public include files (including generated ones)
  • It can be used a standalone or as a sub-project

However:

  • No support yet for any other OS
  • Not all tests and utilities are compiled

bgoglin and others added 30 commits June 28, 2022 11:36
@bgoglin
VERSION: prepare for 2.8rc1
7a263b5 
Bump soname to 21:0:6 (vs 20:3:5 for 2.7.1),

Reuse AM 1.16.5 and AC 2.71, but bump LT to 2.4.7.

Signed-off-by: Brice Goglin <[email protected]>
@bgoglin
configure: disable netloc by default in 2.8
1974932 
Still too experimental, might get removed if we don't find some manpower to revive it

And also disable netloc in sonarscanner job

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 2082c24
                       and 15a927b)
(cherry picked from commit eddcbcf)
(cherry picked from commit 30075ab)
(cherry picked from commit 29af441)
(cherry picked from commit dfab43e)
(cherry picked from commit 4fcdc93)
(cherry picked from commit b2eff2b)
Signed-off-by: Brice Goglin <[email protected]>
@bgoglin
ci.inria.fr: move the v2.8 nightly build to a different time
3c6fd18 
Reuse the timeslot of v2.3 which is unlikely to ever run again.

Signed-off-by: Brice Goglin <[email protected]>
@bgoglin
private/windows.h: guard against _ANONYMOUS_UNION/STRUCT redefinition
b62ca67 
It's already defined in Cygwin's /usr/include/w32api/_mingw.h

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c91712e)
@bgoglin
VERSION: 2.8.0rc1 released, doing rc2 now
1f746c0 
Signed-off-by: Brice Goglin <[email protected]>
@bgoglin
VERSION: 2.8.0 released, doing 2.8.1rc1 now
cf388b0 
Signed-off-by: Brice Goglin <[email protected]>
@bgoglin
pci: document the PCI affinity quirk for HPE Cray EX235a
0db5fdc 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit ac18e13)
@bgoglin
utils/tests/calc: check for multiple levels of groups
38fbe3d 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 4732ec4)
@bgoglin
levelzero: fix subdevice info properties
32d273f 
Thanks to Jonathan Peyton for the fix.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 1f0283e)
@bgoglin
pci+linux: make 32bits domain warnings non-critical
6d1ea4f 
This issue seems to happen more frequently, don't report it unless
all errors are enabled.

This warning means that some PCI devices might be missing.
That's not critical enough to warn users (most of them know nothing about hwloc).
For the record, CUDA init warnings were demoted the same a couple releases ago.

Refs open-mpi#354

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 69fbc20)
@bgoglin
core: if debug is enabled and debug messages are disabled, unhide err…
346d231 
…ors too

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 64e01e8)
@bgoglin
xml: also demote 32bits pci domain import errors to non-critical
d8f96b3 
Forgotten in commit 69fbc20

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c100a52)
@bgoglin
xml: fix an error message in the future xml obj id attribute
4a782cc 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 3329842)
@bgoglin
completion: fix diff --refname
2475694 
Don't fallthough to xml filenames after setting COMPREPLY for --refname.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c57d0fe)
@bgoglin
bitmap.h: fix the doc for foreach()
0d4a0d2 
We were still using index instead of id.

Thanks to Guillaume Mercier.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 61083f1)
@bgoglin
doc/examples: add memory-attributes.c
f0a9246 
Closes open-mpi#542

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 8d81da3)
@bgoglin
memattrs.h: cleanup doxygen formatting of the enum of memory attributes
f2ae31a 
Escape double-quotes in comments (they break the parser and the remaining
enum item doc is ignored in manpages).

Hide initializer values.

Uniformize the source formatting to keep it easy to read even if the
manpage formatting isn't perfect (looks like doxygen/manpage wants
only two paragraph per enum item, but we'd have to change lots of
other places, and it would make the source harder to read).

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit d316fa7)
@bgoglin
doc/examples: use hwloc_uint64_t in memory-attributes.c
676f9db 
MinGW doesn't get uint64_t on our CI.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 601b58d)
@bgoglin
nvml/bw: add nvlink versions 3&4
060a4a1 
Possible version values aren't documented in NVML API doc,
but wikipedia and other sources say 50GB/s per link
for v3 (Ampere) and v4 (Hopper).

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 352348a)
@bgoglin
tests/backends: fix build with ClangCI 15 compiler frontend for MSVC
863e887 
hwloc_backends.c needs some open/read/close/mktemp redirection
to _foo. Copy what we did in pci-common.c and add mktemp.

Thanks to Mario Emmenlauer for the report.

Fixes open-mpi#546

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 0de22a5)
@bgoglin
completion/lstopo: update the list of output formats
ba439ea 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 3b5d5d8)
@bgoglin
cuda: update to compute capability 9
3cbcde1 
128 FP32 cores again.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit a908085)
@hzhou @bgoglin
Leak in hwloc__xml_import_cpukind
b6cf3af 
Looks like we always need free the locally allocated infos.

(cherry picked from commit b86f63f)
@bgoglin
lstopo/draw: fix the displaying of non-HBM L0 device memory
f58797e 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 4630002)
@bgoglin
cuda: update for compute capability 8.9
a28cec6 
And 8.7 is officially documented now.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 604fdd5)
@bgoglin
doxy: number of character +s
a5bbb02 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e84553d)
@bgoglin
diff: reuse temp variable
637927f 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit b5c07dd)
@bgoglin
pci: don't ignore CXL memory expanders
69d5d8b 
And use "CXLMem" as a string for PCI_CLASS_MEMORY_CXL = 0x0502

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e0f56e2)
@bgoglin
utils/gather-topology: gather cxl devices and drivers
20c4d4a 
We'll need to double-check whether they are useful in the end.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit a172c84)
@bgoglin
NEWS: bring 2.7.2 bullets
802b625 
Signed-off-by: Brice Goglin <[email protected]>
bgoglin and others added 5 commits January 3, 2023 13:14
@bgoglin
contrib/*/check-versions.sh: add --update to usage
c6f6ed8 
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c938d81)
@bgoglin
contrib/android/check-versions.sh: make a remaining verbose message c…
f588db9 
…onditional

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 471e43d)
@bgoglin
contrib/dist/make_dist_tarball: update android/windows VERSION too
f2ec6ef 
HWLOC_VERSION* macros were X.Yrc1 instead of X.Y in the MSVC builds.

Windows CMAKE builds don't include suffixes such as rc1, but we still
update the hwloc version there for consistency (and in case X.Y was
changed too).

Android isn't updated since it isn't distributed in tarballs.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit accb702)
@bgoglin
darwin: don't forget to filter perflevel caches
cfab985 
Forgotten in f7c9aa8

Thanks to Michel Lesoinne for the report.

Closes open-mpi#564

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit aa0ef16)
@FunMiles
feat: Create a CMakeLists.txt file
1a5a66d 
Currently only effective for MacOS. Hoping it can be improved to become general for more systems.
@FunMiles FunMiles mentioned this pull request Feb 1, 2023
Open
@bgoglin
Copy link
Contributor

bgoglin commented Feb 1, 2023

What do you mean "standalone" or "subproject"? It's cmake-specific ways to use it?

Do you need/want to make this for more than just MacOS X? Do you need/want more than just the library? I'd like to keep things simple and easy to test and maintain. Hence supporting less things would be nice. Building/running most tests under tests/hwloc shouldn't be too hard if only supporting Mac OS X. Running other tests may require to build tools, at least lstopo. Building lstopo requires some configuration/detection, for instance because of all lstopo backends (cairo, etc). Building lstopo in minimalistic mode (no cairo, no X11, etc) is easier (and will be enough for some tests).

@FunMiles
Copy link
Author

FunMiles commented Feb 1, 2023 •
edited
Loading

What do you mean "standalone" or "subproject"? It's cmake-specific ways to use it?

Standalone vs subproject is a CMake specific issue. You can put the source of a library within your own project and then bring in the CMakeLists.txt of the subproject within your compilation. If you do this, you generally may not want that all the testing code be built, nor that the subproject library be installed. As the use via FetchContent mechanism does bring the library as a subproject, I wanted to make sure the build is clean for that case.
It is nowadays good CMake practice that any library project should be build in such a way that both form of use are cleanly supported.

Do you need/want to make this for more than just MacOS X? Do you need/want more than just the library? I'd like to keep things simple and easy to test and maintain. Hence supporting less things would be nice. Building/running most tests under tests/hwloc shouldn't be too hard if only supporting Mac OS X. Running other tests may require to build tools, at least lstopo. Building lstopo requires some configuration/detection, for instance because of all lstopo backends (cairo, etc). Building lstopo in minimalistic mode (no cairo, no X11, etc) is easier (and will be enough for some tests).

I would like to make this for MacOS, Linux and Windows at least.
It is probably best to limit the tests initially. I should be able to add Cairo/X11 detection fairly easily (at least for systems to which I have access) and supporting all the standard tools should be a goal.

@bgoglin
Copy link
Contributor

bgoglin commented Feb 1, 2023

Full Linux support would duplicate looooots of configure checks, and testing them will be very difficult because the target platforms are not in our CI and/or hardly accessible for us.

@FunMiles
Copy link
Author

FunMiles commented Feb 1, 2023 •
edited
Loading

Full Linux support would duplicate looooots of configure checks, and testing them will be very difficult because the target platforms are not in our CI and/or hardly accessible for us.

I am sure you are correct. Let's start small, but at least support basic Linux configs? A lot of code in Linux uses CMake. I could put some warning output messages when running CMake for Linux users stating what is not yet supported/tested.

@jsquyres
Copy link
Member

jsquyres commented Feb 1, 2023 •
edited
Loading

I am sure you are correct. Let's start small, but at least support basic Linux configs?

It's not really a question of only supporting a smaller set of features. Many of the configure tests are for portability between all the different distros / versions of Linux. E.g., the same feature XYZ may have differences between different distros and versions. Linux != Linux != Linux, unfortunately.

A lot of code in Linux uses CMake.

Just to be clear: all the code in Linux that uses CMake can still use hwloc as it is. The justifications for using CMake is not about making it available to users who currently cannot use hwloc because it is built with the Autotools.

JackBoosY
JackBoosY reviewed Feb 2, 2023
View reviewed changes
contrib/windows-cmake/CMakeLists.txt
@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 3.15)
Copy link

Choose a reason for hiding this comment

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

I think this file is not needed, right?

@FunMiles
Copy link
Author

I am sure you are correct. Let's start small, but at least support basic Linux configs?

It's not really a question of only supporting a smaller set of features. Many of the configure tests are for portability between all the different distros / versions of Linux. E.g., the same feature XYZ may have differences between different distros and versions. Linux != Linux != Linux, unfortunately.

A lot of code in Linux uses CMake.

Just to be clear: all the code in Linux that uses CMake can still use hwloc as it is. The justifications for using CMake is not about making it available to users who currently cannot use hwloc because it is built with the Autotools.

I've been away for a long time, but is there an easy way to get a list for all the differences that impact hwloc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackBoosY JackBoosY JackBoosY left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@FunMiles @bgoglin @jsquyres @JackBoosY @hzhou