Add cms_ prefix to float<BITS>x<ELEM_NUM>_t typedefs #16748
Conversation
We have recently moved to Eigen 3.3 with
- Many ARM NEON improvements, including support for ARMv8 (64-bit code),
VFPv4 (fused-multiply-accumulate instruction), and correct tuning of
the target number of vector registers.
Internally Eigen loads arm_neon.h on AArch64 target to use ARM NEON
intrinsics. The header defines extra system types:
float<BITS>x<ELEM_NUM>_t
Let's add cms_ prefix to our types to avoid conflicts with system
types, which lead to compilation errors.
NOTE: these types are not directly used within CMSSW.
Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
|
A new Pull Request was created by @davidlt for CMSSW_9_0_X. It involves the following packages: DataFormats/Math @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
-1 Tested at: 4eb4c2c You can see the results of the tests here: I found follow errors while testing this PR Failed tests: Build
I found an error when building: >> Compiling /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/Utilities/StaticAnalyzers/src/CmsException.cpp
>> Compiling /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/Utilities/StaticAnalyzers/src/edmChecker.cpp
>> Compiling /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/Utilities/StaticAnalyzers/src/dablooms.c
>> Compiling /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/Utilities/StaticAnalyzers/src/murmur.c
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/RecoTracker/TkHitPairs/src/InnerDeltaPhi.cc: In member function 'PixelRecoRange InnerDeltaPhi::phiRange(const Point2D&, float, float) const':
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/RecoTracker/TkHitPairs/src/InnerDeltaPhi.cc:233:3: error: 'float32x4_t' was not declared in this scope
float32x4_t num{dHitmag,dLayer,theROrigin * (dHitmag-dLayer),1.f};
^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-23-2300/src/RecoTracker/TkHitPairs/src/InnerDeltaPhi.cc:234:15: error: expected ';' before 'den'
float32x4_t den{2*theRCurvature,2*theRCurvature,dHitmag*dLayer,1.f};
^
|
|
Comparison not run due to Build errors (RelVals and Igprof tests were also skipped) |
cmsbuild
added
comparison-notrun
tests-rejected
and removed
comparison-pending
tests-started
labels
We avoid using float32x4_t name for typedef because it can be defined by the system. Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-notrun
tests-rejected
labels
|
Pull request #16748 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again. |
|
Fixed. I guess, I missed it while grepping for its usage in CMSSW. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
+1 Technical change to avoid name clash between CMS-declared types from ExtVec.h and system types. There should be no change in run-time behavior. The code changes are satisfactory, and Jenkins tests against baseline CMSSW_9_0_X_2016-11-23-2300 show no significant differences, as expected. |
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
|
+1 |
We have recently moved to Eigen 3.3 with
VFPv4 (fused-multiply-accumulate instruction), and correct tuning of
the target number of vector registers.
Internally Eigen loads
arm_neon.hon AArch64 target to use ARM NEONintrinsics. The header defines extra system types:
Let's add
cms_prefix to our types to avoid conflicts with systemtypes, which lead to compilation errors.
NOTE: these types are not directly used within CMSSW.
Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch