-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Updating LLVM to version 18.1 for CMSSW_14_2_X #45870
Comments
cms-bot internal usage |
A new Issue was created by @smuzaffar. @Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
Shouldn't the trailing |
There are some other kind of changes: diff --git a/HeterogeneousCore/AlpakaInterface/interface/CachedBufAlloc.h b/HeterogeneousCore/AlpakaInterface/interface/CachedBufAlloc.h
index 36aa8ec079b..64b518e02c2 100644
--- a/HeterogeneousCore/AlpakaInterface/interface/CachedBufAlloc.h
+++ b/HeterogeneousCore/AlpakaInterface/interface/CachedBufAlloc.h
@@ -26,8 +26,9 @@ namespace cms::alpakatools {
template <typename TElem, typename TDim, typename TIdx, typename TQueue>
struct CachedBufAlloc<TElem, TDim, TIdx, alpaka::DevCpu, TQueue, void> {
template <typename TExtent>
- ALPAKA_FN_HOST static auto allocCachedBuf(alpaka::DevCpu const& dev, TQueue queue, TExtent const& extent)
- -> alpaka::BufCpu<TElem, TDim, TIdx> {
+ ALPAKA_FN_HOST static auto allocCachedBuf(alpaka::DevCpu const& dev,
+ TQueue queue,
+ TExtent const& extent) -> alpaka::BufCpu<TElem, TDim, TIdx> {
// non-cached, queue-ordered asynchronous host-only memory
return alpaka::allocAsyncBuf<TElem, TIdx>(queue, extent);
} (I don't mind, just pointing it out) |
ah right, sorry I missed that one. The change in #45870 (comment) is also only suggested by llvm18 and llvm17 will revert it |
yes, I agree |
There is |
@smuzaffar for development work in a personal branch based on a branch of 14_1, this PR causes a lot of git conflicts, as LLVM 18.1 changes many lines of code. Can you advise how people can run the LLVM 18.1 corrections on their personal 14_1 branches, before they make a PR to master, to avoid this? |
@tomalin , first I would suggest to move the development to 14.2.X if changes are to be included in master branch.
Note that there are some manual changes required too e.g. removing the |
@smuzaffar Have we concluded everything here? (i.e. can we close the issue?) |
+core |
This issue is fully signed and ready to be closed. |
@cms-sw/all-l2 , we are going to update llvm to version 18.1.6 for
CMSSW_14_2_X
. The new clang-format inLLVM 18.1
proposes couple of new changes which were not handled properly by llvm 17.(){} -> () {}
: clang-format in llvm 17 was only adding the space it there is no;
after the function definition e.g. for https://github.com/cms-sw/cmssw/blob/master/DataFormats/Math/interface/Graph.h#L121-L122llvm 17 was adding the extra space but not for https://github.com/cms-sw/cmssw/blob/master/Alignment/CocoaDDLObjects/interface/CocoaMaterialElementary.h#L20
llvm 18 has fixed this and now clang-format properly handle this case.
List of all clang-format changes (2148 files in total) are available here. Once llvm 18 is part of 14.2.X IBs then @cms-sw/core-l2 team will open code format updates PRs for files which are not already touched by any existing PR. Note that these changes llvm18 based code-format changes should be done once llvm 18 is in IBs otherwise PR code-checks using llvm17 will suggest to revert these changes.
Let us know if you have any concerns
The text was updated successfully, but these errors were encountered: