Merged
Conversation
Change-Id: Ic17f7e26da0318a308d79bbea8022f744a7e362f
llvm/llvm-project#60896 Change-Id: I914b654c53a2e134ce219ce634dc94ded9353a20
fixes #939 Change-Id: I01cefe96c5a1dbd6d6058b78579e737565d7f7a5
Change-Id: I06c8085b4ce0f59fd9ee4d24392a7a912c86639b
Change-Id: I1d575f50a333855402063eff45405e7a19c1bac9
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a few updates: a guard condition was added in the dashboard code to ensure proper loop execution, a clang analyzer exclusion was applied to avoid false positives in CPU counters, and the CI workflow was updated to use the newer scan-build-18 command.
- Updated dashboard loop condition to check for valid UPI links.
- Wrapped a section in cpucounters.cpp with a clang analyzer guard.
- Modified CI configuration to use scan-build-18.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/dashboard.cpp | Added an extra condition in the for loop to check NumUPILinksPerSocket. |
| src/cpucounters.cpp | Added preprocessor guards to exclude code during clang analysis. |
| .github/workflows/ci-clang-scan.yml | Updated the scan-build command to scan-build-18. |
Comment on lines
+806
to
808
| for (size_t s = 0; s < NumSockets && NumUPILinksPerSocket > 0; ++s) | ||
| { | ||
| const auto S = std::to_string(s); |
There was a problem hiding this comment.
The additional check '&& NumUPILinksPerSocket > 0' in the loop condition may cause the loop to be skipped entirely if NumUPILinksPerSocket is 0. To improve clarity and avoid potential unintended behavior, consider verifying NumUPILinksPerSocket before entering the loop.
Suggested change
| for (size_t s = 0; s < NumSockets && NumUPILinksPerSocket > 0; ++s) | |
| { | |
| const auto S = std::to_string(s); | |
| if (NumUPILinksPerSocket > 0) | |
| { | |
| for (size_t s = 0; s < NumSockets; ++s) | |
| { | |
| const auto S = std::to_string(s); |
opcm
approved these changes
Apr 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.