diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 9d775297..4e94b5a2 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -34,6 +34,9 @@ #include #include +#define __STDC_FORMAT_MACROS +#include + #include "benchmark/benchmark.h" #include "benchmark_api_internal.h" #include "check.h" @@ -183,11 +186,7 @@ bool BenchmarkFamilies::FindBenchmarks( } } - // we know that the args are always non-negative (see 'AddRange()'), - // thus print as 'unsigned'. BUT, do a cast due to the 32-bit builds. - instance.name.args += - StrFormat("%lu", static_cast(arg)); - + instance.name.args += StrFormat("%" PRId64, arg); ++arg_i; } @@ -334,7 +333,6 @@ Benchmark* Benchmark::ArgNames(const std::vector& names) { Benchmark* Benchmark::DenseRange(int64_t start, int64_t limit, int step) { CHECK(ArgsCnt() == -1 || ArgsCnt() == 1); - CHECK_GE(start, 0); CHECK_LE(start, limit); for (int64_t arg = start; arg <= limit; arg += step) { args_.push_back({arg}); diff --git a/src/benchmark_register.h b/src/benchmark_register.h index 0705e219..61377d74 100644 --- a/src/benchmark_register.h +++ b/src/benchmark_register.h @@ -5,29 +5,103 @@ #include "check.h" +namespace benchmark { +namespace internal { + +// Append the powers of 'mult' in the closed interval [lo, hi]. +// Returns iterator to the start of the inserted range. +template +typename std::vector::iterator +AddPowers(std::vector* dst, T lo, T hi, int mult) { + CHECK_GE(lo, 0); + CHECK_GE(hi, lo); + CHECK_GE(mult, 2); + + const size_t start_offset = dst->size(); + + static const T kmax = std::numeric_limits::max(); + + // Space out the values in multiples of "mult" + for (T i = 1; i <= hi; i *= mult) { + if (i >= lo) { + dst->push_back(i); + } + // Break the loop here since multiplying by + // 'mult' would move outside of the range of T + if (i > kmax / mult) break; + } + + return dst->begin() + start_offset; +} + +template +void AddNegatedPowers(std::vector* dst, T lo, T hi, int mult) { + // We negate lo and hi so we require that they cannot be equal to 'min'. + CHECK_GT(lo, std::numeric_limits::min()); + CHECK_GT(hi, std::numeric_limits::min()); + CHECK_GE(hi, lo); + CHECK_LE(hi, 0); + + // Add positive powers, then negate and reverse. + // Casts necessary since small integers get promoted + // to 'int' when negating. + const auto lo_complement = static_cast(-lo); + const auto hi_complement = static_cast(-hi); + + const auto it = AddPowers(dst, hi_complement, lo_complement, mult); + + std::for_each(it, dst->end(), [](T& t) { t *= -1; }); + std::reverse(it, dst->end()); +} + template void AddRange(std::vector* dst, T lo, T hi, int mult) { - CHECK_GE(lo, 0); + static_assert(std::is_integral::value && std::is_signed::value, + "Args type must be a signed integer"); + CHECK_GE(hi, lo); CHECK_GE(mult, 2); // Add "lo" dst->push_back(lo); - static const T kmax = std::numeric_limits::max(); + // Handle lo == hi as a special case, so we then know + // lo < hi and so it is safe to add 1 to lo and subtract 1 + // from hi without falling outside of the range of T. + if (lo == hi) return; - // Now space out the benchmarks in multiples of "mult" - for (T i = 1; i < kmax / mult; i *= mult) { - if (i >= hi) break; - if (i > lo) { - dst->push_back(i); - } + // Ensure that lo_inner <= hi_inner below. + if (lo + 1 == hi) { + dst->push_back(hi); + return; } - // Add "hi" (if different from "lo") - if (hi != lo) { + // Add all powers of 'mult' in the range [lo+1, hi-1] (inclusive). + const auto lo_inner = static_cast(lo + 1); + const auto hi_inner = static_cast(hi - 1); + + // Insert negative values + if (lo_inner < 0) { + AddNegatedPowers(dst, lo_inner, std::min(hi_inner, T{-1}), mult); + } + + // Treat 0 as a special case (see discussion on #762). + if (lo <= 0 && hi >= 0) { + dst->push_back(0); + } + + // Insert positive values + if (hi_inner > 0) { + AddPowers(dst, std::max(lo_inner, T{1}), hi_inner, mult); + } + + // Add "hi" (if different from last value). + if (hi != dst->back()) { dst->push_back(hi); } } +} // namespace internal +} // namespace benchmark + #endif // BENCHMARK_REGISTER_H diff --git a/src/string_util.h b/src/string_util.h index fc5f8b03..c5dcee6f 100644 --- a/src/string_util.h +++ b/src/string_util.h @@ -12,7 +12,9 @@ void AppendHumanReadable(int n, std::string* str); std::string HumanReadableNumber(double n, double one_k = 1024.0); -#ifdef __GNUC__ +#if defined(__MINGW32__) +__attribute__((format(__MINGW_PRINTF_FORMAT, 1, 2))) +#elif defined(__GNUC__) __attribute__((format(printf, 1, 2))) #endif std::string diff --git a/test/benchmark_gtest.cc b/test/benchmark_gtest.cc index 10683b43..9557b20e 100644 --- a/test/benchmark_gtest.cc +++ b/test/benchmark_gtest.cc @@ -4,6 +4,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +namespace benchmark { +namespace internal { namespace { TEST(AddRangeTest, Simple) { @@ -30,4 +32,97 @@ TEST(AddRangeTest, Advanced64) { EXPECT_THAT(dst, testing::ElementsAre(5, 8, 15)); } -} // end namespace +TEST(AddRangeTest, FullRange8) { + std::vector dst; + AddRange(&dst, int8_t{1}, std::numeric_limits::max(), 8); + EXPECT_THAT(dst, testing::ElementsAre(1, 8, 64, 127)); +} + +TEST(AddRangeTest, FullRange64) { + std::vector dst; + AddRange(&dst, int64_t{1}, std::numeric_limits::max(), 1024); + EXPECT_THAT( + dst, testing::ElementsAre(1LL, 1024LL, 1048576LL, 1073741824LL, + 1099511627776LL, 1125899906842624LL, + 1152921504606846976LL, 9223372036854775807LL)); +} + +TEST(AddRangeTest, NegativeRanges) { + std::vector dst; + AddRange(&dst, -8, 0, 2); + EXPECT_THAT(dst, testing::ElementsAre(-8, -4, -2, -1, 0)); +} + +TEST(AddRangeTest, StrictlyNegative) { + std::vector dst; + AddRange(&dst, -8, -1, 2); + EXPECT_THAT(dst, testing::ElementsAre(-8, -4, -2, -1)); +} + +TEST(AddRangeTest, SymmetricNegativeRanges) { + std::vector dst; + AddRange(&dst, -8, 8, 2); + EXPECT_THAT(dst, testing::ElementsAre(-8, -4, -2, -1, 0, 1, 2, 4, 8)); +} + +TEST(AddRangeTest, SymmetricNegativeRangesOddMult) { + std::vector dst; + AddRange(&dst, -30, 32, 5); + EXPECT_THAT(dst, testing::ElementsAre(-30, -25, -5, -1, 0, 1, 5, 25, 32)); +} + +TEST(AddRangeTest, NegativeRangesAsymmetric) { + std::vector dst; + AddRange(&dst, -3, 5, 2); + EXPECT_THAT(dst, testing::ElementsAre(-3, -2, -1, 0, 1, 2, 4, 5)); +} + +TEST(AddRangeTest, NegativeRangesLargeStep) { + // Always include -1, 0, 1 when crossing zero. + std::vector dst; + AddRange(&dst, -8, 8, 10); + EXPECT_THAT(dst, testing::ElementsAre(-8, -1, 0, 1, 8)); +} + +TEST(AddRangeTest, ZeroOnlyRange) { + std::vector dst; + AddRange(&dst, 0, 0, 2); + EXPECT_THAT(dst, testing::ElementsAre(0)); +} + +TEST(AddRangeTest, NegativeRange64) { + std::vector dst; + AddRange(&dst, -4, 4, 2); + EXPECT_THAT(dst, testing::ElementsAre(-4, -2, -1, 0, 1, 2, 4)); +} + +TEST(AddRangeTest, NegativeRangePreservesExistingOrder) { + // If elements already exist in the range, ensure we don't change + // their ordering by adding negative values. + std::vector dst = {1, 2, 3}; + AddRange(&dst, -2, 2, 2); + EXPECT_THAT(dst, testing::ElementsAre(1, 2, 3, -2, -1, 0, 1, 2)); +} + +TEST(AddRangeTest, FullNegativeRange64) { + std::vector dst; + const auto min = std::numeric_limits::min(); + const auto max = std::numeric_limits::max(); + AddRange(&dst, min, max, 1024); + EXPECT_THAT( + dst, testing::ElementsAreArray(std::vector{ + min, -1152921504606846976LL, -1125899906842624LL, + -1099511627776LL, -1073741824LL, -1048576LL, -1024LL, -1LL, 0LL, + 1LL, 1024LL, 1048576LL, 1073741824LL, 1099511627776LL, + 1125899906842624LL, 1152921504606846976LL, max})); +} + +TEST(AddRangeTest, Simple8) { + std::vector dst; + AddRange(&dst, 1, 8, 2); + EXPECT_THAT(dst, testing::ElementsAre(1, 2, 4, 8)); +} + +} // namespace +} // namespace internal +} // namespace benchmark diff --git a/test/multiple_ranges_test.cc b/test/multiple_ranges_test.cc index c64acabc..b25f40eb 100644 --- a/test/multiple_ranges_test.cc +++ b/test/multiple_ranges_test.cc @@ -40,8 +40,7 @@ class MultipleRangesFixture : public ::benchmark::Fixture { // NOTE: This is not TearDown as we want to check after _all_ runs are // complete. virtual ~MultipleRangesFixture() { - assert(actualValues.size() == expectedValues.size()); - if (actualValues.size() != expectedValues.size()) { + if (actualValues != expectedValues) { std::cout << "EXPECTED\n"; for (auto v : expectedValues) { std::cout << "{"; diff --git a/test/options_test.cc b/test/options_test.cc index fdec6917..7bfc2354 100644 --- a/test/options_test.cc +++ b/test/options_test.cc @@ -35,6 +35,16 @@ BENCHMARK(BM_basic)->UseRealTime(); BENCHMARK(BM_basic)->ThreadRange(2, 4); BENCHMARK(BM_basic)->ThreadPerCpu(); BENCHMARK(BM_basic)->Repetitions(3); +BENCHMARK(BM_basic) + ->RangeMultiplier(std::numeric_limits::max()) + ->Range(std::numeric_limits::min(), + std::numeric_limits::max()); + +// Negative ranges +BENCHMARK(BM_basic)->Range(-64, -1); +BENCHMARK(BM_basic)->RangeMultiplier(4)->Range(-8, 8); +BENCHMARK(BM_basic)->DenseRange(-2, 2, 1); +BENCHMARK(BM_basic)->Ranges({{-64, 1}, {-8, -1}}); void CustomArgs(benchmark::internal::Benchmark* b) { for (int i = 0; i < 10; ++i) {