Skip to content
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

Fix static warnings on DT L1T #43544

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Conversation

jfernan2
Copy link
Contributor

PR description:

This PR fixes some static warnings in DT L1T emulator detected during integration of AMv2 in #43390

Out of the 8 warnings (see below), only the first 2 have been fixed by this PR.

Those related to core.uninitialized.Branch look false positives as far as I can see, since the vectors pointed out have been initialized previously at the beginning on purpouse to false in https://github.com/cms-sw/cmssw/blob/master/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc#L123-L127

The last one claims a nonFinite computation but this is exactly what is being tried to avoid.

I am not sure if the code experts like @makortel have any advice on how to proceed. Thanks in advance.

Static Warnings:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/GlobalCoordsObtainer.cc:222:3: warning: Value stored to 'x_msb' is never read [deadcode.DeadStores]
222 | x_msb = from_two_comp(x_msb, PHI_LUT_ADDR_WIDTH);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/GlobalCoordsObtainer.cc:225:3: warning: Value stored to 'tanpsi_msb' is never read [deadcode.DeadStores]
225 | tanpsi_msb = from_two_comp(tanpsi_msb, PHIB_LUT_ADDR_WIDTH);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:592:15: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
592 | if (useFitSL3[sl3])
| ^~~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:844:19: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
844 | if (useFitSL1[sl1])
| ^~~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:905:19: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
905 | if (useFitSL3[sl3])
| ^~~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:1011:9: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
1011 | if (!useFit[i])
| ^~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:1016:11: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
1016 | if (!useFit[j])

/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_0_X_2023-11-29-2300/src/L1Trigger/DTTriggerPhase2/src/MuonPathAnalyzerInChamber.cc:245:9: warning: cms.NonFiniteMath [cms.NonFiniteMath]
245 | if (isnan(jm_x))
| ^
1 warning generated.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43544/38130

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 11, 2023

A new Pull Request was created by @jfernan2 for master.

It involves the following packages:

  • L1Trigger/DTTriggerPhase2 (upgrade, l1)

@cmsbuild, @epalencia, @aloeliger, @srimanob can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @battibass, @JanFSchulte, @missirol this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@epalencia
Copy link
Contributor

Please test

@makortel
Copy link
Contributor

The last one claims a nonFinite computation but this is exactly what is being tried to avoid.

The static analyzer report https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e133a0/36189/llvm-analysis/ contains a suggestion for what to do for that warning

std::isnan / std::isinf does not work when fast-math is used. Please use edm::isNotFinite from 'FWCore/Utilities/interface/isFinite.h'

@makortel
Copy link
Contributor

About the "Branch condition evaluates to a garbage value" warnings, the links from the static analyzer report show the exact branch decisions the analyzer did to come to its conclusion. Here is the one about L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc:592
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e133a0/36189/llvm-analysis/report-091cc9.html#EndPath

I think the warning in this case is about the case where not SL1metaPrimitives.empty() and SL3metaPrimitives.empty(). It seems to me the static analyzer either assumes the SL1metaPrimitives/SL3metaPrimitives containers can change between calls, or doesn't somehow recognize the connection between empty(), size(), and begin()+end().

Note that variable length arrays (VLA, bool useFitSL1[SL1metaPrimitives.size()];) are not standard C++ (but an extension by GCC, and clang supports it as well). Zero-length arrays seem to be another extension by GCC. I think it would be much better to avoid at least the zero-length VLAs.

@jfernan2
Copy link
Contributor Author

Thanks @makortel
That suggestion scaped my attention, I have implemented it in another commit

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43544/38134

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

Pull request #43544 was updated. @aloeliger, @cmsbuild, @srimanob, @epalencia can you please check and sign again.

@epalencia
Copy link
Contributor

Please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-899d3e/36421/summary.html
COMMIT: 857d96a
CMSSW: CMSSW_14_0_X_2023-12-11-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 11-Dec-2023 22:51:24 CET-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named step2_DIGI_L1TrackTrigger_L1_DIGI2RAW_HLT.py
Exception Message:
 unknown python problem occurred.
RuntimeError: An exception of category 'FileInPathError' occurred.
Exception Message:
edm::FileInPath unable to find file L1Trigger/DTTriggerPhase2/data/fitterlut_sl1.dat anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/matrix-results/24900.0_CloseByPGun_CE_H_Coarse_Scint+2026D98


At:
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Types.py(881): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(381): insertContentsInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(516): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Modules.py(161): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1216): _insertManyInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1489): fillProcessDesc
  <string>(2): <module>

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Dec-2023 22:51:39 CET-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named step2_DIGI_L1TrackTrigger_L1_DIGI2RAW_HLT.py
Exception Message:
 unknown python problem occurred.
RuntimeError: An exception of category 'FileInPathError' occurred.
Exception Message:
edm::FileInPath unable to find file L1Trigger/DTTriggerPhase2/data/fitterlut_sl1.dat anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/matrix-results/24896.0_CloseByPGun_CE_E_Front_120um+2026D98


At:
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Types.py(881): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(381): insertContentsInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(516): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Modules.py(161): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1216): _insertManyInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1489): fillProcessDesc
  <string>(2): <module>

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Dec-2023 22:55:24 CET-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named step2_DIGI_L1TrackTrigger_L1_DIGI2RAW_HLT.py
Exception Message:
 unknown python problem occurred.
RuntimeError: An exception of category 'FileInPathError' occurred.
Exception Message:
edm::FileInPath unable to find file L1Trigger/DTTriggerPhase2/data/fitterlut_sl1.dat anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36421/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/poison:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src:/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/external/el8_amd64_gcc12/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/matrix-results/24834.0_TTbar_14TeV+2026D98


At:
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Types.py(881): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(381): insertContentsInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Mixins.py(516): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Modules.py(161): insertInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1216): _insertManyInto
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02815/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_0_X_2023-12-11-1100/src/FWCore/ParameterSet/python/Config.py(1489): fillProcessDesc
  <string>(2): <module>

----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

RelVals-INPUT

  • 24034.024034.0_TTbar_14TeV+2026D96/step2_TTbar_14TeV+2026D96.log
  • 20834.020834.0_TTbar_14TeV+2026D88/step2_TTbar_14TeV+2026D88.log
  • 20834.50120834.501_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU/step2_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU.log
Expand to see more relval errors ...

@epalencia
Copy link
Contributor

Please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-899d3e/36426/summary.html
COMMIT: 857d96a
CMSSW: CMSSW_14_0_X_2023-12-11-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43544/36426/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 25 lines from the logs
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3429858
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3429836
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@srimanob
Copy link
Contributor

+Upgrade

The warnings on Value stored to XXX is never read and cms.NonFiniteMath are gone in the static check.

@aloeliger
Copy link
Contributor

+l1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2024

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

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit e7ee1c8 into cms-sw:master Jan 8, 2024
1 check passed
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.

7 participants