Skip to content

Commit

Permalink
Remove leveldb::port::kLittleEndian.
Browse files Browse the repository at this point in the history
Clang 10 includes the optimizations described in
https://bugs.llvm.org/show_bug.cgi?id=41761. This means that the
platform-independent implementations of {Decode,Encode}Fixed{32,64}()
compile to one instruction on the most recent Clang and GCC.

PiperOrigin-RevId: 306330166
  • Loading branch information
pwnall committed Apr 14, 2020
1 parent ba369dd commit 201f522
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 61 deletions.
3 changes: 0 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ option(LEVELDB_BUILD_TESTS "Build LevelDB's unit tests" ON)
option(LEVELDB_BUILD_BENCHMARKS "Build LevelDB's benchmarks" ON)
option(LEVELDB_INSTALL "Install LevelDB's header and library" ON)

include(TestBigEndian)
test_big_endian(LEVELDB_IS_BIG_ENDIAN)

include(CheckIncludeFile)
check_include_file("unistd.h" HAVE_UNISTD_H)

Expand Down
6 changes: 0 additions & 6 deletions port/port_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,4 @@
#cmakedefine01 HAVE_SNAPPY
#endif // !defined(HAVE_SNAPPY)

// Define to 1 if your processor stores words with the most significant byte
// first (like Motorola and SPARC, unlike Intel and VAX).
#if !defined(LEVELDB_IS_BIG_ENDIAN)
#cmakedefine01 LEVELDB_IS_BIG_ENDIAN
#endif // !defined(LEVELDB_IS_BIG_ENDIAN)

#endif // STORAGE_LEVELDB_PORT_PORT_CONFIG_H_
4 changes: 0 additions & 4 deletions port/port_example.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ namespace port {
// TODO(jorlow): Many of these belong more in the environment class rather than
// here. We should try moving them and see if it affects perf.

// The following boolean constant must be true on a little-endian machine
// and false otherwise.
static const bool kLittleEndian = true /* or some other expression */;

// ------------------ Threading -------------------

// A Mutex represents an exclusive lock.
Expand Down
2 changes: 0 additions & 2 deletions port/port_stdcxx.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@
namespace leveldb {
namespace port {

static const bool kLittleEndian = !LEVELDB_IS_BIG_ENDIAN;

class CondVar;

// Thinly wraps std::mutex.
Expand Down
50 changes: 4 additions & 46 deletions util/coding.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,13 @@ int VarintLength(uint64_t v);
char* EncodeVarint32(char* dst, uint32_t value);
char* EncodeVarint64(char* dst, uint64_t value);

// TODO(costan): Remove port::kLittleEndian and the fast paths based on
// std::memcpy when clang learns to optimize the generic code, as
// described in https://bugs.llvm.org/show_bug.cgi?id=41761
//
// The platform-independent code in DecodeFixed{32,64}() gets optimized to mov
// on x86 and ldr on ARM64, by both clang and gcc. However, only gcc optimizes
// the platform-independent code in EncodeFixed{32,64}() to mov / str.

// Lower-level versions of Put... that write directly into a character buffer
// REQUIRES: dst has enough space for the value being written

inline void EncodeFixed32(char* dst, uint32_t value) {
uint8_t* const buffer = reinterpret_cast<uint8_t*>(dst);

if (port::kLittleEndian) {
// Fast path for little-endian CPUs. All major compilers optimize this to a
// single mov (x86_64) / str (ARM) instruction.
std::memcpy(buffer, &value, sizeof(uint32_t));
return;
}

// Platform-independent code.
// Currently, only gcc optimizes this to a single mov / str instruction.
// Recent clang and gcc optimize this to a single mov / str instruction.
buffer[0] = static_cast<uint8_t>(value);
buffer[1] = static_cast<uint8_t>(value >> 8);
buffer[2] = static_cast<uint8_t>(value >> 16);
Expand All @@ -80,15 +64,7 @@ inline void EncodeFixed32(char* dst, uint32_t value) {
inline void EncodeFixed64(char* dst, uint64_t value) {
uint8_t* const buffer = reinterpret_cast<uint8_t*>(dst);

if (port::kLittleEndian) {
// Fast path for little-endian CPUs. All major compilers optimize this to a
// single mov (x86_64) / str (ARM) instruction.
std::memcpy(buffer, &value, sizeof(uint64_t));
return;
}

// Platform-independent code.
// Currently, only gcc optimizes this to a single mov / str instruction.
// Recent clang and gcc optimize this to a single mov / str instruction.
buffer[0] = static_cast<uint8_t>(value);
buffer[1] = static_cast<uint8_t>(value >> 8);
buffer[2] = static_cast<uint8_t>(value >> 16);
Expand All @@ -105,16 +81,7 @@ inline void EncodeFixed64(char* dst, uint64_t value) {
inline uint32_t DecodeFixed32(const char* ptr) {
const uint8_t* const buffer = reinterpret_cast<const uint8_t*>(ptr);

if (port::kLittleEndian) {
// Fast path for little-endian CPUs. All major compilers optimize this to a
// single mov (x86_64) / ldr (ARM) instruction.
uint32_t result;
std::memcpy(&result, buffer, sizeof(uint32_t));
return result;
}

// Platform-independent code.
// Clang and gcc optimize this to a single mov / ldr instruction.
// Recent clang and gcc optimize this to a single mov / ldr instruction.
return (static_cast<uint32_t>(buffer[0])) |
(static_cast<uint32_t>(buffer[1]) << 8) |
(static_cast<uint32_t>(buffer[2]) << 16) |
Expand All @@ -124,16 +91,7 @@ inline uint32_t DecodeFixed32(const char* ptr) {
inline uint64_t DecodeFixed64(const char* ptr) {
const uint8_t* const buffer = reinterpret_cast<const uint8_t*>(ptr);

if (port::kLittleEndian) {
// Fast path for little-endian CPUs. All major compilers optimize this to a
// single mov (x86_64) / ldr (ARM) instruction.
uint64_t result;
std::memcpy(&result, buffer, sizeof(uint64_t));
return result;
}

// Platform-independent code.
// Clang and gcc optimize this to a single mov / ldr instruction.
// Recent clang and gcc optimize this to a single mov / ldr instruction.
return (static_cast<uint64_t>(buffer[0])) |
(static_cast<uint64_t>(buffer[1]) << 8) |
(static_cast<uint64_t>(buffer[2]) << 16) |
Expand Down

0 comments on commit 201f522

Please sign in to comment.