[PATCH] D53087: Add benchmarks for std::function.

Eric Fiselier via Phabricator reviews at reviews.llvm.org
Thu Oct 11 10:01:39 PDT 2018


EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM after addressing inline comments.



================
Comment at: benchmarks/CartesianBenchmarks.hpp:16
+#include "benchmark/benchmark.h"
+#include "../test/support/test_macros.h"
+
----------------
I think this can be just `test_macros.h`. The build should add `support` to the include directories.


================
Comment at: benchmarks/CartesianBenchmarks.hpp:26
+template <class D, class E, size_t N, size_t... Is>
+struct MakeEnumValueTuple : MakeEnumValueTuple<D, E, N - 1, N - 1, Is...> {};
+
----------------
In C++17 we could write this as:

```
template <class D, class E, size_t ...Idxs>
constexpr auto MakeEnumValueTuple(std::index_sequence<Idxs...>) {
  return std::make_tuple(EnumValue<D, E, Idxs>{}...);
}

template <class Derived, class EnumType, size_t NumLabels>
using EnumValuesAsTuple = decltype(internal::MakeEnumValueTuple<Derived, EnumType>(std::make_index_sequence<NumLabels>{}));

```


================
Comment at: benchmarks/CartesianBenchmarks.hpp:90
+template <class T>
+TEST_ALWAYS_INLINE inline T maybeOpaque(T value, bool opaque) {
+  if (opaque) benchmark::DoNotOptimize(value);
----------------
I really need to land something like this in the Benchmark library.


================
Comment at: benchmarks/function.bench.cpp:17
+#include "benchmark/benchmark.h"
+#include "../test/support/test_macros.h"
+
----------------
`#include "test_macros.h"` should work.


Repository:
  rCXX libc++

https://reviews.llvm.org/D53087





More information about the libcxx-commits mailing list