-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[GCC13] Suppress array-bound error #45179 from PixelSeeding #45340
Conversation
cms-bot internal usage |
please test with #45335 for el9_amd64_gcc13 |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45340/40753
|
A new Pull Request was created by @smuzaffar for master. It involves the following packages:
@jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: UnitTests RelVals Unit TestsI found 4 errors in the following unit tests: ---> test testTauEmbeddingWorkflow2016preVFP had ERRORS ---> test testTauEmbeddingWorkflow2017 had ERRORS ---> test testTauEmbeddingWorkflow2016postVFP had ERRORS and more ... RelVals
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0b315/40145/summary.html
Comparison SummarySummary:
|
What's up with the failed relvals and unit tests? |
@mandrenguyen , failed workflows and relvals are for el8/gcc13 , so please ignore those. For productiuon arch , PR results #45340 (comment) look good |
assign heterogeneous |
+1 |
@cms-sw/heterogeneous-l2 gentle ping |
Rather than changing the flags in the |
-heterogeneous |
@fwyzard , I tried disabling the warning using |
@fwyzard , these warnings ( and errors for GCC 13) only happen when we link serial backend [a]. There is one warning for each of https://github.com/cms-sw/cmssw/blob/master/RecoTracker/PixelSeeding/plugins/alpaka/CAHitNtupletGeneratorKernels.dev.cc#L559-L561. Warning array subscript corresponds to [a]
|
Thanks for the detailed pointers, I'll have a look. |
I started looking into it, and quickly ended up on the GCC Bugzilla:
:-/ |
@smuzaffar is there a way to put these flags into |
@fwyzard , SCRAM does not export cxxflags from BuildFiles. Only way to export flags is to define a toolfile e.g. Adding such a dependency in
|
@fwyzard , should we go with the solution I have proposed in #45340 (comment) ? |
Sorry for the delay, I'm slowly catching up after last week's hackathon. I do like the solution you proposed in #45340 (comment) - but I don't want to push for the extra complexity. So I'm also fine with adding the flags explicitly in |
|
OK, then let's take the fix that requires minimal changes in SCRAM and in the externals (i.e. this one) and replace it with something better if you make changes to SCRAM or if we need to use the same flags somewhere else ? |
byt he way, we already have ofast-flag toolfile used by various cmssw packages . To avoid exporting such a dependency to dependent packages one can add NO_RECURSIVE_EXPORT flag in toolfile |
Should we rerun the gcc13 tests ? |
please test for el9_amd64_gcc13 |
Ah, I see. The code that requires the extra flag is in a header file, so I think in this case we should propagate the flag to the packages that depend on it. |
Does cms-sw/cmsdist#9301 look OK ? |
thanks @fwyzard , yes it looks good. |
I've opened #45455 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0b315/40388/summary.html Comparison SummarySummary:
|
closing this in favor of #45455 |
Fixes #45179
This PR suppresses the array-bound error for RecoTracker/PixelSeeding . It should fix the build errors we get in GCC 13 IBs