RFR: 8328934: Assert that ABS input and output are legal [v3]

[Aleksey Shipilev]

On Fri, 12 Apr 2024 22:08:42 GMT, Andrew Haley <aph at openjdk.org> wrote:

>>> > Caught some failures, which made me think we want richer diagnostics around this. With new version, we print stuff like:
>>> > ```
>>> > #  Internal Error (/Users/shipilev/Work/shipilev-jdk/src/hotspot/share/opto/loopnode.cpp:2965), pid=32195, tid=27139
>>> > #  Error: ABS: argument should not allow overflow
>>> > ```
>>> 
>>> LOL, don't say you weren't warned! ;-)
>>> 
>>> ```
>>>   T res = (x < 0 && x != std::numeric_limits<T>::min()) ? -x : x;
>>> ```
>> 
>> I mean, we catch the proper error in some tests: https://bugs.openjdk.org/browse/JDK-8330158
>> Do we really need to do this `x != std::numeric_limits<T>::min()` dance here?
>
>> > ```
>> >   T res = (x < 0 && x != std::numeric_limits<T>::min()) ? -x : x;
>> > ```
>> 
>> I mean, we catch the proper error in some tests: https://bugs.openjdk.org/browse/JDK-8330158 Do we really need to do this `x != std::numeric_limits<T>::min()` dance here?
> 
> I think so. Several of us have worked on eliminating undefined behaviour in HotSpot and we've made good progress. I think it would be sad for new UB to be pushed now, especially in a case like this when it wouldn't be accidental. UB is just something we have to deal with, because C++. :-(

All tests, including aggressive compiler tests, are passing. I see no test failures on new asserts anymore. There are a few Fuzzer test failures due to #19001, which I don't think hide any new assert triggers. @theRealAph @dean-long -- still good with this? If so, I'll integrate.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18751#issuecomment-2083289046