Skip to content

Conversation

@mrodozov
Copy link
Contributor

@mrodozov mrodozov commented Mar 1, 2021

Resolves: #6666
This builds the LLVM external

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2021

A new Pull Request was created by @mrodozov (Mircho Rodozov) for branch IB/CMSSW_11_3_X/master.

@cmsbuild, @smuzaffar, @mrodozov can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 2, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2021

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13182/summary.html
COMMIT: e2ab0c3
CMSSW: CMSSW_11_3_X_2021-03-01-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/6691/13182/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation error when building:

call_subprocess(
File "/build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/slc7_amd64_gcc900/external/py3-pip/20.3.3/lib/python3.8/site-packages/pip/_internal/utils/subprocess.py", line 240, in call_subprocess
raise InstallationError(exc_msg)
pip._internal.exceptions.InstallationError: Command errored out with exit status 1: /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/slc7_amd64_gcc900/external/python3/3.8.2-bcolbf/bin/python3 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/py2-llvmlite/0.33.0-00294e2e5be24bf0759475cbe506703a/cmsdist-tmp/pip-req-build-92zvxjsw/setup.py'"'"'; __file__='"'"'/build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/py2-llvmlite/0.33.0-00294e2e5be24bf0759475cbe506703a/cmsdist-tmp/pip-req-build-92zvxjsw/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/py2-llvmlite/0.33.0-00294e2e5be24bf0759475cbe506703a/cmsdist-tmp/pip-record-n8fwnlri/install-record.txt --single-version-externally-managed --user --prefix= --compile --install-headers /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/BUILDROOT/00294e2e5be24bf0759475cbe506703a/opt/cmssw/slc7_amd64_gcc900/external/py2-llvmlite/0.33.0-00294e2e5be24bf0759475cbe506703a/include/python3.8/llvmlite Check the logs for full command output.
Removed build tracker: '/build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/py2-llvmlite/0.33.0-00294e2e5be24bf0759475cbe506703a/cmsdist-tmp/pip-req-tracker-1xsgj6tj'
error: Bad exit status from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.kYud8a (%build)


RPM build errors:
Macro %rpmbuild_libdir defined but not used within scope
Bad exit status from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.kYud8a (%build)


@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2021

Pull request #6691 was updated.

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 8, 2021

please test
hopefully full toolconf will build.

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 8, 2021

please test for slc7_aarch64_gcc9

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 8, 2021

please test for slc7_ppc64le_gcc9

@smuzaffar
Copy link
Contributor

@mrodozov , you need to issue please test for slc7_aarch64_gcc9 again. In one iteration bot only recognize the last command. Normally you have to wait for the bot to ack your last command before issuing the next one :-)

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 8, 2021

please test for slc7_aarch64_gcc9

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2021

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13338/summary.html
COMMIT: de88813
CMSSW: CMSSW_11_3_X_2021-03-08-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/6691/13338/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test-clang-tidy had ERRORS
---> test testLLVMLite had ERRORS

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-c0a525/34634.0_TTbar_14TeV+2026D76+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-c0a525/34834.999_TTbar_14TeV+2026D76PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2849195
  • DQMHistoTests: Total failures: 10
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2849162
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2021

-1

Failed Tests: UnitTests RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13342/summary.html
COMMIT: de88813
CMSSW: CMSSW_11_3_X_2021-03-07-2300/slc7_ppc64le_gcc9
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/6691/13342/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testLLVMLite had ERRORS
---> test test-clang-tidy had ERRORS

RelVals

  • 11634.91111634.911_TTbar_14TeV+2021_DD4hep+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_TTbar_14TeV+2021_DD4hep+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2021

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13344/summary.html
COMMIT: de88813
CMSSW: CMSSW_11_3_X_2021-03-08-1100/slc7_aarch64_gcc9
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/6691/13344/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test-clang-tidy had ERRORS
---> test testLLVMLite had ERRORS

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 9, 2021

please test with cms-sw/cmssw#33110

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13481/summary.html
COMMIT: 6b600df
CMSSW: CMSSW_11_3_X_2021-03-12-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmsdist/6691/13481/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2635087
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2635052
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files

@silviodonato
Copy link
Contributor

@cms-sw/externals-l2 can we merge this PR with cms-sw/cmssw#33110? Do we need cms-sw/cmssw#33136 too?

@mrodozov
Copy link
Contributor Author

mrodozov commented Mar 15, 2021

cms-sw/cmssw#33110 and cms-sw/cmssw#33136 can and should be merged on their own . 33110 was updated to work with both llvm10 and 11 . I'll quote this (from above comments)

looks like new llvm generate few more warnings for CLANG build ( https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13406/build-logs/ ). Once ready we should first get llvm 11. 1 only CLANG builds

for which we have #6720

@smuzaffar
Copy link
Contributor

please test for CMSSW_11_3_CLANG_X

@smuzaffar
Copy link
Contributor

@mrodozov , @silviodonato , once we have cms-sw/cmssw#33171 merged then all the new build warnings should go away and we can merge it directly in the master IB.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: HeaderConsistency
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0a525/13524/summary.html
COMMIT: 6b600df
CMSSW: CMSSW_11_3_CLANG_X_2021-03-14-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/6691/13524/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 4274 lines to the logs
  • Reco comparison results: 17928 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2635087
  • DQMHistoTests: Total failures: 79903
  • DQMHistoTests: Total nulls: 7
  • DQMHistoTests: Total successes: 2555155
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.012 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 136.874 ): 0.016 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files

@smuzaffar
Copy link
Contributor

+externals
looks good to go, CLANG IBs will have an extra warnings which should be fixed when cms-sw/cmssw#33171 is merged

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_11_3_X/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)

