Support DoNotOptimize on types with const members (#1997)

include/benchmark/utils.h

@@ -26,7 +26,21 @@ namespace benchmark {
 
 namespace internal {
 BENCHMARK_EXPORT void UseCharPointer(char const volatile*);
-}
+
+// An object of a class type with a const data member has nothing to store
+// into, so gcc and clang reject it as an asm *output* operand (issue #1997).
+// No trait spells that directly; assignability is the closest approximation.
+// Arrays are never assignable but are valid outputs, so allow them outright.
+// A class with a reference data member is also a valid output, but is
+// indistinguishable here and is conservatively treated as an input instead.
+template <class Tp>
+struct IsValidAsmOutput {
+  typedef typename std::remove_reference<Tp>::type T;
+  static const bool value = std::is_array<T>::value ||
+                            std::is_copy_assignable<T>::value ||
+                            std::is_move_assignable<T>::value;
+};
+}  // namespace internal
 
 #if (!defined(__GNUC__) && !defined(__clang__)) || defined(__pnacl__) || \
     defined(__EMSCRIPTEN__)
@@ -51,7 +65,9 @@ inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) {
 }
 
 template <class Tp>
-inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp& value) {
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value>::type
+    DoNotOptimize(Tp& value) {
 #if defined(__clang__)
   asm volatile("" : "+r,m"(value) : : "memory");
 #else
@@ -60,13 +76,25 @@ inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp& value) {
 }
 
 template <class Tp>
-inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp&& value) {
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value>::type
+    DoNotOptimize(Tp&& value) {
 #if defined(__clang__)
   asm volatile("" : "+r,m"(value) : : "memory");
 #else
   asm volatile("" : "+m,r"(value) : : "memory");
 #endif
 }
+
+// Types that cannot be written to are passed as an input operand instead.
+// A forwarding reference covers both lvalues and rvalues here; unlike the
+// read-write overloads above, an input operand needs no separate lvalue form.
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<!internal::IsValidAsmOutput<Tp>::value>::type
+    DoNotOptimize(Tp&& value) {
+  asm volatile("" : : "r,m"(value) : "memory");
+}
 #elif (__GNUC__ >= 5)
 template <class Tp>
 BENCHMARK_DEPRECATED_MSG(BENCHMARK_DONOTOPTIMIZE_CONST_REF_DEPRECATED_MSG)
@@ -88,35 +116,49 @@ inline BENCHMARK_ALWAYS_INLINE
 
 template <class Tp>
 inline BENCHMARK_ALWAYS_INLINE
-    typename std::enable_if<std::is_trivially_copyable<Tp>::value &&
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value &&
+                            std::is_trivially_copyable<Tp>::value &&
                             (sizeof(Tp) <= sizeof(Tp*))>::type
     DoNotOptimize(Tp& value) {
   asm volatile("" : "+m,r"(value) : : "memory");
 }
 
 template <class Tp>
 inline BENCHMARK_ALWAYS_INLINE
-    typename std::enable_if<!std::is_trivially_copyable<Tp>::value ||
-                            (sizeof(Tp) > sizeof(Tp*))>::type
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value &&
+                            (!std::is_trivially_copyable<Tp>::value ||
+                             (sizeof(Tp) > sizeof(Tp*)))>::type
     DoNotOptimize(Tp& value) {
   asm volatile("" : "+m"(value) : : "memory");
 }
 
 template <class Tp>
 inline BENCHMARK_ALWAYS_INLINE
-    typename std::enable_if<std::is_trivially_copyable<Tp>::value &&
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value &&
+                            std::is_trivially_copyable<Tp>::value &&
                             (sizeof(Tp) <= sizeof(Tp*))>::type
     DoNotOptimize(Tp&& value) {
   asm volatile("" : "+m,r"(value) : : "memory");
 }
 
 template <class Tp>
 inline BENCHMARK_ALWAYS_INLINE
-    typename std::enable_if<!std::is_trivially_copyable<Tp>::value ||
-                            (sizeof(Tp) > sizeof(Tp*))>::type
+    typename std::enable_if<internal::IsValidAsmOutput<Tp>::value &&
+                            (!std::is_trivially_copyable<Tp>::value ||
+                             (sizeof(Tp) > sizeof(Tp*)))>::type
     DoNotOptimize(Tp&& value) {
   asm volatile("" : "+m"(value) : : "memory");
 }
+
+// Types that cannot be written to are passed as an input operand instead.
+// A forwarding reference covers both lvalues and rvalues here; unlike the
+// read-write overloads above, an input operand needs no separate lvalue form.
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<!internal::IsValidAsmOutput<Tp>::value>::type
+    DoNotOptimize(Tp&& value) {
+  asm volatile("" : : "r,m"(value) : "memory");
+}
 #endif
 
 #elif defined(_MSC_VER)

test/donotoptimize_test.cc

@@ -27,6 +27,27 @@ struct BitRef {
   BitRef(int i, unsigned char& b) : index(i), byte(b) {}
 };
 
+// A type with a const data member cannot be an asm output operand; passing one
+// to DoNotOptimize used to fail to compile. See issue #1997.
+struct ConstMember {
+  ConstMember() : value() {}
+  const int value;
+};
+
+// Same, but too large to be passed in a register.
+struct BigConstMember {
+  BigConstMember() : value(), padding() {}
+  const int value;
+  int padding[16];
+};
+
+// DoNotOptimize is typically reached through a deduced non-const reference,
+// which is how the types above escape the deprecated const-ref overload.
+template <class Tp>
+void PassByReference(Tp& value) {
+  benchmark::DoNotOptimize(value);
+}
+
 int main(int argc, char* argv[]) {
   benchmark::MaybeReenterWithoutASLR(argc, argv);
 
@@ -67,4 +88,14 @@ int main(int argc, char* argv[]) {
 
   // Check that accept rvalue.
   benchmark::DoNotOptimize(BitRef::Make());
+
+  ConstMember cm;
+  benchmark::DoNotOptimize(cm);
+  PassByReference(cm);
+  benchmark::DoNotOptimize(ConstMember());
+
+  BigConstMember bcm;
+  benchmark::DoNotOptimize(bcm);
+  PassByReference(bcm);
+  benchmark::DoNotOptimize(BigConstMember());
 }