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

cmake: should not compile crc32c_ppc.c on intel arch. #14423

Merged
merged 1 commit into from Apr 20, 2017

Conversation

Projects
None yet
4 participants
@tchaikov
Copy link
Contributor

tchaikov commented Apr 10, 2017

and should not compile crc32c_intel_fast.c on ARM.

Signed-off-by: Kefu Chai kchai@redhat.com

cmake: should not compile crc32c_ppc.c on intel arch.
and should not compile crc32c_intel_fast.c on ARM.

Signed-off-by: Kefu Chai <kchai@redhat.com>

@tchaikov tchaikov added the cleanup label Apr 10, 2017

@tchaikov tchaikov requested a review from cbodley Apr 10, 2017

@tchaikov

This comment has been minimized.

Copy link
Contributor Author

tchaikov commented Apr 14, 2017

@alimaredia and @cbodley could you take a look at this change at your convenience?

it silences the following warning:

/var/ceph/ceph/src/common/crc32c_ppc.c:21:21: warning: ‘crc32_align’ defined but not used [-Wunused-function]
 static unsigned int crc32_align(unsigned int crc, unsigned char const *p,
                     ^~~~~~~~~~~

@tchaikov tchaikov added the build/ops label Apr 14, 2017

@tchaikov

This comment has been minimized.

Copy link
Contributor Author

tchaikov commented Apr 14, 2017

retest this please

@alimaredia

This comment has been minimized.

Copy link
Contributor

alimaredia commented Apr 14, 2017

@tchaikov do we test builds on machines that trigger those options? The PR lgtm though

@tchaikov

This comment has been minimized.

Copy link
Contributor Author

tchaikov commented Apr 14, 2017

only HAVE_INTEL is triggered by our CI builds. and HAVE_ARM is triggered for out point release builds AFAICT. but i am not sure if shaman builds aarch64 now. gitbuilders used to do so. @dmick @alfredodeza do we have aarch64 slaves used by shaman?

@cbodley
Copy link
Contributor

cbodley left a comment

looks good, but haven't tested

@liewegas liewegas merged commit 0486c63 into ceph:master Apr 20, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@tchaikov tchaikov deleted the tchaikov:wip-cmake-crc32-cleanup branch Apr 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.