@smuzaffar smuzaffar merged commit 4ae5869 into cms-sw:IB/CMSSW_11_3_X/master Mar 16, 2021
@davidlange6
Copy link
Contributor

Hi @smuzaffar @mrodozov - did you look at clang-tidy changes due to this upgrade? I've noticed a few unexpected changes in the last day (eg, here was a change needed after an unrelated commit was pushed yesterday cms-sw/cmssw#33189

@smuzaffar
Copy link
Contributor

yes I did run clang-tidy and format for this, see the following comments
#6691 (comment)
#6691 (comment)

Most of these are already reported by llvm 10 too.

@mrodozov mrodozov deleted the update-llvm-11.1 branch March 17, 2021 13:22
@davidlange6
Copy link
Contributor

davidlange6 commented Mar 17, 2021 via email

@smuzaffar
Copy link
Contributor

cms-sw/cmssw@04a1e0b is code format change. Looks like llvm 11 has improved on few formats . See the below links for code format changes for full cmssw

LLVM 10: https://cmssdt.cern.ch/SDT/jenkins-artifacts/jenkins-test-code-format/CMSSW_11_3_X_2021-03-08-2300/59/code-format.patch (61 changes)
LLVM 11: https://cmssdt.cern.ch/SDT/jenkins-artifacts/jenkins-test-code-format/CMSSW_11_3_X_2021-03-08-1100/63/code-format.patch (112 changes)

@smuzaffar
Copy link
Contributor

it could be if google code style has updated few defaults. Let me see the differences

@makortel
Copy link
Contributor

Would it make sense to have an automated reformatting campaign? (after double-checking the style definition itself)

@smuzaffar
Copy link
Contributor

here is the code format changes which is reasonable. I would not recommend to change the google default. After all it is not a big bang changes. Only 50 new changes.

--- l10.txt     2021-03-17 15:02:22.291374654 +0100
+++ l11.txt     2021-03-17 15:03:58.029633511 +0100
@@ -1,19 +1,21 @@
 >> Local Products Rules ..... started
 >> Local Products Rules ..... done
-clang-format   -dump-config /build/muz/CMSSW_11_3_X_2021-03-07-0000/src/.clang-format
+clang-format   -dump-config /build/muz/CMSSW_11_3_X_2021-03-16-2300/src/.clang-format
 ---
 Language:        Cpp
 AccessModifierOffset: -2
 AlignAfterOpenBracket: Align
 AlignConsecutiveMacros: false
 AlignConsecutiveAssignments: false
+AlignConsecutiveBitFields: false
 AlignConsecutiveDeclarations: false
 AlignEscapedNewlines: Left
-AlignOperands:   true
+AlignOperands:   Align
 AlignTrailingComments: true
 AllowAllArgumentsOnNextLine: true
 AllowAllConstructorInitializersOnNextLine: true
 AllowAllParametersOfDeclarationOnNextLine: true
+AllowShortEnumsOnASingleLine: true
 AllowShortBlocksOnASingleLine: Never
 AllowShortCaseLabelsOnASingleLine: false
 AllowShortFunctionsOnASingleLine: All
@@ -29,7 +31,7 @@
 BraceWrapping:
   AfterCaseLabel:  false
   AfterClass:      false
-  AfterControlStatement: false
+  AfterControlStatement: Never
   AfterEnum:       false
   AfterFunction:   false
   AfterNamespace:  false
@@ -39,6 +41,8 @@
   AfterExternBlock: false
   BeforeCatch:     false
   BeforeElse:      false
+  BeforeLambdaBody: false
+  BeforeWhile:     false
   IndentBraces:    false
   SplitEmptyFunction: true
   SplitEmptyRecord: true
@@ -85,10 +89,13 @@
 IncludeIsMainRegex: '([-_](test|unittest))?$'
 IncludeIsMainSourceRegex: ''
 IndentCaseLabels: true
+IndentCaseBlocks: false
 IndentGotoLabels: true
 IndentPPDirectives: None
+IndentExternBlock: AfterExternBlock
 IndentWidth:     2
 IndentWrappedFunctionNames: false
+InsertTrailingCommas: None
 JavaScriptQuotes: Leave
 JavaScriptWrapImports: true
 KeepEmptyLinesAtTheStartOfBlocks: false
@@ -98,6 +105,7 @@
 NamespaceIndentation: All
 ObjCBinPackProtocolList: Never
 ObjCBlockIndentWidth: 2
+ObjCBreakBeforeNestedBlockParam: true
 ObjCSpaceAfterProperty: false
 ObjCSpaceBeforeProtocolList: true
 PenaltyBreakAssignment: 2
@@ -135,6 +143,8 @@
       - PARSE_TEXT_PROTO
       - ParseTextOrDie
       - ParseTextProtoOrDie
+      - ParseTestProto
+      - ParsePartialTestProto
     CanonicalDelimiter: ''
     BasedOnStyle:    google
 ReflowComments:  false
@@ -166,5 +176,9 @@
 TabWidth:        8
 UseCRLF:         false
 UseTab:          Never
+WhitespaceSensitiveMacros:
+  - STRINGIZE
+  - PP_STRINGIZE
+  - BOOST_PP_STRINGIZE

@smuzaffar
Copy link
Contributor

Would it make sense to have an automated reformatting campaign? (after double-checking the style definition itself)

Yes, it is about time to do one. Although for default clang-tidy and format , we have 120 packages (194 files) which needs format/tidy updates but we can enable cms-hanlde check too (needs 6.5K changes)

@makortel
Copy link
Contributor

I would do the cms-handle separately as it is conceptually different from "changes from LLVM upgrade" (and try it out on a few packages first to get feedback like we did when we started to use clang-tidy).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

llvm/clang 11.1

7 participants