[PATCH] D109217: [NOT FOR COMMIT] [llvm-dwarfdump] Fix unsigned overflow when calculating stats

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D109217#2985647 <https://reviews.llvm.org/D109217#2985647>, @tschuett wrote:

> In D109217#2985644 <https://reviews.llvm.org/D109217#2985644>, @dblaikie wrote:
>
>> In D109217#2984816 <https://reviews.llvm.org/D109217#2984816>, @tschuett wrote:
>>
>>> Could something ala:
>>> https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
>>> help?
>>
>> Probably enough to write the portable code (so it works on MSVC too, etc) and let the compiler optimize it. At least for this saturation code, Clang produces the same with or without the intrinsic (& GCC produces something else entirely - to itself and to clang): https://godbolt.org/z/jb3nnaKvT
>
> Maybe LLVM should learn saturating integers that assert in debug mode?

Perhaps. (though anything that asserts in debug mode should basically be UB in non-debug mode, in my opinion - if you aren't testing/using the functionality, it shouldn't be defined)

For ints we've got UBSan, but unsigned ints are defined on wrap. There's no unsigned type that's UB on overflow - certainly might be nice to have them to clarify the difference between a think you want to do weird bitfiddling with and expect all the overflow, etc, and a thing that's meant to do maths and where sanitizers could diagnose overflow, etc.

But I think a couple of manual overflow checks here is probably OK - might be worth putting it in a generic function and applying it to all the statistics to make things more robust/generic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109217/new/

https://reviews.llvm.org/D109217