RFR: 8273139: C2: assert(f <= 1 && f >= 0) failed: Incorrect frequency
Emanuel Peter
duke at openjdk.java.net
Tue Jan 18 08:08:50 UTC 2022
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.
-------------
Commit messages:
- Spelling mistake
- 8273139: C2: assert(f <= 1 && f >= 0) failed: Incorrect frequency
Changes: https://git.openjdk.java.net/jdk/pull/7113/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7113&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8273139
Stats: 18 lines in 1 file changed: 2 ins; 4 del; 12 mod
Patch: https://git.openjdk.java.net/jdk/pull/7113.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7113/head:pull/7113
PR: https://git.openjdk.java.net/jdk/pull/7113
More information about the hotspot-compiler-dev
mailing list