Fix DoNotOptimize compile failure on types with const members#2254
Fix DoNotOptimize compile failure on types with const members#2254devtejasx wants to merge 2 commits into
Conversation
GCC and Clang reject objects of class types with const data members as asm output operands, since such objects cannot be stored to. This made DoNotOptimize(x) fail to compile for any such type when reached through a deduced non-const reference (issue google#1997), with no usable alternative because the const-ref overload is deprecated. Dispatch on assignability - the closest portable approximation of contains-no-const-member - and route non-assignable types to an input-only constraint with a memory clobber. Such objects cannot be legally written through anyway, so no optimization barrier is lost. Reference-member types (valid outputs, but indistinguishable by trait) conservatively take the same fallback. Fixes google#1997
Exercises DoNotOptimize with register-sized and larger-than-register types holding const data members, as lvalues, rvalues, and through a deduced non-const reference (the reproducer from issue google#1997).
|
just going to put this here first: https://github.com/google/benchmark/blob/main/AGENTS.md please explain why this is such a big problem that we need such complex code to be introduced to the library. |
|
Disclosure, per AGENTS.md: this PR was developed with AI assistance (Claude Code). I've reviewed the change and the analysis, and I stand behind both — happy to argue any part of it on the merits. On the substance — why this is a real problem:
Why the patch looks the way it does: the GCC branch already dispatches on trivially-copyable × register-size (8 overloads before this change). The patch adds one boolean trait to that existing matrix, and the fallbacks reuse the exact constraints of the existing deprecated If you'd prefer a smaller surface, two options I'd be fine with:
Happy to rework in either direction. |
Fixes #1997
Problem
GCC and Clang both reject objects of class types with
constdata members as asm output operands — such objects cannot be stored to:Since the
const&overload ofDoNotOptimizeis deprecated, there was no usable spelling at all for these types when reached through a deduced non-const reference ([](auto& v) { DoNotOptimize(v); }), as reported in #1997.Fix
Add an
internal::IsAsmOutputOperandtrait and route non-output-capable types to an input-only constraint with a memory clobber. Such objects cannot be legally written through anyway, so no optimization barrier is lost relative to the read-write constraint.There is no portable trait for "contains a const member", so the trait approximates it as "not assignable" (
is_array || is_copy_assignable || is_move_assignable):BitReftest type) are actually valid asm outputs but are indistinguishable by trait — they conservatively take the same fallback, which still compiles and keeps the clobberBoth the clang/ICC branch and the GCC ≥ 5 branch get the dispatch; the GCC branch keeps its existing register-size/trivially-copyable split in the fallbacks (
"r,m"vs"m", mirroring the deprecated const-ref overloads). MSVC needs no change (no inline asm).Testing
donotoptimize_test.ccextended with const-member types: register-sized and larger, as lvalues, rvalues, and through a deduced non-const reference; builds and runs clean with-Wall -Wextraon GCC and Clang,-O1and-O3Disclosure
Per AGENTS.md: this PR was developed with AI assistance (Claude Code). The change and analysis have been reviewed and are owned by me.