Skip to content

F
folly
Commit e741d8ef authored by Roger Kim's avatar Roger Kim Committed by Facebook GitHub Bot
Browse files
Options

Replace if-statements with `if constexpr` statements 

Summary:
We have found that BitVectorCoding.h incorrectly outputs
-Wdivision-by-zero errors when another file that includes it fails to compile.
In the absence of other compilation errors, the -Wdivision-by-zero error does
not appear (since the compiler correctly can tell that "division by zero" is
not an issue in the code).

In order to get rid of these erroneous error logs, we decided to replace the
if-statements that surround the divisions (and check that the divisor is not
zero) with `if constexpr` statements since those will stop the compiler from
mistakenly thinking that division by zero is possible.

Reviewed By: ot, philippv, luciang

Differential Revision: D26961916

fbshipit-source-id: f65ef6d7782c94e58d6d28b9f8b053f205e3b7c1
parent 28799ed1
Hide whitespace changes
Inline Side-by-side
Showing with 63 additions and 53 deletions
folly/experimental/BitVectorCoding.h
View file @ e741d8ef
......@@ -128,7 +128,7 @@ struct BitVectorEncoder {
folly::Bits<folly::Unaligned<uint64_t>>::set(
reinterpret_cast<folly::Unaligned<uint64_t>*>(block), inner);
if (skipQuantum != 0) {
if constexpr (skipQuantum != 0) {
size_t nextSkipPointerSize = value / skipQuantum;
while (skipPointersSize_ < nextSkipPointerSize) {
auto pos = skipPointersSize_++;
......@@ -137,7 +137,7 @@ struct BitVectorEncoder {
}
}
if (forwardQuantum != 0) {
if constexpr (forwardQuantum != 0) {
if (size_ != 0 && (size_ % forwardQuantum == 0)) {
const auto pos = size_ / forwardQuantum - 1;
folly::storeUnaligned<SkipValueType>(
......@@ -183,11 +183,11 @@ struct BitVectorEncoder<Value, SkipValue, kSkipQuantum, kForwardQuantum>::
size_t bitVectorSizeInBytes = (upperBound / 8) + 1;
layout.bits = bitVectorSizeInBytes;
if (skipQuantum != 0) {
if constexpr (skipQuantum != 0) {
size_t numSkipPointers = upperBound / skipQuantum;
layout.skipPointers = numSkipPointers * sizeof(SkipValueType);
}
if (forwardQuantum != 0) {
if constexpr (forwardQuantum != 0) {
size_t numForwardPointers = size / forwardQuantum;
layout.forwardPointers = numForwardPointers * sizeof(SkipValueType);
}
......@@ -305,13 +305,15 @@ class BitVectorReader : detail::ForwardPointers<Encoder::forwardQuantum>,
position_ += n;
// Use forward pointer.
if (Encoder::forwardQuantum > 0 && n > Encoder::forwardQuantum) {
const size_t steps = position_ / Encoder::forwardQuantum;
const size_t dest = folly::loadUnaligned<SkipValueType>(
this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest);
n = position_ + 1 - steps * Encoder::forwardQuantum;
if constexpr (Encoder::forwardQuantum > 0) {
if (n > Encoder::forwardQuantum) {
const size_t steps = position_ / Encoder::forwardQuantum;
const size_t dest = folly::loadUnaligned<SkipValueType>(
this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest);
n = position_ + 1 - steps * Encoder::forwardQuantum;
}
}
size_t cnt;
......@@ -351,13 +353,15 @@ class BitVectorReader : detail::ForwardPointers<Encoder::forwardQuantum>,
return true;
}
if (Encoder::skipQuantum > 0 && v - value_ > Encoder::skipQuantum) {
size_t q = v / Encoder::skipQuantum;
auto skipPointer = folly::loadUnaligned<SkipValueType>(
this->skipPointers_ + (q - 1) * sizeof(SkipValueType));
position_ = static_cast<SizeType>(skipPointer) - 1;
if constexpr (Encoder::skipQuantum > 0) {
if (v - value_ > Encoder::skipQuantum) {
size_t q = v / Encoder::skipQuantum;
auto skipPointer = folly::loadUnaligned<SkipValueType>(
this->skipPointers_ + (q - 1) * sizeof(SkipValueType));
position_ = static_cast<SizeType>(skipPointer) - 1;
reposition(q * Encoder::skipQuantum);
reposition(q * Encoder::skipQuantum);
}
}
// Find the value.
......
folly/experimental/EliasFanoCoding.h
View file @ e741d8ef
......@@ -411,13 +411,15 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
position_ += n; // n 1-bits will be read.
// Use forward pointer.
if (Encoder::forwardQuantum > 0 && UNLIKELY(n > Encoder::forwardQuantum)) {
const size_t steps = position_ / Encoder::forwardQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest + steps * Encoder::forwardQuantum);
n = position_ + 1 - steps * Encoder::forwardQuantum; // n is > 0.
if constexpr (Encoder::forwardQuantum > 0) {
if (UNLIKELY(n > Encoder::forwardQuantum)) {
const size_t steps = position_ / Encoder::forwardQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest + steps * Encoder::forwardQuantum);
n = position_ + 1 - steps * Encoder::forwardQuantum; // n is > 0.
}
}
size_t cnt;
......@@ -442,20 +444,21 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
DCHECK_GE(v, value_);
// Use skip pointer.
if (Encoder::skipQuantum > 0 &&
UNLIKELY(v >= value_ + Encoder::skipQuantum)) {
const size_t steps = v / Encoder::skipQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
if constexpr (Encoder::skipQuantum > 0) {
if (UNLIKELY(v >= value_ + Encoder::skipQuantum)) {
const size_t steps = v / Encoder::skipQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest + Encoder::skipQuantum * steps);
position_ = dest - 1;
reposition(dest + Encoder::skipQuantum * steps);
position_ = dest - 1;
// Correct value_ will be set during the next() call at the end.
// Correct value_ will be set during the next() call at the end.
// NOTE: Corresponding block of lower bits sequence may be
// prefetched here (via __builtin_prefetch), but experiments
// didn't show any significant improvements.
// NOTE: Corresponding block of lower bits sequence may be
// prefetched here (via __builtin_prefetch), but experiments
// didn't show any significant improvements.
}
}
// Skip by blocks.
......@@ -492,24 +495,27 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
FOLLY_ALWAYS_INLINE SizeType prepareSkipTo(ValueType v) const {
auto position = position_;
if (Encoder::skipQuantum > 0 && v >= value_ + Encoder::skipQuantum) {
auto outer = outer_;
const size_t steps = v / Encoder::skipQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
position = dest - 1;
outer = (dest + Encoder::skipQuantum * steps) / 8;
// Prefetch up to the beginning of where we linear search. After that,
// hardware prefetching will outperform our own. In addition, this
// simplifies calculating what to prefetch as we don't have to calculate
// the entire destination address. Two cache lines are prefetched because
// this results in fewer cycles used (based on practical results) than
// one. However, three cache lines does not have any additional effect.
const auto addr = start_ + outer;
__builtin_prefetch(addr);
__builtin_prefetch(addr + kCacheLineSize);
if constexpr (Encoder::skipQuantum > 0) {
if (v >= value_ + Encoder::skipQuantum) {
auto outer = outer_;
const size_t steps = v / Encoder::skipQuantum;
const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
position = dest - 1;
outer = (dest + Encoder::skipQuantum * steps) / 8;
// Prefetch up to the beginning of where we linear search. After that,
// hardware prefetching will outperform our own. In addition, this
// simplifies calculating what to prefetch as we don't have to calculate
// the entire destination address. Two cache lines are prefetched
// because this results in fewer cycles used (based on practical
// results) than one. However, three cache lines does not have any
// additional effect.
const auto addr = start_ + outer;
__builtin_prefetch(addr);
__builtin_prefetch(addr + kCacheLineSize);
}
}
return position;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment