force read for doNotOptimizeAway(*ptr_to_small_trivial)

Summary:
doNotOptimizeAway's "X" input operand constraint is interpreted
more loosely by gcc than by clang, resulting in surprising behavior
for doNotOptimizeAway(*ptr) and a difference in behavior between gcc
and clang benchmarks.  clang also is more aggressive about placing an
input operand into a register even when the constraint would allow it to
be in memory, so an "r,m" constraint has a similar problem.  This diff
changes the input constraint so that register-sized values must actually
be copied into a register, which makes the behavior more intuitive and
more consistent across platforms.

Reviewed By: davidtgoldblatt

Differential Revision: D4199767

fbshipit-source-id: aa56a7b11cb3229b95da87295f0dfc38476959d2

diff --git a/folly/Benchmark.h b/folly/Benchmark.h
index d1db800c3900df0699f322eb088d09e22636766e..4c1e2806de166b6de65629bed1bf8de277814ca5 100644 (file)
--- a/folly/Benchmark.h
+++ b/folly/Benchmark.h
@@ -284,12 +284,27 @@ struct DoNotOptimizeAwayNeedsIndirect {
 template <typename T>
 auto doNotOptimizeAway(const T& datum) -> typename std::enable_if<
     !detail::DoNotOptimizeAwayNeedsIndirect<T>::value>::type {
-  asm volatile("" ::"X"(datum));
+  // The "r" constraint forces the compiler to make datum available
+  // in a register to the asm block, which means that it must have
+  // computed/loaded it.  We use this path for things that are <=
+  // sizeof(long) (they have to fit), trivial (otherwise the compiler
+  // doesn't want to put them in a register), and not a pointer (because
+  // doNotOptimizeAway(&foo) would otherwise be a foot gun that didn't
+  // necessarily compute foo).
+  //
+  // An earlier version of this method had a more permissive input operand
+  // constraint, but that caused unnecessary variation between clang and
+  // gcc benchmarks.
+  asm volatile("" ::"r"(datum));
 }
 
 template <typename T>
 auto doNotOptimizeAway(const T& datum) -> typename std::enable_if<
     detail::DoNotOptimizeAwayNeedsIndirect<T>::value>::type {
+  // This version of doNotOptimizeAway tells the compiler that the asm
+  // block will read datum from memory, and that in addition it might read
+  // or write from any memory location.  If the memory clobber could be
+  // separated into input and output that would be preferrable.
   asm volatile("" ::"m"(datum) : "memory");
 }