-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[MkFit] round up size arguments to aligned_alloc upwards to align value. #37111
[MkFit] round up size arguments to aligned_alloc upwards to align value. #37111
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37111/28609
|
A new Pull Request was created by @osschar (Matevž Tadel) for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Co-authored-by: Slava Krutelyov <[email protected]>
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37111/28611
|
Pull request #37111 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37111/28613
|
Pull request #37111 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again. |
@cmsbuild, please test for CMSSW_12_3_ASAN_X |
please test for slc7_aarch64_gcc11 lets tests the change RecoTracker/MkFitCore/src/Matriplex/MatriplexCommon.h for non-x86_64 archs too |
-1 Failed Tests: RelVals RelVals
|
I suppose, as in the previous update in #37014 these workflows are failing also in the aarch IB. |
-1 Failed Tests: RelVals RelVals
|
it seems like only phase-2 (non-mkFit) workflows failed. Apparently the ones with mkFit completed OK. |
I agree, and the phase2 workflow failures in ASAN occur also in IBs |
@cmsbuild, please test (regular tests too) |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e3d658/22754/summary.html Comparison SummarySummary:
|
+reconstruction
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
std::aligned_alloc calls were passing the desired allocated size, not rounded up to a multiple of align argument which seems to result in UB according to the standard.
All allocations in mkFit are aligned to cache-line (64) required for vector intrinsics.
This PR addresses #37096.