mentioned in: bitcoin#34655
"> no reason why subtracting would ever fail on an unsigned type
TrySub is meant as "try to subtract without wraparound": if i < j, it leaves i unchanged (hence the Try prefix) and returns false, so callers can act.
It's unfortunate that this has a runtime impact even outside fuzzing
I think bool ret = i >= T{j}; i -= T{j}; return ret; is nicer, because in contexts where the return value is ignored, it is exactly equivalent to just a subtraction
Do we want to be exactly equivalent though?
The current cheap branching prevents the accounting counters from wrapping, so even if accounting is wrong elsewhere, the values stay closer to what we expect.
I have tried a few variants to see the effect of the change (plain subtraction vs return + internal Assume vs mutating TrySub + caller Assume with [[unlikely]]), see https://godbolt.org/z/nf9nT9Mjj
Details
Both compilers emit the same single subtract (plus the bool zero-extend in the bool case).
For example, GCC:
sub_return_size:
sub QWORD PTR [rdi], rsi
retand for bool:
sub_return_bool:
movzx esi, sil
sub QWORD PTR [rdi], rsi
retSo return + internal Assume has zero codegen cost in non-abort builds, and adding [[unlikely]] doesn't seem to change anything there either.
Result 2: Only the mutating TrySub variant changes semantics (avoids wrap), and it necessarily adds a branch. [[unlikely]] didn't change codegen here either.
GCC (size_t case):
sub_trysub_size:
mov rax, QWORD PTR [rdi]
cmp rax, rsi
jb .Lskip
sub rax, rsi
mov QWORD PTR [rdi], rax
.Lskip:
retclang emits the same idea, just as sub then jb then store:
sub_trysub_size:
mov rax, qword ptr [rdi]
sub rax, rsi
jb .Lskip
mov qword ptr [rdi], rax
.Lskip:
ret…and the sub_trysub_unlikely_* versions were identical to sub_trysub_* for both compilers (so [[unlikely]] doesn't seem to buy anything here either; the cold path is already the taken jump).
Note: I have experimented a bit with __builtin_sub_overflow(), it does change GCC codegen slighly to the same sub; jb shape clang already emits (removing the explicit cmp), but I’m not sure it’s an actual win, so I’ll leave it for a follow-up.
" - l0rinc
mentioned in: bitcoin#34655
"> no reason why subtracting would ever fail on an unsigned type
TrySubis meant as "try to subtract without wraparound": ifi < j, it leavesiunchanged (hence theTryprefix) and returnsfalse, so callers can act.Do we want to be exactly equivalent though?
The current cheap branching prevents the accounting counters from wrapping, so even if accounting is wrong elsewhere, the values stay closer to what we expect.
I have tried a few variants to see the effect of the change (plain subtraction vs return + internal
Assumevs mutatingTrySub+ callerAssumewith[[unlikely]]), see https://godbolt.org/z/nf9nT9MjjDetails
Both compilers emit the same single subtract (plus the bool zero-extend in the bool case).
For example,
GCC:and for
bool:So return + internal
Assumehas zero codegen cost in non-abort builds, and adding[[unlikely]]doesn't seem to change anything there either.Result 2: Only the mutating
TrySubvariant changes semantics (avoids wrap), and it necessarily adds a branch.[[unlikely]]didn't change codegen here either.GCC (
size_tcase):clangemits the same idea, just assubthenjbthenstore:…and the
sub_trysub_unlikely_*versions were identical tosub_trysub_*for both compilers (so[[unlikely]]doesn't seem to buy anything here either; the cold path is already the taken jump).Note: I have experimented a bit with
__builtin_sub_overflow(), it does changeGCCcodegen slighly to the samesub; jbshapeclangalready emits (removing the explicitcmp), but I’m not sure it’s an actual win, so I’ll leave it for a follow-up.