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

Enable USE_SSEVECT only on x86_64 with SSE #6667

Merged
merged 1 commit into from Nov 28, 2014

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented Nov 27, 2014

Patch enables USE_SSEVECT (vector implementation based on SSE
intrinsics) only on a proper hardware, i.e, x86_64 with SSE.

For the rest of architectures (armv7hl, aarch64, power8, etc.) use
USE_EXTVECT.

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

Patch enables USE_SSEVECT (vector implementation based on SSE
intrinsics) only on a proper hardware, i.e, x86_64 with SSE.

For the rest of architectures (armv7hl, aarch64, power8, etc.) use
USE_EXTVECT.

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@davidlt
Copy link
Contributor Author

davidlt commented Nov 27, 2014

Compiler on x86_64 fine (apart compiler errors in DQM, same in IB). Yet to compile on aarch64.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlt for CMSSW_7_4_X.

Enable USE_SSEVECT only on x86_64 with SSE

It involves the following packages:

DataFormats/Math

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@davidlt
Copy link
Contributor Author

davidlt commented Nov 28, 2014

This is being tested by Jenkins on x86_64 (currently running analyzer). I merged this into my previous aarch64 build and compiling.

@davidlt
Copy link
Contributor Author

davidlt commented Nov 28, 2014

Adding @VinInn

@VinInn
Copy link
Contributor

VinInn commented Nov 28, 2014

+1

@cmsbuild
Copy link
Contributor

@@ -3,7 +3,11 @@

#if ( defined(IN_DICTBUILD) || defined(__REFLEX__) || defined(__CINT__) || defined(__MIC__)) || (__BIGGEST_ALIGNMENT__<16)
#elif defined(__GNUC__) || defined(__clang__)
#define USE_SSEVECT
# if defined(__x86_64__) && defined(__SSE__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is defined(__SSE__) alone not enough, is it an extra sanity check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, extra sanity check.

@davidlt
Copy link
Contributor Author

davidlt commented Nov 28, 2014

Seems to compile on aarch64. Will retest once approved and in IB.

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2014

I meant to wait for comparisons to show up in jenkins before signing .. technically this should have compiled to exactly the same code and there should be no diffs at all

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2014

+1

for #6667 f758ae3
jenkins showed no diffs, as expected

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes (tests are also fine). This pull request will be automatically merged.

cmsbuild added a commit that referenced this pull request Nov 28, 2014
Enable USE_SSEVECT only on x86_64 with SSE
@cmsbuild cmsbuild merged commit ae5ba32 into cms-sw:CMSSW_7_4_X Nov 28, 2014
@slava77 slava77 mentioned this pull request Dec 2, 2014
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.

None yet

4 participants