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

byteorder: use gcc intrinsics for byteswap #15012

Merged
merged 6 commits into from May 12, 2017

Conversation

Projects
None yet
4 participants
@tchaikov
Copy link
Contributor

tchaikov commented May 9, 2017

and detect endianness using cmake

template<typename T>
typename std::enable_if<sizeof(T) == sizeof(uint16_t), T>::type
inline swab(T val) {
return __builtin_bswap16(val);

This comment has been minimized.

Copy link
@tchaikov

tchaikov May 9, 2017

Author Contributor

this builtin will emit platform specific instructions. on i686/amd64, gcc will emit BSWAP. so i think this change would improve the performance a little bit.

@tchaikov tchaikov force-pushed the tchaikov:wip-byteswap branch from 95f819f to 2f81013 May 9, 2017

@tchaikov tchaikov added the rbd label May 9, 2017

@tchaikov tchaikov requested a review from dillaman May 9, 2017

@@ -26,7 +26,7 @@ void decode_big_endian_string(std::string &str, bufferlist::iterator &it) {
#if defined(CEPH_LITTLE_ENDIAN)
uint32_t length;
::decode(length, it);
length = swab32(length);

This comment has been minimized.

Copy link
@tchaikov

tchaikov May 9, 2017

Author Contributor

@dillaman rbd_replay/ActionTypes.cc and rbd-nbd.cc are updated to accommodate the changes in byteswap.h, could you help take a look?

This comment has been minimized.

Copy link
@dillaman

dillaman May 9, 2017

Contributor

lgtm

@@ -824,6 +824,12 @@ if (NOT WITH_SYSTEM_ROCKSDB)

endif(NOT WITH_SYSTEM_ROCKSDB)

include(TestBigEndian)

This comment has been minimized.

Copy link
@tchaikov

tchaikov May 9, 2017

Author Contributor

@cbodley could you take a look at the cmake changes? acconfig.h will be included by byteorder.h in 199a262 after this change. and byteorder.h is where CEPH_BIG_ENDIAN and CEPH_LITTLE_ENDIAN are defined. so the this change should be backward compatible.

This comment has been minimized.

Copy link
@cbodley

cbodley May 9, 2017

Contributor

thanks, looks good 👍

#define init_le16(x) { (__u16)mswab16(x) }
#define init_le64(x) { (__u64)mswab(x) }
#define init_le32(x) { (__u32)mswab(x) }
#define init_le16(x) { (__u16)mswab(x) }

This comment has been minimized.

Copy link
@liewegas

liewegas May 9, 2017

Member

seems like this needs the template <> parameter since we don't know that x has the right type

@tchaikov tchaikov force-pushed the tchaikov:wip-byteswap branch from 2f81013 to b4d22d7 May 9, 2017

@tchaikov

This comment has been minimized.

Copy link
Contributor Author

tchaikov commented May 9, 2017

@liewegas fixed and repushed.

@tchaikov tchaikov force-pushed the tchaikov:wip-byteswap branch 4 times, most recently from ac6ed14 to 184daaf May 9, 2017

init_le32(0), // fl_cas_hash
init_le32(0), // fl_object_stripe_unit
init_le32(-1), // fl_unused
init_le32(-1), // fl_pg_pool

This comment has been minimized.

Copy link
@tchaikov

tchaikov added some commits May 9, 2017

cmake: use cmake module to detect endianness
Signed-off-by: Kefu Chai <kchai@redhat.com>
byteorder: use gcc intrinsics for byteswap
* use gcc intrinsics for byteswap
* use template to wrap them.
* add the modeline for emacs/vim
* update the caller of the mswab/swab accordingly

Signed-off-by: Kefu Chai <kchai@redhat.com>
byteorder: remove the cruft to detect endianness
Signed-off-by: Kefu Chai <kchai@redhat.com>
common/sctp_crc32: include acconfig.h for the detecting endianness
so byteorder.h can be a pure c++ header.

Signed-off-by: Kefu Chai <kchai@redhat.com>
byteorder: refactor ceph_le{16,32,64} using template
to improve the readablity and easier for debugging.

Signed-off-by: Kefu Chai <kchai@redhat.com>
radosstriper: do not use GCC extension to initialize struct members
it's obsolete since GCC 2.5 and is not portable. see
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

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

@tchaikov tchaikov force-pushed the tchaikov:wip-byteswap branch from 184daaf to 5e3f837 May 10, 2017

@tchaikov

This comment has been minimized.

Copy link
Contributor Author

tchaikov commented May 10, 2017

changelog

  • rebased against master

@tchaikov tchaikov requested a review from liewegas May 10, 2017

@liewegas liewegas added the needs-qa label May 10, 2017

@tchaikov tchaikov merged commit 6fbcac4 into ceph:master May 12, 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-byteswap branch May 12, 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.