Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional #33355

VinInn · 2021-04-07T11:40:13Z

Resolves #32746

only one test affected.
I verified that no timing regression occur anyhow (so pragma not really needed)

VinInn · 2021-04-07T11:40:36Z

@cmsbuild please test

cmsbuild · 2021-04-07T11:47:12Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33355/21942

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33355/21942/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33355/21942/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

VinInn · 2021-04-07T12:16:06Z

compiling with -DUSE_VECTORIZATION_PRAGMA -Rpass=loop-vectorize will produce

src/DataFormats/Math/test/CholeskyInvert_t.cpp:81:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (auto& m : mm) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:75:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (unsigned int i = 0; i < SIZE; ++i) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:114:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (auto& m : mm) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:103:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (unsigned int i = 0; i < SIZE; ++i) {
      ^
>> Building binary DataFormatsFastMath_t
src/DataFormats/Math/test/CholeskyInvert_t.cpp:81:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (auto& m : mm) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:75:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (unsigned int i = 0; i < SIZE; ++i) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:103:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (unsigned int i = 0; i < SIZE; ++i) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:114:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (auto& m : mm) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:81:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (auto& m : mm) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:75:5: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
    for (unsigned int i = 0; i < SIZE; ++i) {
    ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:103:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (unsigned int i = 0; i < SIZE; ++i) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:114:7: remark: vectorized loop (vectorization width: 2, interleaved count: 1) [-Rpass=loop-vectorize]
      for (auto& m : mm) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:114:7: warning: loop not vectorized: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Wpass-failed=transform-warning]
src/DataFormats/Math/test/CholeskyInvert_t.cpp:103:7: warning: loop not vectorized: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Wpass-failed=transform-warning]
      for (unsigned int i = 0; i < SIZE; ++i) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:114:7: warning: loop not vectorized: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Wpass-failed=transform-warning]
      for (auto& m : mm) {
      ^
src/DataFormats/Math/test/CholeskyInvert_t.cpp:103:7: warning: loop not vectorized: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Wpass-failed=transform-warning]
      for (unsigned int i = 0; i < SIZE; ++i) {
      ^

that shows that llvm vectorizes for ranks 2,3,4 and not for 5,6
My recollection is that GCC was vectorizing rank 5 as well

cmsbuild · 2021-04-07T12:19:36Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33355/21943

This PR adds an extra 16KB to repository

cmsbuild · 2021-04-07T12:19:58Z

A new Pull Request was created by @VinInn (Vincenzo Innocente) for master.

It involves the following packages:

DataFormats/Math

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @fabiocos, @rovere this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

VinInn · 2021-04-07T12:22:39Z

@cmsbuild please test

VinInn · 2021-04-07T12:47:10Z

@cmsbuild please test

cmsbuild · 2021-04-07T12:52:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33355/21945

This PR adds an extra 20KB to repository

cmsbuild · 2021-04-07T12:52:55Z

Pull request #33355 was updated. @perrotta, @jpata, @slava77 can you please check and sign again.

VinInn · 2021-04-07T13:41:24Z

how to ask a clang test?

cmsbuild · 2021-04-07T15:53:22Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0a1fdc/14068/summary.html
COMMIT: 57d7dfd
CMSSW: CMSSW_11_3_X_2021-04-06-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33355/14068/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640868
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2640839
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

mrodozov · 2021-04-07T19:49:40Z

please test for CMSSW_11_3_CLANG_X
you test it against a clang release which will bring the clang compiler
Could you edit your initial comment first line:
Adresses issue #32746 -> Resolves #32746
which will close the issue when this is merged

smuzaffar · 2021-04-07T20:10:56Z

Adresses issue #32746 -> Resolves #32746

done @mrodozov

cmsbuild · 2021-04-07T23:24:02Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0a1fdc/14078/summary.html
COMMIT: 57d7dfd
CMSSW: CMSSW_11_3_CLANG_X_2021-04-06-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33355/14078/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 4798 lines to the logs
Reco comparison results: 17418 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640868
DQMHistoTests: Total failures: 84613
DQMHistoTests: Total nulls: 6
DQMHistoTests: Total successes: 2556227
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.02 KiB( 36 files compared)
DQMHistoSizes: changed ( 136.793 ): 0.004 KiB JetMET/SUSYDQM
DQMHistoSizes: changed ( 136.874 ): 0.016 KiB JetMET/SUSYDQM
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: found differences in 1 / 36 workflows

slava77 · 2021-04-08T17:08:41Z

+reconstruction

for #33355 57d7dfd

code changes are in line with the PR description
jenkins tests pass and comparisons with the baseline show no differences (in the regular build; ignoring differences in the CLANG test due to a non-CLANG reference used)
CMSSW_11_3_CLANG_X build log does not show a warning for CholeskyInvert_t.cpp anymore (present in the IB, e.g. https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc900/CMSSW_11_3_CLANG_X_2021-04-04-2300/DataFormats/Math )

cmsbuild · 2021-04-08T17:09:04Z

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-04-08T23:57:43Z

+1

make pragma optional

5e83fae

cmsbuild added this to the CMSSW_11_3_X milestone Apr 7, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 7, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 7, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 7, 2021

code format

61b7d1e

cmsbuild added code-checks-pending tests-pending and removed code-checks-rejected tests-started labels Apr 7, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 7, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 7, 2021

force inline for all ranks

ee7280b

cmsbuild added code-checks-pending and removed code-checks-approved tests-started labels Apr 7, 2021

cmsbuild added code-checks-pending tests-pending and removed code-checks-rejected tests-started labels Apr 7, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 7, 2021

VinInn changed the title ~~Solve vectorization issue by force inlining and Silence CLANG Warning by making pragma optional~~ Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional Apr 7, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 7, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 7, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Apr 8, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 8, 2021

cmsbuild merged commit 55c4efe into cms-sw:master Apr 8, 2021

cmsbuild mentioned this pull request Apr 9, 2021

Added time info to the displaced vertexes for muon guns #33382

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional #33355

Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional #33355

VinInn commented Apr 7, 2021 •

edited by smuzaffar

Loading

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021 •

edited

Loading

cmsbuild commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

mrodozov commented Apr 7, 2021 •

edited

Loading

smuzaffar commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

slava77 commented Apr 8, 2021 •

edited

Loading

cmsbuild commented Apr 8, 2021

qliphy commented Apr 8, 2021

Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional #33355

Solve vectorization issue by forcing inlining and Silence CLANG Warning by making pragma optional #33355

Conversation

VinInn commented Apr 7, 2021 • edited by smuzaffar Loading

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021 • edited Loading

cmsbuild commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

VinInn commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

Comparison Summary

mrodozov commented Apr 7, 2021 • edited Loading

smuzaffar commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

Comparison Summary

slava77 commented Apr 8, 2021 • edited Loading

cmsbuild commented Apr 8, 2021

qliphy commented Apr 8, 2021

VinInn commented Apr 7, 2021 •

edited by smuzaffar

Loading

VinInn commented Apr 7, 2021 •

edited

Loading

mrodozov commented Apr 7, 2021 •

edited

Loading

slava77 commented Apr 8, 2021 •

edited

Loading