From aebb140d3e6d1837294bef12a3d922e40a1a8f08 Mon Sep 17 00:00:00 2001 From: Nathan Bronson Date: Fri, 18 Nov 2016 08:07:09 -0800 Subject: [PATCH] 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 --- folly/Benchmark.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/folly/Benchmark.h b/folly/Benchmark.h index d1db800c..4c1e2806 100644 --- a/folly/Benchmark.h +++ b/folly/Benchmark.h @@ -284,12 +284,27 @@ struct DoNotOptimizeAwayNeedsIndirect { template auto doNotOptimizeAway(const T& datum) -> typename std::enable_if< !detail::DoNotOptimizeAwayNeedsIndirect::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 auto doNotOptimizeAway(const T& datum) -> typename std::enable_if< detail::DoNotOptimizeAwayNeedsIndirect::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"); } -- 2.34.1