Skip to content

Commit

Permalink
Revert "[C++] Refactor uint128 (protocolbuffers#8416)"
Browse files Browse the repository at this point in the history 
This reverts commit b604419.
  • Loading branch information
@acozzette
acozzette committed Apr 2, 2021
1 parent 97cb3a8 commit 398d344
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 132 deletions.
15 changes: 5 additions & 10 deletions src/google/protobuf/stubs/int128.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,18 @@
#include <iomanip>
#include <ostream> // NOLINT(readability/streams)
#include <sstream>
#include <string>

#include <google/protobuf/stubs/logging.h>

#include <google/protobuf/port_def.inc>

namespace google {
namespace protobuf {
namespace int128_internal {

const uint128_pod kuint128max = {
static_cast<uint64>(PROTOBUF_LONGLONG(0xFFFFFFFFFFFFFFFF)),
static_cast<uint64>(PROTOBUF_LONGLONG(0xFFFFFFFFFFFFFFFF))
};

// Returns the 0-based position of the last set bit (i.e., most significant bit)
// in the given uint64. The argument may not be 0.
Expand Down Expand Up @@ -185,14 +188,6 @@ std::ostream& operator<<(std::ostream& o, const uint128& b) {
return o << rep;
}

void VerifyValidShift(std::string op, int amount) {
// Shifting more than 127 is UB in Abseil, just crash for now to verify
// callers don't depend on it returning 0.
GOOGLE_CHECK_LT(amount, 128) << "Error executing operator " << op
<< ": shifts of more than 127 are undefined";
}

} // namespace int128_internal
} // namespace protobuf
} // namespace google

Expand Down
127 changes: 69 additions & 58 deletions src/google/protobuf/stubs/int128.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,33 +33,38 @@
#include <google/protobuf/stubs/common.h>

#include <iosfwd>
#include <limits>
#include <string>

#include <google/protobuf/port_def.inc>

