-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[OpenMP] Fix: Add missing preprocessor guard for hwloc types on macOS #156708
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
Conversation
@bursow I believe there is more than one of those guards missing. I only mentioned one example in my issue. |
Can you write those too, I'll check them. Thanks |
@bursow 🤔 $ grep --line-number "if KMP_USE_HWLOC$" openmp/runtime/src/kmp_alloc.cpp
17:#if KMP_USE_HWLOC
1548:#if KMP_USE_HWLOC
1683:#if KMP_USE_HWLOC
1983:#if KMP_USE_HWLOC
2204:#if KMP_USE_HWLOC
2342:#if KMP_USE_HWLOC
2381:#if KMP_USE_HWLOC |
I checked those other lines as well, but they do not use the hwloc_membind_policy_t type, so no further changes were needed. Thanks for the suggestion! |
@bursow if the |
You're absolutely right, thanks for pointing that out. I was focused only on the specific build error, but I see now that the issue is broader. I'll update the other if KMP_USE_HWLOC blocks to also include the KMP_AFFINITY_SUPPORTED guard, which will fix any potential issues with other hwloc symbols like __kmp_hwloc_topology. Thanks for the detailed feedback. |
I don't know another polite way to say this, but this exact exchange is what I would expect from an LLM, not from someone who contributes to LLVM itself... |
@@ -1563,6 +1563,7 @@ static bool __kmp_is_hwloc_membind_supported(hwloc_membind_policy_t policy) { | |||
return false; | |||
#endif | |||
} | |||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks wrong. The #if
above was already there, so adding an extra #endif
will definitely break something.
Fix #156679
The issue stems from the __kmp_is_hwloc_membind_supported function in openmp/runtime/src/kmp_alloc.cpp. The function uses a type from the hwloc library (hwloc_membind_policy_t), but it was only guarded by #if KMP_USE_HWLOC. On macOS, KMP_AFFINITY_SUPPORTED is not defined, so the necessary header file (hwloc.h) isn't included, leading to an unknown type error.
The fix is to update the preprocessor guard to #if KMP_USE_HWLOC && KMP_AFFINITY_SUPPORTED, ensuring the function is only compiled when the hwloc type is available.