-
Notifications
You must be signed in to change notification settings - Fork 738
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
[UR] Bump UR version and enable dynamic linking with UMF #13343
Conversation
@igchor Does this mean that we will be requiring ANYONE who use UR to install libhwloc on Linux and hwloc-win64-build-2.10.0 on windows? Can we somehow detect these, and don't switch to dynamic linking if these libs are not installed? |
Yes, that's correct (although the minimum required version is 2.3.0). The hwloc is required not because we link dynamically with UMF but because I've bumped UMF version we are using. This new UMF version has a dependency on hwloc which cannot be disabled. Will this be a problem for intel/llvm? |
Thanks @igchor for the clarification. Installing hwloc in CI/containers won't be a problem. But that also means sycl compilers and DPC++ will have to add a dependency to hwloc? If so, I think we have to update quite a bunch of documents to reflect that? Is that really what we want? FYI. @stdale-intel . |
@jsji you are correct on both ends. @uditagarwal97 updated the Windows CI machines, and Linux containers. Docs and other things are being updated to reflect this as well. |
Yes, and there's already been a discussion about how to handle this for DPC++.
@jsji do you want me to update any docs in intel/llvm in this PR? if yes, could you let me know which ones should I update? |
Thanks @igchor . I think @stdale-intel mentioned that "Docs and other things are being updated to reflect this as well." , not sure who is doing that, but I guess someone is already taking care of that. |
@igchor if you can take care of the intel/llvm doc PR that would be awesome! I'd start with editing https://github.com/intel/llvm/blob/sycl/sycl/doc/GetStartedGuide.md. But i'll see if there is other docs as well. |
Done. Please let me know if I should update anything else. |
Install hwloc which is required by UMF (a dependency of UR). This is needed by #13343
Explicitly set 'ZES_ENABLE_SYSMAN=0'. HWLOC initializes this environment variable in its constructor, causing this test to fail, as retrieving free memory information is expected not to work in this test.
1d617bf
to
8a5f65c
Compare
@intel/llvm-gatekeepers this is ready for merge from our side. The HIP failure is unrelated and is affecting all other PRs. |
@pbalcer this is waiting on approvals from intel/dpcpp-devops-reviewers, intel/dpcpp-doc-reviewers, and/or intel/llvm-reviewers-runtime |
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.
devops lgtm given stewart has approved
@martygrant, since @sarnex and @uditagarwal97 have given their approval, I believe this is ready to be merged |
Don't we still need |
I'm not sure, @martygrant said |
The CI seems to be saying |
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.
I think we should make it clear that the hwloc
prereq is a Linux-only requirement. Other than that, I think this is fine.
Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
@intel/llvm-gatekeepers this is ready for merge, I belive we have all the required approvals now |
@igchor, this PR broke our post-commit, could you please take a look? https://github.com/intel/llvm/actions/runs/10106492701
|
CI on macOS is also broken:
|
@pbalcer, would you be able to have a look at the above? |
We are working on a fix. Will create a PR soon. |
|
Testing PR for: oneapi-src/unified-runtime#1430 --------- Co-authored-by: Krzysztof Swiecicki <krzysztof.swiecicki@intel.com> Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
`hwloc` became a dependency after #13343 was merged. CI infrastructure has been updated to have `hwloc` installed, but it's still missing on macOS runner. Additionally, UMF compile options caused errors on SYCL runners that should be resolved after UR version bump included in this patch. Logs: https://github.com/intel/llvm/actions/runs/10106492701/job/27948711331 and https://github.com/intel/llvm/actions/runs/10106492701/job/27948711538.
Note this is NOT true, it turns out that we haven't have hwloc installed in ANY of our internal build system, so I am going to revert this in sycl-web first to unblock pulldown. This means this (and any new UR changes) will NOT be pulldown into downstream until we get the internal build system fixed , I have contacted our infrastructure team, hopefully they can help to install it ASAP. |
Testing PR for: oneapi-src/unified-runtime#1430