Add EventSetupRecord::get checker #10
Conversation
gartung
changed the title
|
A new Pull Request was created by @gartung (Patrick Gartung) for branch cms/release/10.x/92d5c1b. @cmsbuild, @smuzaffar, @mrodozov can you please review it and eventually sign? Thanks. |
|
plase test |
|
we are currently getting 11.1 in |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
5fd0a36
to
05f2317
Compare
|
Pull request #10 was updated. |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
05f2317
to
3894bd9
Compare
|
Pull request #10 was updated. |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
3894bd9
to
25d8d1a
Compare
|
Pull request #10 was updated. |
5 similar comments
|
Pull request #10 was updated. |
|
Pull request #10 was updated. |
|
Pull request #10 was updated. |
|
Pull request #10 was updated. |
|
Pull request #10 was updated. |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
9ddf816
to
2748142
Compare
|
Pull request #10 was updated. |
gartung
changed the title
|
Please test |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
2748142
to
7802bc9
Compare
|
abort |
gartung
force-pushed
the
gartung-esrgetcheck
branch
from
0ebe011
to
18f656c
Compare
|
Pull request #10 was updated. |
|
please test |
|
-1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b70eeb/13472/summary.html External BuildI found compilation error when building: /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/llvm/10.0.0-f4e43475a52f131bbd8e74e1681e48dd/llvm-10.0.0-4b3acf7d70558fefca729efe381e3900da4591dd/clang/tools/include-what-you-use/iwyu.cc:3634:50: required from here /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/llvm/10.0.0-f4e43475a52f131bbd8e74e1681e48dd/llvm-10.0.0-4b3acf7d70558fefca729efe381e3900da4591dd/clang/tools/include-what-you-use/iwyu.cc:2042:5: warning: enumeration value 'CK_LValueToRValueBitCast' not handled in switch [-Wswitch] /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/llvm/10.0.0-f4e43475a52f131bbd8e74e1681e48dd/llvm-10.0.0-4b3acf7d70558fefca729efe381e3900da4591dd/clang/tools/include-what-you-use/iwyu.cc:2042:5: warning: enumeration value 'CK_FixedPointToIntegral' not handled in switch [-Wswitch] /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/BUILD/slc7_amd64_gcc900/external/llvm/10.0.0-f4e43475a52f131bbd8e74e1681e48dd/llvm-10.0.0-4b3acf7d70558fefca729efe381e3900da4591dd/clang/tools/include-what-you-use/iwyu.cc:2042:5: warning: enumeration value 'CK_IntegralToFixedPoint' not handled in switch [-Wswitch] ninja: build stopped: subcommand failed. error: Bad exit status from /build/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.66hS09 (%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.66hS09 (%build) |
|
@mrodozov @smuzaffar The build failure is unrelated to this pull request. Do I need to merge in another branch? |
|
@gartung , cms/release/11.x/1fdec59 is not yet in IB. We are in process of updating to llvm 11.1. hopefully in a day or so we will integrate it and then you can test this or you can test it with cms-sw/cmsdist#6691 (which also include newer iwyu) |
|
please test with cms-sw/cmsdist#6691 |
|
Some unit tests are failing but I assume that is because of missing updates to other packages that depend on llvm/clang that Mircho made on the CMSDIST PR. |
|
-1 Failed Tests: UnitTests Unit TestsI found errors in the following unit tests: ---> test testLLVMLite had ERRORS ---> test test-clang-tidy had ERRORS Comparison SummarySummary:
|
|
+externals |
cmsbuild
added
externals-approved
fully-signed
and removed
externals-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next cms/release/11.x/1fdec59 IBs (but tests are reportedly failing). 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) |
This patch implemented TTI.IntImmCost() properly. Each BPF insn has 32bit immediate space, so for any immediate which can be represented as 32bit signed int, the cost is technically free. If an int cannot be presented as a 32bit signed int, a ld_imm64 instruction is needed and a TCC_Basic is returned. This change is motivated when we observed that several bpf selftests failed with latest llvm trunk, e.g., #10/16 strobemeta.o:FAIL #10/17 strobemeta_nounroll1.o:FAIL #10/18 strobemeta_nounroll2.o:FAIL #10/19 strobemeta_subprogs.o:FAIL llvm#96 snprintf_btf:FAIL The reason of the failure is due to that SpeculateAroundPHIsPass did aggressive transformation which alters control flow for which currently verifer cannot handle well. In llvm12, SpeculateAroundPHIsPass is not called. SpeculateAroundPHIsPass relied on TTI.getIntImmCost() and TTI.getIntImmCostInst() for profitability analysis. This patch implemented TTI.getIntImmCost() properly for BPF backend which also prevented transformation which caused the above test failures. Differential Revision: https://reviews.llvm.org/D96448 (cherry picked from commit a260ae7)
This clang-tidy checker will flag all uses up EventSetupRecord::get with a deprecated warning, Links to wiki pages with directions on replacement with EventSetup::getHandle(ESGetToken<>) are also printed.
Resolves cms-sw/framework-team#69