Skip to content

Commit

Permalink
Improve performance of safe absolute value
Browse files Browse the repository at this point in the history
Creates a fast runtime path for checked and clamped absolute value.

Change-Id: Ibabc3cd8d4eda0a44703d3197d03bba7c746ba11
Reviewed-on: https://chromium-review.googlesource.com/573529
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Justin Schuh <jschuh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487318}
  • Loading branch information
jschuh authored and Commit Bot committed Jul 18, 2017
1 parent e5a9e8c commit a1639d9
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 12 deletions.
6 changes: 1 addition & 5 deletions base/numerics/checked_math.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,7 @@ class CheckedNumeric {
}

constexpr CheckedNumeric Abs() const {
return CheckedNumeric<T>(
AbsWrapper(state_.value()),
IsValid() &&
(!std::is_signed<T>::value || std::is_floating_point<T>::value ||
AbsWrapper(state_.value()) != std::numeric_limits<T>::lowest()));
return !IsValueNegative(state_.value()) ? *this : -*this;
}

template <typename U>
Expand Down
10 changes: 3 additions & 7 deletions base/numerics/clamped_math.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,9 @@ class ClampedNumeric {
}

constexpr ClampedNumeric Abs() const {
return ClampedNumeric<T>(
// The negation of two's complement int min is int min, so that's the
// only overflow case we have to check for.
(!std::is_signed<T>::value || std::is_floating_point<T>::value ||
AbsWrapper(value_) != std::numeric_limits<T>::lowest())
? AbsWrapper(value_)
: std::numeric_limits<T>::max());
// The negation of two's complement int min is int min, so that's the
// only overflow case where we will saturate.
return ClampedNumeric<T>(SaturatedAbsWrapper(value_));
}

template <typename U>
Expand Down
24 changes: 24 additions & 0 deletions base/numerics/clamped_math_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,30 @@
namespace base {
namespace internal {

template <typename T,
typename std::enable_if<std::is_integral<T>::value>::type* = nullptr>
constexpr T SaturatedAbsWrapper(T value) {
// The calculation below is a static identity for unsigned types, but for
// signed integer types it provides a non-branching, saturated absolute value.
// This works because SafeUnsignedAbs() returns an unsigned type, which can
// represent the absolute value of all negative numbers of an equal-width
// integer type. The call to IsValueNegative() then detects overflow in the
// special case of numeric_limits<T>::min(), by evaluating the bit pattern as
// a signed integer value. If it is the overflow case, we end up subtracting
// one from the unsigned result, thus saturating to numeric_limits<T>::max().
return MustTreatAsConstexpr(value) && !ClampedAbsFastOp<T>::is_supported
? static_cast<T>(SafeUnsignedAbs(value) -
IsValueNegative<T>(SafeUnsignedAbs(value)))
: ClampedAbsFastOp<T>::Do(value);
}

template <
typename T,
typename std::enable_if<std::is_floating_point<T>::value>::type* = nullptr>
constexpr T SaturatedAbsWrapper(T value) {
return value < 0 ? -value : value;
}

template <typename T, typename U, class Enable = void>
struct ClampedAddOp {};

Expand Down
16 changes: 16 additions & 0 deletions base/numerics/safe_math_clang_gcc_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,22 @@ struct ClampedMulFastOp {
}
};

template <typename T>
struct ClampedAbsFastOp {
// The generic code is pretty much optimal on arm, so we use it instead.
#if defined(__ARMEL__) || defined(__arch64__)
static const bool is_supported = false;
#else
static const bool is_supported = std::is_signed<T>::value;
#endif
static T Do(T value) {
// This variable assignment is necessary to prevent the compiler from
// emitting longer, ugly, branchy code.
T negated = ClampedSubFastOp<T, T>::template Do<T>(T(0), value);
return IsValueNegative(value) ? negated : value;
}
};

} // namespace internal
} // namespace base

Expand Down
9 changes: 9 additions & 0 deletions base/numerics/safe_math_shared_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ struct ClampedMulFastOp {
return CheckOnFailure::template HandleFailure<V>();
}
};

template <typename T>
struct ClampedAbsFastOp {
static const bool is_supported = false;
static constexpr T Do(T) {
// Force a compile failure if instantiated.
return CheckOnFailure::template HandleFailure<T>();
}
};
#endif // BASE_HAS_OPTIMIZED_SAFE_MATH
#undef BASE_HAS_OPTIMIZED_SAFE_MATH

Expand Down

0 comments on commit a1639d9

Please sign in to comment.