flatten localToGlobal in all cases and also for opt builds
We were flattening in one case (perigeeSurface) but in my understanding only for debug
builds (special trick from @ssnyder compiling the code with opt even in debug build and using attribute flatten
).
The issue is "opt" does not "flatten" (actually the aim of the flatten is to inline the Eigen code)
either with gcc11...
Before this MR in opt
After the MR in opt
ping @ssnyder so as to take a look
For completeness I noticed this here first so have an example handy
- clang 14 https://godbolt.org/z/YYh3adKTx
- gcc 11 https://godbolt.org/z/TYWTbn5Ye
- gcc 11 (flatten) https://godbolt.org/z/597Pnjfc9
- gcc 8 https://godbolt.org/z/b8xMavcEG
So this "inlining" is something that clang does (also previous versions) but gcc stopped doing...
"This" -> is inline certain 5x5 matrix operations it seems .
Merge request reports
Activity
This merge request affects 1 package:
- Tracking/TrkDetDescr/TrkSurfaces
This merge request affects 6 files:
- Tracking/TrkDetDescr/TrkSurfaces/src/ConeSurface.cxx
- Tracking/TrkDetDescr/TrkSurfaces/src/CylinderSurface.cxx
- Tracking/TrkDetDescr/TrkSurfaces/src/DiscSurface.cxx
- Tracking/TrkDetDescr/TrkSurfaces/src/PerigeeSurface.cxx
- Tracking/TrkDetDescr/TrkSurfaces/src/PlaneSurface.cxx
- Tracking/TrkDetDescr/TrkSurfaces/src/StraightLineSurface.cxx
Adding @amorley as watcher
added Tracking master review-pending-level-1 labels
CI Result SUCCESS (hash f6ae6b3e) Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 52380] added review-pending-expert label and removed review-pending-level-1 label
Changes and CI look fine from L1 point of view. Waiting for expert review from @ssnyder before approving.
Kira (L1)
Thanks Christos.
Annoyingly, clang also defines
__GNUC__
(to 4 IIRC). So one should verify that clang is happy with the gnu: attribute or change to__GNUC__
>= 11 or something.Edited by Scott Snyderhttps://releases.llvm.org/14.0.0/tools/clang/docs/AttributeReference.html#flatten
above example
- clang 14 https://godbolt.org/z/YYh3adKTx
- clang14 with
[[gnu::flatten]]
https://godbolt.org/z/b8x5j5nG9
Actually should be happy since 3.6 or so with this C++11 syntax
I tend to like
[[gnu:: ]]
than__attribute()__
tbh ... and clang doc is very good to give you the possibilities .Edited by Christos Anastopoulosadded review-approved label
removed review-pending-expert label
mentioned in commit 75b00db1
added sweep:ignore label
mentioned in merge request !53502 (merged)
mentioned in merge request !53560 (merged)