diff --git a/src/benchmark.cc b/src/benchmark.cc index 91280295e..fa7eb7485 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -222,42 +222,25 @@ State::State(std::string name, IterationCount max_iters, } } - // Note: The use of offsetof below is technically undefined until C++17 - // because State is not a standard layout type. However, all compilers - // currently provide well-defined behavior as an extension (which is - // demonstrated since constexpr evaluation must diagnose all undefined - // behavior). However, GCC and Clang also warn about this use of offsetof, - // which must be suppressed. -#if defined(__INTEL_COMPILER) -#pragma warning push -#pragma warning(disable : 1875) -#elif defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Winvalid-offsetof" -#endif -#if defined(__NVCC__) -#pragma nv_diagnostic push -#pragma nv_diag_suppress 1427 -#endif -#if defined(__NVCOMPILER) -#pragma diagnostic push -#pragma diag_suppress offset_in_non_POD_nonstandard -#endif - // Offset tests to ensure commonly accessed data is on the first cache line. - const int cache_line_size = 64; - static_assert( - offsetof(State, skipped_) <= (cache_line_size - sizeof(skipped_)), ""); -#if defined(__INTEL_COMPILER) -#pragma warning pop -#elif defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif -#if defined(__NVCC__) -#pragma nv_diagnostic pop -#endif -#if defined(__NVCOMPILER) -#pragma diagnostic pop -#endif + // Ensure commonly accessed data is on the first cache line. + // + // offsetof() cannot be used for this: State is not a standard-layout type, so + // offsetof() on it is only conditionally-supported and is diagnosed by + // -Winvalid-offsetof (and its equivalents on ICC/NVCC/NVHPC), which had to be + // silenced with a stack of vendor-specific pragmas. State also cannot be made + // standard-layout -- that would require every non-static data member to be of + // standard-layout type, and `counters` is a std::map, which is not. + // + // So compute the offset from the live object, which is well defined. Unlike + // BM_CHECK this stays enabled under NDEBUG: a State is constructed once per + // benchmark instance and thread, never inside the iteration loop, so the + // check costs nothing measurable and the invariant holds in every build. + BM_CHECK_ALWAYS(reinterpret_cast(&skipped_) - + reinterpret_cast(this) <= + /*cache_line_size=*/64 - + static_cast(sizeof(skipped_))) + << "the commonly accessed members of State must fit in the first cache " + "line"; } void State::PauseTiming() { diff --git a/src/check.h b/src/check.h index aa8c78c92..b6fc22ff0 100644 --- a/src/check.h +++ b/src/check.h @@ -79,15 +79,20 @@ class CheckHandler { } // end namespace internal } // end namespace benchmark -// The BM_CHECK macro returns a std::ostream object that can have extra -// information written to it. -#ifndef NDEBUG -#define BM_CHECK(b) \ +// Like BM_CHECK, but enabled in every build configuration. Reserved for +// invariants that are cheap enough to verify unconditionally and that must not +// be violated silently in a release build. +#define BM_CHECK_ALWAYS(b) \ (b ? ::benchmark::internal::GetNullLogInstance() \ : ::benchmark::internal::CheckHandler( \ std::string_view(#b), std::string_view(__FILE__), \ std::string_view(__func__), __LINE__) \ .GetLog()) + +// The BM_CHECK macro returns a std::ostream object that can have extra +// information written to it. +#ifndef NDEBUG +#define BM_CHECK(b) BM_CHECK_ALWAYS(b) #else #define BM_CHECK(b) ::benchmark::internal::GetNullLogInstance() #endif