Retrieve HWLOC API version from hwloc.h header #492

kswiecicki · 2024-05-13T10:36:59Z

The FindLIBHWLOC.cmake module previously used the config.h header to retrieve the HWLOC library version. This header was not always present depending on how the library was distributed. To address this issue, we now rely on the HWLOC API version retrieved from hwloc.h, the main HWLOC library header.

Description

Checklist

Code compiles without errors locally
All tests pass locally
CI workflows execute properly

lukaszstolarczuk · 2024-05-13T15:18:52Z

I've tested this change on my fork with 2 scenarios (in qemu workflow):

hwloc from source (as it was before my multi-numa tests change) link - 2.3.0
hwloc from apt, without pkg-config link - 2.8.0

Both scenarios properly reported the version.

On the side note, we still use 2.3.0 as our minimum, but it won't work with many_preferred, I guess (as I wrote here)

lplewa · 2024-05-13T15:57:02Z

cmake/FindLIBHWLOC.cmake

+    if(EXISTS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h")
+        file(STRINGS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h" LIBHWLOC_API_VERSION
+             REGEX "#define[ \t]HWLOC_API_VERSION[ \t]0x[0-9]+")
+        string(REGEX
+               REPLACE "#define[ \t]HWLOC_API_VERSION[ \t](0x[0-9]+)" "\\1"
+                       LIBHWLOC_API_VERSION "${LIBHWLOC_API_VERSION}")
+
+        math(EXPR LIBHWLOC_API_PATCH "(${LIBHWLOC_API_VERSION} & 0xFF)")
+        math(EXPR LIBHWLOC_API_MINOR "(${LIBHWLOC_API_VERSION} & 0xFF00) >> 8")
+        math(EXPR LIBHWLOC_API_MAJOR
+             "(${LIBHWLOC_API_VERSION} & 0xFF0000) >> 16")
+
+        set(LIBHWLOC_API_VERSION
+            "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")
+


NIT: parsing c header in cmake by using regex is a red flag for me - we could just create C program to extract this information - i.e like one bellow. I'm not fully convinced that this is better - but regex and math inside of cmake also are not to great.

Following code is AI generated and might not work. It might be better to just create support tool and compile it normally instead of using try_run - but i'm leaving this to your consideration.

Suggested change

if(EXISTS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h")

file(STRINGS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h" LIBHWLOC_API_VERSION

REGEX "#define[ \t]HWLOC_API_VERSION[ \t]0x[0-9]+")

string(REGEX

REPLACE "#define[ \t]HWLOC_API_VERSION[ \t](0x[0-9]+)" "\\1"

LIBHWLOC_API_VERSION "${LIBHWLOC_API_VERSION}")

math(EXPR LIBHWLOC_API_PATCH "(${LIBHWLOC_API_VERSION} & 0xFF)")

math(EXPR LIBHWLOC_API_MINOR "(${LIBHWLOC_API_VERSION} & 0xFF00) >> 8")

math(EXPR LIBHWLOC_API_MAJOR

"(${LIBHWLOC_API_VERSION} & 0xFF0000) >> 16")

set(LIBHWLOC_API_VERSION

"${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")

# Define the C code that accepts an argument for the version part

set(HWLOC_VERSION_CODE "

#include <hwloc.h>

#include <stdlib.h>

int main(int argc, char* argv[]) {

int part = atoi(argv[1]); // Convert the argument to an integer

if(part == 0) {

return (HWLOC_API_VERSION & 0xFF0000) >> 16; // Major version

} else if(part == 1) {

return (HWLOC_API_VERSION & 0xFF00) >> 8; // Minor version

}

return (HWLOC_API_VERSION & 0xFF); // Patch version

}")

foreach(idx RANGE 0 2)

try_run(RUN_RESULT_VAR COMPILE_RESULT_VAR

${CMAKE_BINARY_DIR}

"${HWLOC_VERSION_CODE}"

ARGS ${idx}

CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${LIBHWLOC_INCLUDE_DIR}"

COMPILE_OUTPUT_VARIABLE COMPILE_OUTPUT

RUN_OUTPUT_VARIABLE VERSION_OUTPUT)

if(${idx} EQUAL 0)

set(LIBHWLOC_API_MAJOR ${VERSION_OUTPUT})

elseif(${idx} EQUAL 1)

set(LIBHWLOC_API_MINOR ${VERSION_OUTPUT})

else()

set(LIBHWLOC_API_PATCH ${VERSION_OUTPUT})

endif()

endforeach()

set(LIBHWLOC_API_VERSION "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")

an approach with separate program (also AI generated not tested)

#include <stdio.h> #include <stdlib.h> #include "hwloc.h" int main(int argc, char** argv) { int major = (HWLOC_API_VERSION & 0xFF0000) >> 16; int minor = (HWLOC_API_VERSION & 0xFF00) >> 8; int patch = (HWLOC_API_VERSION & 0xFF); int component = atoi(argv[1]); // Convert the argument to an integer if(component == 0) { return major; } else if(component == 1) { return minor; } else { return patch; } }

if(EXISTS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h") # Create an executable for extracting the version add_executable(hwloc_version_extractor hwloc_version_extractor.c) target_include_directories(hwloc_version_extractor PRIVATE ${LIBHWLOC_INCLUDE_DIR}) # Execute the program for each version component foreach(version_idx IN ITEMS 0 1 2) execute_process(COMMAND hwloc_version_extractor ${version_idx} RESULT_VARIABLE result OUTPUT_VARIABLE output_var ERROR_QUIET OUTPUT_STRIP_TRAILING_WHITESPACE) # Assigning the exit code directly to the version component variable if(version_idx EQUAL 0) set(LIBHWLOC_API_MAJOR ${result}) elseif(version_idx EQUAL 1) set(LIBHWLOC_API_MINOR ${result}) else() set(LIBHWLOC_API_PATCH ${result}) endif() endforeach() # Combine the version components set(LIBHWLOC_API_VERSION "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}") endif()

TBH we could just do entire version check by using c program. (create program which fails to compile if HWLOC version is to low)

KFilipek · 2024-05-13T16:09:55Z

cmake/FindLIBHWLOC.cmake

+    if(EXISTS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h")
+        file(STRINGS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h" LIBHWLOC_API_VERSION
+             REGEX "#define[ \t]HWLOC_API_VERSION[ \t]0x[0-9]+")
+        string(REGEX
+               REPLACE "#define[ \t]HWLOC_API_VERSION[ \t](0x[0-9]+)" "\\1"
+                       LIBHWLOC_API_VERSION "${LIBHWLOC_API_VERSION}")
+
+        math(EXPR LIBHWLOC_API_PATCH "(${LIBHWLOC_API_VERSION} & 0xFF)")
+        math(EXPR LIBHWLOC_API_MINOR "(${LIBHWLOC_API_VERSION} & 0xFF00) >> 8")
+        math(EXPR LIBHWLOC_API_MAJOR
+             "(${LIBHWLOC_API_VERSION} & 0xFF0000) >> 16")
+
+        set(LIBHWLOC_API_VERSION
+            "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")
+


There is a more easy way, I think. Sometimes I heard about writing code to determine the version, so I wrote the following:
CmakeLists.txt

project(test LANGUAGES C CXX) cmake_minimum_required(VERSION 3.22) # Include the header file set(CMAKE_REQUIRED_FLAGS "-std=c++17") include(CheckCXXSourceCompiles) try_run( HWLOC_RUN_RESULT # Variable to store the run result (exit code) HWLOC_COMPILE_RESULT # Variable to store the compile result (TRUE/FALSE) "${CMAKE_SOURCE_DIR}" # Directory to build the test executable SOURCES "${CMAKE_SOURCE_DIR}/main.cpp" RUN_OUTPUT_VARIABLE HWLOC_RUN_OUTPUT # Variable to store the output of the test executable ) if(HWLOC_COMPILE_RESULT) message("hwloc version: ${HWLOC_RUN_OUTPUT}") else() message("Failed to determine hwloc version") endif()

and main.cpp

#include <cstdio> #include "hwloc.h" int main() { // printf("Value: %d", HWLOC_API_VERSION); int LIBHWLOC_API_PATCH = HWLOC_API_VERSION & 0xFF; int LIBHWLOC_API_MINOR = (HWLOC_API_VERSION >> 8) & 0xFF; int LIBHWLOC_API_MAJOR = (HWLOC_API_VERSION >> 16) & 0xFF; printf("%d.%d.%d", LIBHWLOC_API_MAJOR, LIBHWLOC_API_MINOR, LIBHWLOC_API_PATCH); return 0; }

Notice, I've created own hwloc.h file with: #define HWLOC_API_VERSION 0x00030000 for development purposes, so #include "hwloc.h" can be changed to other value.

kswiecicki · 2024-05-14T09:29:24Z

@lplewa @KFilipek thanks for the suggestions. I've combined your solutions. Unfortunately, CMake 3.14 doesn't support building source code from a CMake variable. As a workaround, I had CMake write this code into a file in the build directory.

@lukaszstolarczuk could I ask you to test this solution with the scenarios you tested previously?

KFilipek

LGTM, always you can leave our footprint as a coauthors :)

The FindLIBHWLOC.cmake module previously used the config.h header to retrieve the HWLOC library version. This header was not always present depending on how the library was distributed. To address this issue, we now rely on the HWLOC API version retrieved from hwloc.h, the main HWLOC library header. Co-authored-by: Łukasz Plewa <[email protected]> Co-authored-by: Krzysztof Filipek <[email protected]>

kswiecicki · 2024-05-14T10:57:16Z

LGTM, always you can leave our footprint as a coauthors :)

Done :).

cmake/FindLIBHWLOC.cmake

lukaszstolarczuk · 2024-05-14T13:20:30Z

nonetheless, my tests passed:

https://github.com/lukaszstolarczuk/unified-memory-framework/actions/runs/9079982583/job/24950365881#step:13:626
&&
https://github.com/lukaszstolarczuk/unified-memory-framework/actions/runs/9079966647/job/24950313533#step:13:1631

lukaszstolarczuk · 2024-05-15T12:16:07Z

@igchor, you can tests this on your PR in UR

lukaszstolarczuk approved these changes May 13, 2024

View reviewed changes

lplewa reviewed May 13, 2024

View reviewed changes

KFilipek requested changes May 13, 2024

View reviewed changes

kswiecicki force-pushed the hwloc-cmake-version-fix branch from 4a1d10b to 843d0aa Compare May 14, 2024 09:19

kswiecicki marked this pull request as ready for review May 14, 2024 09:30

kswiecicki requested a review from a team as a code owner May 14, 2024 09:30

KFilipek approved these changes May 14, 2024

View reviewed changes

kswiecicki force-pushed the hwloc-cmake-version-fix branch from 843d0aa to 9db0d3f Compare May 14, 2024 10:53

kswiecicki force-pushed the hwloc-cmake-version-fix branch from 9db0d3f to 5b35db5 Compare May 14, 2024 10:55

lplewa approved these changes May 14, 2024

View reviewed changes

lukaszstolarczuk reviewed May 14, 2024

View reviewed changes

cmake/FindLIBHWLOC.cmake Show resolved Hide resolved

PatKamin approved these changes May 15, 2024

View reviewed changes

lukaszstolarczuk merged commit 7f18587 into oneapi-src:main May 15, 2024
56 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Retrieve HWLOC API version from hwloc.h header #492

Retrieve HWLOC API version from hwloc.h header #492

Uh oh!

kswiecicki commented May 13, 2024 •

edited

Loading

Uh oh!

lukaszstolarczuk commented May 13, 2024

Uh oh!

lplewa May 13, 2024

Uh oh!

lplewa May 13, 2024 •

edited

Loading

Uh oh!

KFilipek May 13, 2024 •

edited

Loading

Uh oh!

KFilipek May 13, 2024

Uh oh!

kswiecicki commented May 14, 2024

Uh oh!

KFilipek left a comment •

edited

Loading

Uh oh!

kswiecicki commented May 14, 2024

Uh oh!

Uh oh!

lukaszstolarczuk commented May 14, 2024

Uh oh!

Uh oh!

lukaszstolarczuk commented May 15, 2024

Uh oh!

Uh oh!

-    if(EXISTS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h")
-        file(STRINGS "${LIBHWLOC_INCLUDE_DIR}/hwloc.h" LIBHWLOC_API_VERSION
-             REGEX "#define[ \t]HWLOC_API_VERSION[ \t]0x[0-9]+")
-        string(REGEX
-               REPLACE "#define[ \t]HWLOC_API_VERSION[ \t](0x[0-9]+)" "\\1"
-                       LIBHWLOC_API_VERSION "${LIBHWLOC_API_VERSION}")
-        math(EXPR LIBHWLOC_API_PATCH "(${LIBHWLOC_API_VERSION} & 0xFF)")
-        math(EXPR LIBHWLOC_API_MINOR "(${LIBHWLOC_API_VERSION} & 0xFF00) >> 8")
-        math(EXPR LIBHWLOC_API_MAJOR
-             "(${LIBHWLOC_API_VERSION} & 0xFF0000) >> 16")
-        set(LIBHWLOC_API_VERSION
-            "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")
+    # Define the C code that accepts an argument for the version part
+    set(HWLOC_VERSION_CODE "
+#include <hwloc.h>
+#include <stdlib.h>
+int main(int argc, char* argv[]) {
+    int part = atoi(argv[1]); // Convert the argument to an integer
+    if(part == 0) {
+        return (HWLOC_API_VERSION & 0xFF0000) >> 16; // Major version
+    } else if(part == 1) {
+        return (HWLOC_API_VERSION & 0xFF00) >> 8; // Minor version
+    }
+    return (HWLOC_API_VERSION & 0xFF); // Patch version
+}")
+    foreach(idx RANGE 0 2)
+        try_run(RUN_RESULT_VAR COMPILE_RESULT_VAR
+                ${CMAKE_BINARY_DIR}
+                "${HWLOC_VERSION_CODE}"
+                ARGS ${idx}
+                CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${LIBHWLOC_INCLUDE_DIR}"
+                COMPILE_OUTPUT_VARIABLE COMPILE_OUTPUT
+                RUN_OUTPUT_VARIABLE VERSION_OUTPUT)
+        if(${idx} EQUAL 0)
+            set(LIBHWLOC_API_MAJOR ${VERSION_OUTPUT})
+        elseif(${idx} EQUAL 1)
+            set(LIBHWLOC_API_MINOR ${VERSION_OUTPUT})
+        else()
+            set(LIBHWLOC_API_PATCH ${VERSION_OUTPUT})
+        endif()
+    endforeach()
+    set(LIBHWLOC_API_VERSION "${LIBHWLOC_API_MAJOR}.${LIBHWLOC_API_MINOR}.${LIBHWLOC_API_PATCH}")

Retrieve HWLOC API version from hwloc.h header #492

Retrieve HWLOC API version from hwloc.h header #492

Uh oh!

Conversation

kswiecicki commented May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

lukaszstolarczuk commented May 13, 2024

Uh oh!

lplewa May 13, 2024

Choose a reason for hiding this comment

Uh oh!

lplewa May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KFilipek May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KFilipek May 13, 2024

Choose a reason for hiding this comment

Uh oh!

kswiecicki commented May 14, 2024

Uh oh!

KFilipek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kswiecicki commented May 14, 2024

Uh oh!

Uh oh!

lukaszstolarczuk commented May 14, 2024

Uh oh!

Uh oh!

lukaszstolarczuk commented May 15, 2024

Uh oh!

Uh oh!

kswiecicki commented May 13, 2024 •

edited

Loading

lplewa May 13, 2024 •

edited

Loading

KFilipek May 13, 2024 •

edited

Loading

KFilipek left a comment •

edited

Loading