RFR: 8273139: C2: assert(f <= 1 && f >= 0) failed: Incorrect frequency [v2]

Dean Long dlong at openjdk.java.net
Wed Jan 19 11:02:26 UTC 2022


On Tue, 18 Jan 2022 08:35:59 GMT, Emanuel Peter <duke at openjdk.java.net> wrote:

>> Relaxed assert which can be triggered falsely upon an unfortunate sequence of float additions, such that the sum of all probabilities exceeds 1 by some very small amount in the order of float precision.
>> 
>> Before: assert(f <= 1 && f >= 0)
>> Now: assert(f >= 0), truncate f to be <=1
>> 
>> Frequency computation is done with floats, which are calculated through counting occurrences and dividing by totals.
>> This is done in multiple locations in the code.
>> 
>> We considered three options (in conversation with @rwestrel @chhagedorn @tobiasholenstein ):
>> 1. Consistently use `fesetround` to correctly round up/down (depends on context) if the results are ever used for frequency/probability calculations (many locations, multiple files). This option requires more code and maintainance. Implementing and testing is difficult. This is fragile if new code is added that impacts frequency computation - would have to remember to round correctly.
>> 2. Modify assert to `f <= 1+eps && 1 >= 0`, where `eps` is related to float precision, and is as close to zero as possible. However, since there can be an arbitrary number of additions, this error could grow arbitrarily. It is thus not evident how to determine `eps`.
>> 3. Drop the `f <= 1` condition (our choice). The exact comparison is inadequate for floats, and there is no evident replacement. This requires less code, is easier to maintain. Disadvantage: a developer may break something and not realize because the assert is no longer present. The impact would most likely be limited to performance, and not crash the VM or cause incorrect execution.
>> 
>> Checked that tests are not affected.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   collapse two lines of code

Based on the extra discussion, it sounds like my suggestions don't really help, which is completely acceptable.  Ship it!

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

PR: https://git.openjdk.java.net/jdk/pull/7113


More information about the hotspot-compiler-dev mailing list