namespace google {
namespace protobuf {
namespace int128_internal {

// An unsigned 128-bit integer type. Thread-compatible.
class PROTOBUF_EXPORT uint128 {
public:
uint128() = default;
struct uint128_pod;

private:
// Use `MakeUint128` instead.
constexpr uint128(uint64 top, uint64 bottom);
// TODO(xiaofeng): Define GOOGLE_PROTOBUF_HAS_CONSTEXPR when constexpr is
// available.
#ifdef GOOGLE_PROTOBUF_HAS_CONSTEXPR
# define UINT128_CONSTEXPR constexpr
#else
# define UINT128_CONSTEXPR
#endif

// An unsigned 128-bit integer type. Thread-compatible.
class PROTOBUF_EXPORT uint128 {
public:
UINT128_CONSTEXPR uint128(); // Sets to 0, but don't trust on this behavior.
UINT128_CONSTEXPR uint128(uint64 top, uint64 bottom);
#ifndef SWIG
constexpr uint128(int bottom);
constexpr uint128(uint32 bottom); // Top 96 bits = 0
UINT128_CONSTEXPR uint128(int bottom);
UINT128_CONSTEXPR uint128(uint32 bottom); // Top 96 bits = 0
#endif
constexpr uint128(uint64 bottom); // hi_ = 0
UINT128_CONSTEXPR uint128(uint64 bottom); // hi_ = 0
UINT128_CONSTEXPR uint128(const uint128_pod &val);

// Trivial copy constructor, assignment operator and destructor.

void Initialize(uint64 top, uint64 bottom);

// Arithmetic operators.
uint128& operator+=(const uint128& b);
uint128& operator-=(const uint128& b);
Expand All @@ -77,10 +82,8 @@ class PROTOBUF_EXPORT uint128 {
uint128& operator++();
uint128& operator--();

friend constexpr uint64 Uint128Low64(const uint128& v);
friend constexpr uint64 Uint128High64(const uint128& v);

friend constexpr uint128 MakeUint128(uint64_t high, uint64_t low);
friend uint64 Uint128Low64(const uint128& v);
friend uint64 Uint128High64(const uint128& v);

// We add "std::" to avoid including all of port.h.
PROTOBUF_EXPORT friend std::ostream& operator<<(std::ostream& o,
Expand All @@ -97,25 +100,35 @@ class PROTOBUF_EXPORT uint128 {
uint64 hi_;

// Not implemented, just declared for catching automatic type conversions.
uint128(uint8) = delete;
uint128(uint16) = delete;
uint128(float v) = delete;
uint128(double v) = delete;
uint128(uint8);
uint128(uint16);
uint128(float v);
uint128(double v);
};

// This is a POD form of uint128 which can be used for static variables which
// need to be operated on as uint128.
struct uint128_pod {
// Note: The ordering of fields is different than 'class uint128' but the
// same as its 2-arg constructor. This enables more obvious initialization
// of static instances, which is the primary reason for this struct in the
// first place. This does not seem to defeat any optimizations wrt
// operations involving this struct.
uint64 hi;
uint64 lo;
};

PROTOBUF_EXPORT extern const uint128_pod kuint128max;

// allow uint128 to be logged
PROTOBUF_EXPORT extern std::ostream& operator<<(std::ostream& o,
const uint128& b);

// Methods to access low and high pieces of 128-bit value.
// Defined externally from uint128 to facilitate conversion
// to native 128-bit types when compilers support them.
inline constexpr uint64 Uint128Low64(const uint128& v) { return v.lo_; }
inline constexpr uint64 Uint128High64(const uint128& v) { return v.hi_; }

constexpr uint128 MakeUint128(uint64_t high, uint64_t low) {
return uint128(high, low);
}
inline uint64 Uint128Low64(const uint128& v) { return v.lo_; }
inline uint64 Uint128High64(const uint128& v) { return v.hi_; }

// TODO: perhaps it would be nice to have int128, a signed 128-bit type?

Expand All @@ -130,17 +143,27 @@ inline bool operator!=(const uint128& lhs, const uint128& rhs) {
return !(lhs == rhs);
}

inline constexpr uint128::uint128(uint64 top, uint64 bottom)
inline UINT128_CONSTEXPR uint128::uint128() : lo_(0), hi_(0) {}
inline UINT128_CONSTEXPR uint128::uint128(uint64 top, uint64 bottom)
: lo_(bottom), hi_(top) {}
inline constexpr uint128::uint128(uint64 bottom)
inline UINT128_CONSTEXPR uint128::uint128(const uint128_pod& v)
: lo_(v.lo), hi_(v.hi) {}
inline UINT128_CONSTEXPR uint128::uint128(uint64 bottom)
: lo_(bottom), hi_(0) {}
#ifndef SWIG
inline constexpr uint128::uint128(uint32 bottom)
inline UINT128_CONSTEXPR uint128::uint128(uint32 bottom)
: lo_(bottom), hi_(0) {}
inline constexpr uint128::uint128(int bottom)
inline UINT128_CONSTEXPR uint128::uint128(int bottom)
: lo_(bottom), hi_(static_cast<int64>((bottom < 0) ? -1 : 0)) {}
#endif

#undef UINT128_CONSTEXPR

inline void uint128::Initialize(uint64 top, uint64 bottom) {
hi_ = top;
lo_ = bottom;
}

// Comparison operators.

#define CMP128(op) \
Expand All @@ -164,9 +187,9 @@ inline uint128 operator-(const uint128& val) {
const uint64 lo_flip = ~Uint128Low64(val);
const uint64 lo_add = lo_flip + 1;
if (lo_add < lo_flip) {
return MakeUint128(hi_flip + 1, lo_add);
return uint128(hi_flip + 1, lo_add);
}
return MakeUint128(hi_flip, lo_add);
return uint128(hi_flip, lo_add);
}

inline bool operator!(const uint128& val) {
Expand All @@ -176,13 +199,13 @@ inline bool operator!(const uint128& val) {
// Logical operators.

inline uint128 operator~(const uint128& val) {
return MakeUint128(~Uint128High64(val), ~Uint128Low64(val));
return uint128(~Uint128High64(val), ~Uint128Low64(val));
}

#define LOGIC128(op) \
inline uint128 operator op(const uint128& lhs, const uint128& rhs) { \
return MakeUint128(Uint128High64(lhs) op Uint128High64(rhs), \
Uint128Low64(lhs) op Uint128Low64(rhs)); \
return uint128(Uint128High64(lhs) op Uint128High64(rhs), \
Uint128Low64(lhs) op Uint128Low64(rhs)); \
}

LOGIC128(|)
Expand All @@ -206,11 +229,7 @@ LOGICASSIGN128(^=)

// Shift operators.

void VerifyValidShift(std::string op, int amount);

inline uint128 operator<<(const uint128& val, int amount) {
VerifyValidShift("<<", amount);

// uint64 shifts of >= 64 are undefined, so we will need some special-casing.
if (amount < 64) {
if (amount == 0) {
Expand All @@ -219,14 +238,15 @@ inline uint128 operator<<(const uint128& val, int amount) {
uint64 new_hi = (Uint128High64(val) << amount) |
(Uint128Low64(val) >> (64 - amount));
uint64 new_lo = Uint128Low64(val) << amount;
return MakeUint128(new_hi, new_lo);
return uint128(new_hi, new_lo);
} else if (amount < 128) {
return uint128(Uint128Low64(val) << (amount - 64), 0);
} else {
return uint128(0, 0);
}
return MakeUint128(Uint128Low64(val) << (amount - 64), 0);
}

inline uint128 operator>>(const uint128& val, int amount) {
VerifyValidShift(">>", amount);

// uint64 shifts of >= 64 are undefined, so we will need some special-casing.
if (amount < 64) {
if (amount == 0) {
Expand All @@ -235,10 +255,12 @@ inline uint128 operator>>(const uint128& val, int amount) {
uint64 new_hi = Uint128High64(val) >> amount;
uint64 new_lo = (Uint128Low64(val) >> amount) |
(Uint128High64(val) << (64 - amount));
return MakeUint128(new_hi, new_lo);
return uint128(new_hi, new_lo);
} else if (amount < 128) {
return uint128(0, Uint128High64(val) >> (amount - 64));
} else {
return uint128(0, 0);
}

return MakeUint128(0, Uint128High64(val) >> (amount - 64));
}

inline uint128& uint128::operator<<=(int amount) {
Expand Down Expand Up @@ -357,17 +379,6 @@ inline uint128& uint128::operator--() {
return *this;
}

constexpr uint128 Uint128Max() {
return MakeUint128((std::numeric_limits<uint64>::max)(),
(std::numeric_limits<uint64>::max)());
}

} // namespace int128_internal

using google::protobuf::int128_internal::uint128;
using google::protobuf::int128_internal::Uint128Max;
using google::protobuf::int128_internal::MakeUint128;

} // namespace protobuf
} // namespace google

Expand Down
Loading

0 comments on commit 398d344

Please sign in to comment.